Fixed
Status Update
Comments
lu...@gmail.com <lu...@gmail.com> #2
Hi Clark,
thanks for reporting this issue.
When you are mentioning the "private date" in refs/meta/external-ids, are you referring to full names and e-mails?
As you mentioned, there isn't any other cleartext information in it.
P.S. The issue is valid IMHO because GitWeb/Gitiles should apply the same ACLs. However, if the data exposed isn't private or sensitive, then we can remove the security flag, otherwise, we won't.
thanks for reporting this issue.
When you are mentioning the "private date" in refs/meta/external-ids, are you referring to full names and e-mails?
As you mentioned, there isn't any other cleartext information in it.
P.S. The issue is valid IMHO because GitWeb/Gitiles should apply the same ACLs. However, if the data exposed isn't private or sensitive, then we can remove the security flag, otherwise, we won't.
lu...@gmail.com <lu...@gmail.com> #3
[Empty comment from Monorail migration]
cl...@gmail.com <cl...@gmail.com> #4
I think there are two concerns. The first is that the hashed passwords are exposed. It is my understanding that it is considered a good idea to avoid exposing hashed passwords. In this case I believe that Gerrit is generating random input data as well as salting so there is less risk because users can't reuse secrets which would then be easily exposed via collisions. But I'm not an expert on those matters and won't claim to know what the stance should be for that data.
The other is the personally identifiable data like email addresses, openids, names, etc.
In both cases it seems that if you try to fetch All-Users:refs/meta/external-ids via Gerrit's git server (over http or ssh) you are denied unless part of a group with the Access Database permission. It is for this reason that I marked this a security concern since it already appears to be Gerrit's position that this data shouldn't be exposed.
The other is the personally identifiable data like email addresses, openids, names, etc.
In both cases it seems that if you try to fetch All-Users:refs/meta/external-ids via Gerrit's git server (over http or ssh) you are denied unless part of a group with the Access Database permission. It is for this reason that I marked this a security concern since it already appears to be Gerrit's position that this data shouldn't be exposed.
hi...@google.com <hi...@google.com> #5
[Empty comment from Monorail migration]
lu...@gmail.com <lu...@gmail.com> #6
> I think there are two concerns. The first is that the hashed passwords are exposed. It is my understanding that it is considered a good idea to avoid exposing hashed passwords.
Of course, it is never a good idea to expose any type of secret, either in cleartext or not, to people that should not be authorised to access them.
>In this case I believe that Gerrit is generating random input data as well as salting so there is less risk because users can't reuse secrets which would then be easily
> exposed via collisions. But I'm not an expert on those matters and won't claim to know what the stance should be for that data.
Being cautious is always good: a rule of thumb should be "never expose data that isn't supposed to be exposed".
> The other is the personally identifiable data like email addresses, openids, names, etc.
Names and e-mail addresses are exposed anyway to anyone that have access to Gerrit. The concerns are more the other ids (OAuth etc.).
> In both cases it seems that if you try to fetch All-Users:refs/meta/external-ids via Gerrit's git server (over http or ssh) you are denied unless part of a group with the Access Database permission. It is for this reason that I marked this a security concern since it already appears to be Gerrit's position that this data shouldn't be exposed.
Thanks for clarifying it. I agree in keeping it as a security issue.
Of course, it is never a good idea to expose any type of secret, either in cleartext or not, to people that should not be authorised to access them.
>In this case I believe that Gerrit is generating random input data as well as salting so there is less risk because users can't reuse secrets which would then be easily
> exposed via collisions. But I'm not an expert on those matters and won't claim to know what the stance should be for that data.
Being cautious is always good: a rule of thumb should be "never expose data that isn't supposed to be exposed".
> The other is the personally identifiable data like email addresses, openids, names, etc.
Names and e-mail addresses are exposed anyway to anyone that have access to Gerrit. The concerns are more the other ids (OAuth etc.).
> In both cases it seems that if you try to fetch All-Users:refs/meta/external-ids via Gerrit's git server (over http or ssh) you are denied unless part of a group with the Access Database permission. It is for this reason that I marked this a security concern since it already appears to be Gerrit's position that this data shouldn't be exposed.
Thanks for clarifying it. I agree in keeping it as a security issue.
lu...@gmail.com <lu...@gmail.com> #7
I would propose from v3.3 to have the refs/meta/external-ids automatically hidden.
I do not see any valid reason why a remote user should need to access the IDs of everyone.
For existing releases, we can mention the risk of exposure of refs/meta/external-ids in the Gerrit documentation.
From v3.3 onwards, it should be protected by default.
I do not see any valid reason why a remote user should need to access the IDs of everyone.
For existing releases, we can mention the risk of exposure of refs/meta/external-ids in the Gerrit documentation.
From v3.3 onwards, it should be protected by default.
lu...@gmail.com <lu...@gmail.com> #8
@Edwin @Saša @Patrick: what do you think?
ha...@google.com <ha...@google.com> #9
HI there,
forgooglesource.com , there is no critical concern, as we don't store credentials in All-Users.
Don't certain plugins (github?) store credentials (eg OAuth tokens) as external ID ? If so, that would be a serious security issue that needs a backport to more versions.
for
Don't certain plugins (github?) store credentials (eg OAuth tokens) as external ID ? If so, that would be a serious security issue that needs a backport to more versions.
lu...@gmail.com <lu...@gmail.com> #10
Gerrit stores the HTTP credentials on the All-Users.git repository and the OAuth external ids.
Plugins (GitHub and OAuth) use that mechanism to store their external-ids, which are piece of information that are considered sensitive because are PII data.
It should be fixed from the earliest release of Gerrit possible.
I would propose v2.14, @Han-Wen: what do you think?
ha...@google.com <ha...@google.com> #11
Are the OAuth external IDs stored unencrypted? I'm concerned that once this is know, people can harvest OAuth credentials en masse, and use that to impersonate users on various sites (eg. on Github).
If admins disable the Gitiles/Gitweb integration, is the hole (provisionally) plugged?
Besides fixing it across many versions, we should also have a plan to get admins from major public sites (Openoffice, WMF, Eclipse) on an early email list for coordinated disclosure. Do we have a list of major public instances?
If admins disable the Gitiles/Gitweb integration, is the hole (provisionally) plugged?
Besides fixing it across many versions, we should also have a plan to get admins from major public sites (Openoffice, WMF, Eclipse) on an early email list for coordinated disclosure. Do we have a list of major public instances?
lu...@gmail.com <lu...@gmail.com> #12
> Are the OAuth external IDs stored unencrypted? I'm concerned that once this is know, people can harvest OAuth credentials en masse, and use that to impersonate users on various sites (eg. on Github).
Only the password is encrypted on the external-ids. All the other data, including the external IDs on any 3rd party OAuth provider, isn't.
> If admins disable the Gitiles/Gitweb integration, is the hole (provisionally) plugged?
This is more a hole in the default ACLs: by setting an ACL entry to the All-Users.git [/refs/meta/external-ids] the problem is resolved.
The issue here is that *when* the users' data were migrated to the All-Users.git repository, we migrated the data but we did not advise about adjusting the ACLs on All-Users.
> Besides fixing it across many versions, we should also have a plan to get admins from major public sites (Openoffice, WMF, Eclipse) on an early email list for coordinated disclosure. Do we have a list of major public instances?
I know WMF uses LDAP, but I have no visibility on OpenOffice or the Eclipse foundation's setups.
Because the immediate solution is *very simple and immediate*, add one ACL entry to the All-Users.git project, we could disclose it earlier and add the missing documentation entry in the Gerrit release notes from v2.14 onwards.
Also, reaching out the Wikimedia's Eclipse's and other public sites would be really appreciated.
Only the password is encrypted on the external-ids. All the other data, including the external IDs on any 3rd party OAuth provider, isn't.
> If admins disable the Gitiles/Gitweb integration, is the hole (provisionally) plugged?
This is more a hole in the default ACLs: by setting an ACL entry to the All-Users.git [/refs/meta/external-ids] the problem is resolved.
The issue here is that *when* the users' data were migrated to the All-Users.git repository, we migrated the data but we did not advise about adjusting the ACLs on All-Users.
> Besides fixing it across many versions, we should also have a plan to get admins from major public sites (Openoffice, WMF, Eclipse) on an early email list for coordinated disclosure. Do we have a list of major public instances?
I know WMF uses LDAP, but I have no visibility on OpenOffice or the Eclipse foundation's setups.
Because the immediate solution is *very simple and immediate*, add one ACL entry to the All-Users.git project, we could disclose it earlier and add the missing documentation entry in the Gerrit release notes from v2.14 onwards.
Also, reaching out the Wikimedia's Eclipse's and other public sites would be really appreciated.
ha...@google.com <ha...@google.com> #13
[Empty comment from Monorail migration]
ek...@google.com <ek...@google.com> #14
Thanks again for the report. The Gerrit team at Google is working on a fix for this issue. We'll update this issue once it's available.
ma...@gmail.com <ma...@gmail.com> #16
[Empty comment from Monorail migration]
ma...@gmail.com <ma...@gmail.com> #17
[Empty comment from Monorail migration]
lu...@gmail.com <lu...@gmail.com> #18
Adding Eryk (CollabNet) as discussed over the ESC mailing list.
lu...@gmail.com <lu...@gmail.com> #19
@Clark I tried to reproduce the problem with the stable-3.2 and the default Gerrit permissions, but when I try to access the All-Users.git/refs/meta/external-ids I get the "Cannot parse URL as a Gitiles URL" error, which is correct and expected.
Can you share your permissions for All-Projects.git and All-Users.git?
I believe I am missing something related to your use-case.
Luca.
Can you share your permissions for All-Projects.git and All-Users.git?
I believe I am missing something related to your use-case.
Luca.
lu...@gmail.com <lu...@gmail.com> #20
@Clark to clarify my feedback: your point 4 I believe is related to GitWeb whilst Gitiles I believe is working as expected.
Your point was:
4. Either navigate by hand or directly modify the url link to show refs/meta/config. For example:https://server.test/gitweb?p=All-Users.git;a=commit;h=refs/meta/external-ids or https://server.test/plugins/gitiles/All-Users/+/refs/meta/external-ids
Thanks for your feedback.
Luca.
Your point was:
4. Either navigate by hand or directly modify the url link to show refs/meta/config. For example:
Thanks for your feedback.
Luca.
cl...@gmail.com <cl...@gmail.com> #21
We've been testing the prescribed mitigations so I'm looking at older versions of the project.config files in All-Users and All-Projects and think I got the correct contents/revisions for you.
As far as getting things from gitiles and gitweb from memory you had to be logged in to see them and we were testing with a non admin user to do that (regular registered user). The refs were then visible via both gitweb and gitiles. It did not seem to work anonymously. There is a possibility that memory is bad and we accidentally fetched those paths as admin users in which case maybe its ok to have permissions to view those contents. But I seem to recall we had tested explicitly with a non admin account.
As far as getting things from gitiles and gitweb from memory you had to be logged in to see them and we were testing with a non admin user to do that (regular registered user). The refs were then visible via both gitweb and gitiles. It did not seem to work anonymously. There is a possibility that memory is bad and we accidentally fetched those paths as admin users in which case maybe its ok to have permissions to view those contents. But I seem to recall we had tested explicitly with a non admin account.
lu...@gmail.com <lu...@gmail.com> #22
Thanks @Clark, I am going to setup a similar test environment with those ACLs and will come back to you shortly.
Let me clarify: I have verified the problem with GitWeb, but I cannot reproduce the problem with Gitiles. Gitiles uses a more sophisticated wrapping of the repository access to provide extra security: I tested and did some specific debugging and I did not manage to break it, without being a Gerrit administrator of course.
However, I'll double-check again with your ACLs.
Let me clarify: I have verified the problem with GitWeb, but I cannot reproduce the problem with Gitiles. Gitiles uses a more sophisticated wrapping of the repository access to provide extra security: I tested and did some specific debugging and I did not manage to break it, without being a Gerrit administrator of course.
However, I'll double-check again with your ACLs.
lu...@gmail.com <lu...@gmail.com> #23
Thanks @Clark, I can reproduce the Gitiles issue now :-)
It must be something related to the ACLs, going to investigate further now. The default Gerrit ACLs do not expose the problem, I need to understand why.
It must be something related to the ACLs, going to investigate further now. The default Gerrit ACLs do not expose the problem, I need to understand why.
lu...@gmail.com <lu...@gmail.com> #24
@Clack, yes, I can reproduce the issue now: see the attached screenshot.
The problem is in the ACLs indeed, other than in Gerrit. I'm developing a fix for it as we speak.
The problem is in the ACLs indeed, other than in Gerrit. I'm developing a fix for it as we speak.
lu...@gmail.com <lu...@gmail.com> #25
@Clark I managed to find the culprit of the problem, apart from the bug that we are fixing.
If you are running a "default Gerrit setup" with the initial set of permissions, you would notice the special ACL of the 'refs/meta/config':
[access "refs/meta/config"]
exclusiveGroupPermissions = read
create = group Administrators
create = group Project Owners
label-Code-Review = -2..+2 group Administrators
label-Code-Review = -2..+2 group Project Owners
push = group Administrators
push = group Project Owners
read = group Administrators
read = group Project Owners
submit = group Administrators
submit = group Project Owners
Example:
1. Run:
docker run -ti -p 8080:8080 -p 29418:29418 -e CANONICAL_WEB_URL=http://localhost:8080 gerritcodereview/gerrit:3.2.3
2. Visit the URL:
http://localhost:8080/plugins/gitiles/All-Projects/+/refs/meta/config/project.config
The important bit of the ACL is the "exclusiveGroupPermissions = read" part that makes sure that even if you have a permissive permission on refs/* that won't allow other users accessing the configuration of the projects, but only the Project Owners and the Gerrit admins will be able to access it.
Your ACL for "refs/meta/config" is:
[access "refs/meta/config"]
read = group Project Owners
create = group Administrators
create = group Project Owners
As you can notice, there isn't any "exclusive" flag applied, that means that the "read = group Project Owners" is really useless, because the permissive refs/* would allow to override that.
With the fix we are developing, that won't be enough to see users' PII data anyway, ad we will still honour the "Access Database" global capability.
However, I do *strongly* recommend you to add the "exclusiveGroupPermissions = read" to your All-Projects's ACL on the "refs/meta/config", as his absence is possibly just a mistake.
Hope that makes sense to you.
If you are running a "default Gerrit setup" with the initial set of permissions, you would notice the special ACL of the 'refs/meta/config':
[access "refs/meta/config"]
exclusiveGroupPermissions = read
create = group Administrators
create = group Project Owners
label-Code-Review = -2..+2 group Administrators
label-Code-Review = -2..+2 group Project Owners
push = group Administrators
push = group Project Owners
read = group Administrators
read = group Project Owners
submit = group Administrators
submit = group Project Owners
Example:
1. Run:
docker run -ti -p 8080:8080 -p 29418:29418 -e CANONICAL_WEB_URL=
2. Visit the URL:
The important bit of the ACL is the "exclusiveGroupPermissions = read" part that makes sure that even if you have a permissive permission on refs/* that won't allow other users accessing the configuration of the projects, but only the Project Owners and the Gerrit admins will be able to access it.
Your ACL for "refs/meta/config" is:
[access "refs/meta/config"]
read = group Project Owners
create = group Administrators
create = group Project Owners
As you can notice, there isn't any "exclusive" flag applied, that means that the "read = group Project Owners" is really useless, because the permissive refs/* would allow to override that.
With the fix we are developing, that won't be enough to see users' PII data anyway, ad we will still honour the "Access Database" global capability.
However, I do *strongly* recommend you to add the "exclusiveGroupPermissions = read" to your All-Projects's ACL on the "refs/meta/config", as his absence is possibly just a mistake.
Hope that makes sense to you.
cl...@gmail.com <cl...@gmail.com> #26
That is good to know thanks.
Looking at our git log history in All-Projects refs/meta/config:project.config Gerrit 2.2.1 initialized refs/meta/config read = group Project Owners and it has never been changed since. This would appear to then be a bug in Gerrit's upgrade system. You may want to consider notifying Gerrit admins check this read permission and set it to exclusive as part of this bug disclosure as others may be in the same boat of upgrading from Gerrit with the defective ACL.
Looking at our git log history in All-Projects refs/meta/config:project.config Gerrit 2.2.1 initialized refs/meta/config read = group Project Owners and it has never been changed since. This would appear to then be a bug in Gerrit's upgrade system. You may want to consider notifying Gerrit admins check this read permission and set it to exclusive as part of this bug disclosure as others may be in the same boat of upgrading from Gerrit with the defective ACL.
lu...@gmail.com <lu...@gmail.com> #27
@Clark yes, I do agree: we have a second bug, the missing "exclusive" flag.
We do need a second fix then, because the exposure of "refs/meta/config" to all users, potentially also the unauthenticated ones, is also very bad: a system should never expose his ACLs to a potentially malicious (or unknown) user :-(
Luca.
We do need a second fix then, because the exposure of "refs/meta/config" to all users, potentially also the unauthenticated ones, is also very bad: a system should never expose his ACLs to a potentially malicious (or unknown) user :-(
Luca.
lu...@gmail.com <lu...@gmail.com> #28
Bingo, @Clark the problem was fixed in Gerrit v2.5 by Edwin over 8 years ago:
https://gerrit-review.googlesource.com/c/gerrit/+/35583
The problem was advertised in the release notes (see [1]) in the "Access Rights" section:
"
Make read access to refs/meta/config by default exclusive to project owners
When initializing a new site a set of default access rights is configured on the All-Projects project. These default access rights include read access on refs/* for Anonymous Users and read access on refs/meta/config for Project Owners. Since the read access on refs/meta/config for Project Owners was not exclusive, Anonymous users were able to access the refs/meta/config branch which by default should only be accessible by the project owners.
"
That means, *IF* the Gerrit site was created from v2.5 onwards (i.e. less than 8 years old), then it would NOT be affected by this issue. Otherwise, it could be potentially affected if the upgrade to v2.5 was done without adjusting the permissions on All-Projects, as it was indicated in the release notes.
What we *could* do is to introduce a check when upgrading to the latest v3.3 and, if the "exclusive" flag is missing, then we could add it by default during init.
WDYT?
Luca.
The problem was advertised in the release notes (see [1]) in the "Access Rights" section:
"
Make read access to refs/meta/config by default exclusive to project owners
When initializing a new site a set of default access rights is configured on the All-Projects project. These default access rights include read access on refs/* for Anonymous Users and read access on refs/meta/config for Project Owners. Since the read access on refs/meta/config for Project Owners was not exclusive, Anonymous users were able to access the refs/meta/config branch which by default should only be accessible by the project owners.
"
That means, *IF* the Gerrit site was created from v2.5 onwards (i.e. less than 8 years old), then it would NOT be affected by this issue. Otherwise, it could be potentially affected if the upgrade to v2.5 was done without adjusting the permissions on All-Projects, as it was indicated in the release notes.
What we *could* do is to introduce a check when upgrading to the latest v3.3 and, if the "exclusive" flag is missing, then we could add it by default during init.
WDYT?
Luca.
cl...@gmail.com <cl...@gmail.com> #30
Automating a check might be nice simply because it seems at least one Gerrit had missed that in the past. However, if we are the only one adding a check might be too much effort for little gain. Not sure I can gauge that for you. Thank you for the release note link. We'll be working to update this soon in our installation. Looks like even newer Gerrit mixes in Administrator access as well. We'll probably add that too.
cl...@gmail.com <cl...@gmail.com> #31
We originally discovered this bug in the course of testing newer Gerrit versions in preparation for an upgrade from 2.13, and that upgrade has been long scheduled to start on Friday of this week. At this point we're drawing dangerously close to being unable to build and adequately test newer container images given the lack of an available patch, and are wondering if there is still hope of it being available in the next 24 hours or whether we should reschedule our upgrade.
Commenting here on this bug and not the other one, as the other one has more people subscribed to it now that likely aren't interested in OpenDev's upgrade plans.
Note that we have tested some of the suggested mitigations, and they don't work well with replication for us so are hoping to use the proper fixes instead.
Commenting here on this bug and not the other one, as the other one has more people subscribed to it now that likely aren't interested in OpenDev's upgrade plans.
Note that we have tested some of the suggested mitigations, and they don't work well with replication for us so are hoping to use the proper fixes instead.
lu...@gmail.com <lu...@gmail.com> #32
[Empty comment from Monorail migration]
ek...@google.com <ek...@google.com> #33
[Empty comment from Monorail migration]
ma...@gmail.com <ma...@gmail.com> #34
[Empty comment from Monorail migration]
ek...@google.com <ek...@google.com> #35
[Monorail components: SteeringCommittee]
ek...@google.com <ek...@google.com> #36
[Monorail components: -ESC]
is...@google.com <is...@google.com> #37
Edits were made to reflect the following in Monorail: auto-CCs.
Description
Affected Version: Gerrit 3.2.3-206-gcd50998875-dirty via stable-3.2 build
What steps will reproduce the problem?https://server.test/gitweb?p=All-Users.git;a=commit;h=refs/meta/external-ids or https://server.test/plugins/gitiles/All-Users/+/refs/meta/external-ids
1. Log in to gerrit
2. Browse -> Repositories and filter to All-Users
3. Select gitweb link or browse link (gitiles)
4. Either navigate by hand or directly modify the url link to show refs/meta/config. For example:
5. You can now see all other users hashed api keys.
What is the expected output?
Only users with Access Database should be able to navigate All-Users:refs/meta/external-ids. This seems to be how things work if you try to git fetch the ref.
What do you see instead?
Registered users (not just Administrators) have access to this sensitive notedb content.
Please provide any additional information below.
This is our gitweb config:
Gerrit upstream hardcodes a .git extension for cgit.
The cgit settings below are the same just without the
.git extension.
[gitweb]
type = gitweb
cgi = /usr/share/gitweb/gitweb.cgi
revision = "?p=${project}.git;a=commitdiff;h=${commit}"
If I manually add an acl for refs/meta/* in All-Users setting Exclusive access to administrators the problem goes away. The problem seems to be that we're enforcing the Access Database controls outside of normal ACLs but gitiles and gitweb rely on normal ACLs to properly filter access.
I know the api passwords are hashed and gerrit doesn't let you input arbitrary secrets to hash making them more difficult to attack, but the security stance elsewhere seems to be that this is private data so I expect it should be made private from gitweb and gitiles as well.
As a separate question is my ACL workaround a valid workaround for this problem? Should the ACL looks like something else?