⚠️ WIP ⚠️ Enable forwarding auth challenges to client#48847
Draft
laurazard wants to merge 1 commit intomoby:masterfrom
Draft
⚠️ WIP ⚠️ Enable forwarding auth challenges to client#48847laurazard wants to merge 1 commit intomoby:masterfrom
laurazard wants to merge 1 commit intomoby:masterfrom
Conversation
Member
|
Looks like it needs a rebase 🙈 |
Member
Author
|
It doesn't really matter, it has vendored code that won't compile (yet), it's just for discussion 😅 |
laurazard
force-pushed
the
client-stream-auth-requests
branch
2 times, most recently
from
393293d to
78f8133
Compare
Member
Author
|
No conflicts/it builds now :') Also significantly simplified the implementation. |
laurazard
force-pushed
the
client-stream-auth-requests
branch
4 times, most recently
from
3bf75ff to
01ea7c2
Compare
laurazard
added
area/api
laurazard
force-pushed
the
client-stream-auth-requests
branch
from
01ea7c2 to
e2d31a1
Compare
Currently, for registry operations requiring auth (such as pull/push), the client is expected to preemptively send credentials it believes will be appropriate/required alonside with the image pull/push request to the daemon, which the daemon will then use to negotiate any challenges (in the form of `WWW-Authenticate` headers) it encounters while communicating with the registry. This means that, as it stands, the daemon has no avenue of communication to request credentials as-needed, during a registry operation – all the credentials must be sent up-front by the client (who without foresight must somehow know credentials will actually be necessary – which poses challenges when paired with mirrors for private registries, etc. since in cases like these the client won't necessarily know where the engine will actually be pulling images from, and we also don't want to just send client credentials to any host). This patch introduces a new way of handling auth during push/pull – following what has been discussed in moby#38591, instead of negotiating auth on behalf of the client (using the provided credentials), the daemon will instead send a `401` back to the client alonside with the received `WWW-Authenticate` header when a challenge is received. The client can then handle the challenge however it sees fit, pick the credentials it wants to use (which addresses the issue with private registry mirrors) and then initiate another pull with the relevant credentials. This new auth handling flow is also better suited for situations where oauth is being used to authenticate against the registry – in case it's necessary to do a refresh flow to fetch a new access token, the client can handle the refresh flow, store the new refresh+access tokens, and start the pull with the new fresh credentials. Signed-off-by: Laura Brehm <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
client-stream-auth-requests
branch
from
e2d31a1 to
a31f030
Compare
github-actions
Bot
added
area/images
Member
|
Did a rebase, as I think this is still something to explore /cc @Benehiko (some changes got easier, as we now have some option-structs for the backend, so no need to change signatures for those) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
Implement client auth protocol – as discussed in #38591.
With this change, a client can opt-in to the new client auth flow by including a
clientAuthURL parameter set to a non-falsy value ("0","false","[empty]").If a client opts in, any
401response with aWWW-Authenticateheader returned by the registry causes the pull to fail and a401response with the unalteredWWW-Authenticateheader to be returned to the client.The client can then handle this challenge any way it sees fit, and initiate another request with the relevant auth configuration.
Consider an OAuth authentication scenario: previously, a client could make use of the
IdentityTokenmoby/api/types/registry/authconfig.go
Line 43 in 48e43eb
RegistryTokenmoby/api/types/registry/authconfig.go
Line 46 in 48e43eb
AuthConfigstruct to send along a bearer and refresh token to be used for the registry.When using the
IdentityTokenfield, on any401response from the registry with anWWW-Authenticateheader, the engine would attempt to use the refresh token passed here against therealmfrom the challenge to fetch an access token to use for authentication. After fetching the new credentials, the engine does not send them back to the client, which means the client cannot store the new, refreshed credentials.On top of that, OAuth tenants frequently implement refresh token rotation – with this practice, each time that a client refreshes it's credentials with a refresh token flow, it receives a new access token AND a new refresh token, which it's expected to store and use for future refresh flows. When this is done, each individual refresh token has a shorter lifetime, after which it cannot be used. This is fine if a client stores the new refresh token every time it does a refresh flow, since the new refresh token has a fresh lifetime. However, given that the engine does not forward the new access + refresh tokens to the client, this flow is incompatible with the current engine auth implementations.
With the new client auth protocol implemented in this patch, instead of the client sending along a refresh token with the pull request, the client instead initiates the pull request, receives a
401back from the engine with the auth challenge, refreshes it's credentials (and can store the new – rotated – refresh token), and then restarts the pull request with the relevant credentials for authentication.- How I did it
Add a
clientAuthflag to the/images/createendpoint, which toggles the client auth handling flow.Also added a
ChallengeHandlerFuncfield to the APIPullOptionsstruct, which is used to pass along "challenge handling closure" to be used by the API client.If the client auth flow is used, then a new
clientChallengeAuthorizeris used when initiating the request, which returns the new error typeErrAuthenticationChallengeif any challenge is received. This newErrAuthenticationChallengeis bubbled up to the API handler, which handles it by responding with a401with the returnedWWW-Authenticateheader.- How to verify it
Check the CLI PR implementing this – docker/cli#5606.
Note
While working at this, I first implemented something more akin to master...simonferquel:auth-streaming – i.e. negotiate credentials by streaming the auth requests while the pull is ongoing instead of responding to the
/images/createrequest with a401and having the client handle the challenge and start the pull again. I can push this if it makes sense, but I prefer this more conventional/less complex approach.- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)