Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read-only credentials #628

Open
gzm0 opened this issue Jan 10, 2023 · 4 comments
Open

Read-only credentials #628

gzm0 opened this issue Jan 10, 2023 · 4 comments

Comments

@gzm0
Copy link

gzm0 commented Jan 10, 2023

Follow up to #381, #59.
Related: #59 (comment)

It would be nice to have an option for different users for write / read permissions (without anonymous reads).

The use case is a company private cache where only CI is allowed to write to the cache but developers are allowed to pull from the cache.

@mostynb
Copy link
Collaborator

mostynb commented Jan 10, 2023

bazel-remote doesn't currently have an authorization library (I don't think htpasswd files support this, and I'm not sure of a good/general way to configure this for mTLS).

So the only way I can think you could do this right now would be to use a second proxy server for authenticated-but-readonly access:

  • bazel-remote running with some form of authentication, which is only available on your CI, and --allow_unauthenticated_reads.
  • CI access to bazel-remote is allowed by firewall rules.
  • A proxy which only accepts requests from a second authentication group (ie developers), and makes unauthenticated requests to bazel-remote is also allowed by firewall rules.
  • non-CI (ie developer) access to bazel-remote is rejected by firewall rules, but authenticated access to the proxy is allowed.

I'm open to suggestions for libraries that could support this entirely inside bazel-remote.

@gzm0
Copy link
Author

gzm0 commented Jan 11, 2023

Thanks for getting back so quickly.

We are currently running the solution with a proxy in front (and bazel-remote works neatly FTR). But that means we're running a full nginx just to check a passwd file.

I've had a look: It seems go-http-auth supports retrieving the username that is authenticated:

When bazel-remote calls JustCheck here:

return auth.JustCheck(authenticator, handler)

That lands here:
https://github.com/abbot/go-http-auth/blob/d3faa315ed8c8d4d7e5a127c0345b42b60b58872/auth.go#L102-L110

So it seems we could:

  1. Add a config option: allow_write_users (or similar)
  2. Check against that in that handler based on method (similar to the check just below:

    bazel-remote/main.go

    Lines 434 to 451 in be88dad

    func unauthenticatedReadWrapper(handler http.HandlerFunc, secrets auth.SecretProvider, addr string) http.HandlerFunc {
    authenticator := &auth.BasicAuth{Realm: addr, Secrets: secrets}
    return func(w http.ResponseWriter, r *http.Request) {
    if r.Method == http.MethodGet || r.Method == http.MethodHead {
    handler(w, r)
    return
    }
    if authenticator.CheckAuth(r) != "" {
    handler(w, r)
    return
    }
    http.Error(w, "Authorization required", http.StatusUnauthorized)
    // TODO: pass in a logger so we can log this event?
    }
    }
    )

Having a "duplicate" of the user list in htpasswd and the config is arguably not super nice. An alternative would be to have two htpasswd files one for read users and one for write users. This then would leave us with the following options:

  1. Check the read httpasswd first, then the write htpasswd:
    This might have a performance impact as it duplicates auth lookups for writes (but maybe thats OK). Also might be harder to implement with go-http-auth (we can't just wrap anymore since we need to control the rejection path).
  2. For read requests check the read htpasswd, for write requests check the write htpasswd:
    Easier to implement, but annoying to use, since now write users need to be duplicated (this is what we currently need to do, because of limitations in the nginx config).

WDYT? I'm happy to give any of these a shot and send a PR if you think any of them is a viable option.

@mostynb
Copy link
Collaborator

mostynb commented Jan 12, 2023

I've been thinking about this, and I'm a little worried about letting the features diverge between mTLS and basic auth. So I think we should do a bit more research to see what other options there are first.

In the meantime, I think running a separate proxy as you are doing is a reasonable solution.

@gzm0
Copy link
Author

gzm0 commented Jan 13, 2023

That's a good point. My initial thought was that we could use the common name on the subject of the client certificate.

What will get a bit trickier is that there can be multiple usernames now (technically multiple TLS certificates or from basic auth), so we need to check that at least one is authorized.

All that being said, it is true that this is getting quite non-trivial. So I empathize with the desire of seeing if there is a library in the ecosystem somewhere that does that (or is configurable to do that). I'm not sure I'm a good help here though (I have done a lot of Go, but not OSS so I have zero clues how to navigate the ecosystem).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants