New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 777065 link

Starred by 1 user

Issue metadata

Status: Duplicate
Owner: ----
Closed: Sep 16
Cc:
Components:
EstimatedDays: 10
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 878043



Sign in to add a comment

Gerrit private issues are not private!

Project Member Reported by bradnelson@chromium.org, Oct 21 2017

Issue description

Gerrit provides URLs that show all incoming git refs:
https://chromium.googlesource.com/chromium/tools/depot_tools/+refs/changes/?format=json

So while an URL like this is private:
https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/731834

Someone that knows what they're doing can fish out from this code reviews that are private, and look at the raw commits:
https://chromium.googlesource.com/chromium/tools/depot_tools/+/65d24a2fa5fe713f24cdb0be9e6667e0f587baf1

We probably only use the changes URL for trybot / buildbot triggering.
We should consider access restricting this.

 
I filed internal gerrit bug https://b.corp.google.com/issues/68063399
This is tracked in buganizer here:
https://b.corp.google.com/issues/68063399

> We should consider access restricting this.

I think we can't restrict read access to refs/changes/* to all devs, because doing so will break "git cl patch" for everybody (maybe except CL owners).

From my PoV, Gerrit should automatically restrict read access to refs/changes/xx/xxyyzz/* at the time of this ref creation iff CL is private.
Cc: aga...@chromium.org
Components: -Infra Infra>Security Infra>SDK
Meanwhile, I think we should loudly FAIL if "git cl upload --private" instead of succeeding and deceiving users:

https://chromium-review.googlesource.com/#/c/chromium/tools/depot_tools/+/731848
Components: Infra>Codereview>Gerrit
Cc: danno@chromium.org hablich@chromium.org

Comment 7 by aga...@chromium.org, Oct 23 2017

All-Projects already grants Read on refs/* to Anonymous Public Users. We can remove reader<all: USERS> from the gob-ctl ACL on chromium/chromium/src, so that read requests will fall through to Gerrit.

I'm git admin, trying to do this now. Problem is that there are a bunch of outbound copy configs using AllUsers as their source scope, which need to be rewritten to use something else.
Owner: aga...@chromium.org
Assigning to agable who has an idea how to fix this.

Comment 9 by aga...@chromium.org, Oct 31 2017

Cc: -aga...@chromium.org
Turns out this is way harder than we thought. If we remove the reader<all: USERS> from the gob-ctl ACL, then codesearch will stop working, since codesearch uses an API which doesn't fall through to gerrit-level permission checks, and apparently doesn't identify itself as a user.

Trying to figure out the next step.
I have resolved the issue for codesearch (both the current and next-gen codesearch mdb groups are explicitly granted Reader access at the host level).

I tried revoking the all_users Read acl at the host level, and some repos immediately stopped serving the /+refs/ endpoint. I'm not sure what happened there. Further experimentation is required.
Components: Infra>Git>Admin
Labels: -Pri-1 Pri-2
Owner: ----
Status: Available (was: Assigned)
Unassigning from myself because I'm on sabbatical. Releasing to Git Admin queue to think about permissions.
EstimatedDays: 10
This git admin also thinks that revoking all_users Read acl at the host level is a good thing. But, I don't know how to predict what will get broken after this.
To me this bug seems like a small 1-2 weeks long project.
Blockedon: 878043
Blockedon: -878043
Mergedinto: 878043
Status: Duplicate (was: Available)
Decision was made in issue 878043
Blockedon: 878043
Project Member

Comment 16 by sheriffbot@chromium.org, Dec 23

Labels: -Restrict-View-SecurityTeam allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment