Page MenuHomePhabricator

Re-enable use of Gerrit HTTP token to push patchsets
Open, Needs TriagePublic

Description

I believe this was disabled in https://gerrit.wikimedia.org/r/c/operations/puppet/+/497066/1/modules/gerrit/templates/gerrit.config.erb

Long-term this is a feature we want, and has been used by multiple scripts/bots.

Just to be clear, is the removal of the Gerrit HTTP token feature going to be a permanent change or just temporary? If this is a permanent thing I'll have to figure out how to modify libraryupgrader as well...

I hope not. We definitely don't want to lose gerrit rest api in the long run.

Event Timeline

Legoktm created this task.Mar 20 2019, 12:06 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 20 2019, 12:06 AM
Paladox moved this task from Bugs & stuff to Local hacks on the Gerrit board.Mar 22 2019, 11:18 PM

Before we move forward and enable this, let's make sure we have understood the security repercussions and have mitigated them (and if we find it impossible to do so, avoid it).

This is a double edge knife feature as it allows anyone with temporary access to a browser session to set a gerrit password and perform actions on behalf of the user, possibly unnoticed for a long time. It's a secondary authentication method. One, that many new time (and perhaps old time) users are not trained to expect (and don't really expect). It also defeats one of the security practices that users may be following, namely rotating passwords often (and does so in a very unclear way for many people). It's noteworthy that It has been used recently to upload malicious changes to a repo in a case where the attacker had 0 knowledge of the LDAP authentication credentials of the victim. Granted, the attacker had access to the account and the web interface and as such would have been able to create the aforementioned change via the gerrit UI, but they clearly found it easier to just set the HTTP password and use it.

I don't know the gerrit permissions model/system, But could we perhaps only enable it for a small group, so anyone that wants it would need to request access? That should reduce the risk level depending on how many people actually use the feature.

That's not how that works unfortunately, it's either enable it for all, or disable it for all (since this is enabled though a gerrit config).

Before we move forward and enable this, let's make sure we have understood the security repercussions and have mitigated them (and if we find it impossible to do so, avoid it).

+1

This is a double edge knife feature as it allows anyone with temporary access to a browser session to set a gerrit password and perform actions on behalf of the user, possibly unnoticed for a long time. It's a secondary authentication method. One, that many new time (and perhaps old time) users are not trained to expect (and don't really expect). It also defeats one of the security practices that users may be following, namely rotating passwords often (and does so in a very unclear way for many people). It's noteworthy that It has been used recently to upload malicious changes to a repo in a case where the attacker had 0 knowledge of the LDAP authentication credentials of the victim. Granted, the attacker had access to the account and the web interface and as such would have been able to create the aforementioned change via the gerrit UI, but they clearly found it easier to just set the HTTP password and use it.

The same could be said of the ssh key. Temporary access to a browser session would enable someone to add a key to perform actions on behalf of a user. The gerrit command line tools have mostly the same capabilities as the http token.

This is a double edge knife feature as it allows anyone with temporary access to a browser session to set a gerrit password and perform actions on behalf of the user, possibly unnoticed for a long time. It's a secondary authentication method. One, that many new time (and perhaps old time) users are not trained to expect (and don't really expect). It also defeats one of the security practices that users may be following, namely rotating passwords often (and does so in a very unclear way for many people). It's noteworthy that It has been used recently to upload malicious changes to a repo in a case where the attacker had 0 knowledge of the LDAP authentication credentials of the victim. Granted, the attacker had access to the account and the web interface and as such would have been able to create the aforementioned change via the gerrit UI, but they clearly found it easier to just set the HTTP password and use it.

The same could be said of the ssh key. Temporary access to a browser session would enable someone to add a key to perform actions on behalf of a user. The gerrit command line tools have mostly the same capabilities as the http token.

That's really good point.

The same could be said of the ssh key. Temporary access to a browser session would enable someone to add a key to perform actions on behalf of a user. The gerrit command line tools have mostly the same capabilities as the http token.

That's really good point.

It is indeed. Could we perhaps mitigate some of the security ramifications of both of these by having some form of notification? e.g. an email with

  • Your gerrit HTTP password has been changed, report to <INSERT contact point> if it wasn't done by you
  • An SSH key was added/removed to your gerrit account, report to <INSERT contact point> if it wasn't done by you.

would suffice in my mind.

@Paladox any idea if such a feature exists in gerrit?

Paladox added a comment.EditedMon, Apr 22, 1:31 PM

Gerrit sends a email if a ssh key is added (not sure about if it's removed). Gerrit does not notify if a http password has changed (though that would be great to implement!)

I can speak to upstream about adding a notification for http password changes.

I've done 2 changes https://gerrit-review.googlesource.com/c/gerrit/+/221772/ and https://gerrit-review.googlesource.com/c/gerrit/+/221774/ which will help users know if someone removed their ssh key or changed there http password (not to be confused with auth password).

Paladox added a subtask: Restricted Task.Wed, Apr 24, 4:11 PM
Paladox removed a subtask: Restricted Task.Mon, May 6, 10:21 PM
Paladox added a subtask: Restricted Task.Fri, May 17, 7:52 PM
thcipriani closed subtask Restricted Task as Invalid.Sat, May 18, 1:50 PM
akosiaris reopened subtask Restricted Task as Open.Sun, May 19, 7:40 AM