dealing with the lack of client secret for refresh token

[panva]

Hello,

attempting to use AppAuth for android i ran into several issues with my server implementation. First off, my server supports PKCE but does not enforce it for application_type native clients, since my assumption was that clients supporting PKCE are protecting their requests with code challenge IN ADDITION to regular secret and otherwise oauth2 handling applies as a fallback(rfc7636#section-5)

Now seeing a pretty awesome (thank you!) AppAuth-Android implementation it looks like my IN ADDITION assumption is wrong since without dynamic registration there is no way to configure a secret in your library. So my thinking is i will still validate client_secret in my server when one is provided, but otherwise assume code_verifier should be present and verified appropriately.

This pretty much deals with grant_type=authorization_code token requests, but what about refresh_token? There is no secret and there is no mention of, in the spec or in the code, of sending the previous code_verifier again with the refresh token request.

Obviously securing refresh token call with client secret brings no security, so might as well just verify that the provided native client_id belongs to the provided refresh token and be done with it(respond with tokens). But couldn't/shouldn't the same code_challenge and code_challenge method be used for the refresh token grant authorization?

So my question is, how is grant_type=refresh_token actually secured when no secret and no code verifier is sent?

[iainmcgin]

PKCE and client secrets are essentially orthogonal; it is the decision of the authorization server whether it requires either for the purposes of authorization_code. It is our opinion (including @WilliamDenniss, one of the authors of the PKCE spec) that client secrets should be used for confidential clients like backends, and PKCE should be used for non-confidential clients like mobile and desktop apps. I can't think of any situations where both would be required, as if a client secret can be used safely, PKCE doesn't add any real value.

When PKCE is used, additional security measures for the refresh token should not be necessary, on the assumption that the client never sends the refresh token with anyone other than the IDP over a secure connection. The initial code exchange indirectly verified the identity of the app, so as long as the refresh token is never leaked, there is no problem.

Of course, refresh tokens have been known to leak from apps and servers with poor token hygiene. The forthcoming token binding specification should provide some additional safety here, where a key pair is bound to refresh / access tokens and usage of them requires the presence of that key pair, which is typically stored in hardware and can only be retrieved by fully compromising the client device rather than just sniffing traffic. Once this spec is closer to ratification, we will implement it in AppAuth and use it by default, similar to the way we currently enable PKCE by default.

[panva]

@iainmcgin this answers my questions, thank you.

[panva]

I may be out of line here to ask, but just quickly going through token binding it seems impossible to implement for AS behind TLS offloading servers/proxies such as nginx, caddy, heroku router or aws API gateway that does not have a direct connection with the client but rather with aforementioned proxy. Does that sound about right?

[iainmcgin]

I'm not sure, Dirk Balfanz would definitely be the person to ask - I'll see if I can solicit an opinion from him. It may require that the SSL Terminator is able to carry additional state through to the authorization server that it could use to validate the client.

[panva]

Why is the handling different for dynamic registration clients (sends client_secret using the right auth method) from static client, that do not support client secret and hence auth method at all?

Server side this creates quite a lot of hassle, making PKCE implementation not a simple extension, but rather a bunch of condition checks when client authentication can be deffered to pkce, even messier with refresh_token when there is no additional parameter in the body to detect used PKCE. Relaxing a bunch of MUSTs in OIDC etc.

Since auth method and secret handling is in the library already, why can't it just be configured for pre-existing clients? AppAuth-iOS only supports client_secret_post but at least it's consistent in using it when client secret is available.

[iainmcgin]

I may not be understanding your question correctly, but you can pass a ClientAuthentication instance to performTokenRequest to add headers and request parameters. This also means you can directly pass an instance of ClientSecretBasic or ClientSecretPost in order to do send a client secret from a static client. We really don't recommend or advertise this, as such secrets are easily extracted from APKs.

[iainmcgin]

Oh, if there are real inconsistencies in the behavior between iOS and Android variants of AppAuth, please do let myself or @WilliamDenniss know. The feature sets are drifting due to the different pace and focus of each library, but we do try to ensure they produce the same behavior for the features they have in common.

[panva]

Yes I can pass an instance to authentication request, i figured out as much. But then an auth option is missing for performActionWithFreshTokens.

option is to either pass a map with client_secret as its refreshTokenAdditionalParams or checking expiration manually and then doing performTokenRequest again with auth option.

What i'm trying to say, since token_endpoint_auth_method handling is already present for dynamic clients, why is it not at least an optional configuration for static clients.

[iainmcgin]

Ah, now that seems to be a genuine oversight. We should allow passing a ClientAuthentication instance to performActionWithFreshTokens - I'll open an issue for that.

[panva]

Well, that would for sure make it easier to explain to my RPs developers, thank you!

I have one additional quirk to pick up (with Android only since client_secret_basic is not supported by iOS), when client_secret_basic is used, should client_id still be submitted in the post body?

https://tools.ietf.org/html/rfc6749#section-2.3
The client MUST NOT use more than one authentication method in each request.

At least in my server implementation encountering an authorization header and client_id or secret in the body throws an error, enforcing this MUST NOT.

[iainmcgin]

The spec doesn't appear to be particularly clear on this point, though I think I agree with your conclusion that we should not be sending client_id if we are using a client secret.

From Section 4.1.3 on auth code exchange:

client_id: REQUIRED, if the client is not authenticating with the authorization server as described in Section 3.2.1.

... where Section 3.2.1 says:

A client MAY use the "client_id" request parameter to identify itself when sending requests to the token endpoint. In the "authorization_code" "grant_type" request to the token endpoint, an unauthenticated client MUST send its "client_id" to prevent itself from inadvertently accepting a code intended for a client with a different "client_id".

So, if we're currently sending client_id when client authentication is in use, I think that's a bug. If that's what you're seeing in testing, please open an issue for it.

[b---c]

Just wanted to note that there is some work being done in the IETF with respect to how token binding can work when an AS or other app are deployed behind TLS offloading servers/proxies. The draft is "HTTPS Token Binding with TLS Terminating Reverse Proxies", which "defines common HTTP header fields that enable a TLS terminating reverse proxy to convey information about the validated Token Binding Message sent by the client to a backend server, which enables that backend server to bind, or verify the binding of, cookies and other security tokens to the client's Token Binding key."