[Openid-specs-ab] Native SSO spec feedback
panva.ip at gmail.com
Thu Feb 28 16:44:20 UTC 2019
You're welcome George, please find my feedback inline.
On Thu, 28 Feb 2019 at 16:56, George Fletcher <gffletch at aol.com> wrote:
> First, thank you very much for your feedback and I sincerely apologize for
> not getting back to you sooner!
> Comments inline.
> On 1/18/19 5:46 AM, Filip Skokan wrote:
> Hello George,
> I've re-read the document and updated my past feedback draft. Here goes
> *Section 3.2 *
> > and as such it is RECOMMENDED that the device secret be implemented as a
>> JWE encrypted for the
>> > Authorization Server.
> This recommendation suggests that the AS does not store the device secret
> state, why is JWE recommended over any opaque value representing the
> device secret state stored on AS side?
> We can relax this to allow for a client only vs server state solution. As
> to JWE, I often recommend it because it makes it easy to do key rotation
> and good security hygiene says keys should be rotated. That say, I agree
> that we don't have to be prescriptive around how in that the value is
> opaque to the client.
> > The client MAY provide the "device_secret" to the /token endpoint during
>> refresh token requests.
> Why? What's the AS supposed to do, refresh the device secret too? Validate
> it as well? Should the AS rotate the secret, is the client expected to
> update the shared storage with every new device secret it receives? Given
> the previous recommendation to use a JWE, how to handle rotation or
> revocation, if it's just an opaque value leading to a record on the side,
> should the used ones be marked as consumed and when re-used revoke the
> grant as a whole?
> So we have the requirement that the server should be able to update the
> device secret if needed. This provides a way for the device secret to be
> "rotated". As to issuing of access_tokens in the refresh_token grant flow,
> then I'd suggest that the device_secret represent the device to which the
> refresh_token was issued. This creates a tighter binding between the
> refresh_token and the device.
> Yes, if an "old" device_secret is used, I would reject the entire /token
> grant_type flow.
> Note that support for other grant_types other than refresh_token may
> require device_secret validation.
> Also, my expectation is that device_secrets contain a unique id (like a
> jti) that can be used on the server side for management, blacklisting, etc.
FS: If the intention is for the server to rotate it using its policies it
should be required to be present. e.g. if a refresh token is received at
the token endpoint which has the "device_sso" scope the device_secret is a
required parameter? Too strict? Dunno, but this way it looks like it's up
to the client to control the rotation by including it or not. That way, as
an implementer i'm forced to load the device_secret by either reference or
other means (decode/decrypt) and i can easily confirm the refresh token and
device secret belong to one another and that the device secret is not
expired / revoked or similar.
> *Section 3.3*
> Why would an app already in possession of a device_secret go through
> browser based code flow when it can do token exchange? If that one was
> rejected, then a browser based code flow? And the point of providing the
> previously rejected device_secret is?
> It's required so that the id_token can be issued with the device_secret
> hash (ds_hash). This allows the id_token, access_token and refresh_token to
> be bound to the device_secret. This constrains these tokens to that device.
FS: I'm not sure I follow. I know ID Token being returned from refresh
token grant or token exchange in this case is optional, but still, as a
client if I get a device secret from the shared storage my instinct
reaction is to do the token exchange, not to start a regular flow. It just
looks to me like this is out there without an explanation on WHY a client
would ever do a regular auth flow when it's already in possession of a
> *Section 3.4*
> > ds_hash
>> > The exact binding between the ds_hash and device_secret is not
>> specified by this profile. As this
>> > binding is managed solely by the Authorization Server, the AS can
>> choose how to protect the
>> > relationship between the id_token and device_secret.
> Why not use the same boilerplate used for other *_hash ID Token properties?
> We can. Partly because if the device_secret contains a randomly generated
> unique id (e.g. jti) then it might not really be required to hash the whole
> device secret. I should probably update the spec with this and am also
> interested in your thoughts.
FS: The device_secret being either opaque or JWT it's always unique, else
it doesn't serve it's purpose right. Ergo, i'd say the ds_hash is
calculated the same way other hash properties are.
>> > OPTIONAL. Device Secret hash value. Its value is the base64url encoding
>> of the left-most half of
>> > the hash of the octets of the ASCII representation of the device_secret
>> value, where the hash
>> > algorithm used is the hash algorithm used in the alg Header Parameter
>> of the ID Token's JOSE
>> > Header. For instance, if the alg is RS256, hash the device_secret value
>> with SHA-256, then take
>> > the left-most 128 bits and base64url encode them. The ds_hash value is
>> a case sensitive string.
> *Section 4.1*
>> > The client MUST use the HTTP Basic Authentication method from RFC
>> > 6749 to authenticate the request to the token endpoint.
> 1) client_secret_basic without client_secret makes little sense and in
> general an AS will issue a client secret when client's
> token_endpoint_auth_method is client secret based.
> So what is the preferred way for a PKCE client to present it's client_id?
> Given the current token exchange spec, I believe the only way is in the
> HTTP Authorization header. Happy for suggestions here.
FS: OIDC Core Section 9 gives a name to such auth method, it's "none" and
the client_id is provided in the body of the request and the Token Exchange
spec also mentions "Client authentication to the authorization server is
done using the normal mechanisms provided by OAuth 2.0.". Again this goes
to the next point, i'd just stick to the boilerplate and not force a client
to use a specific authentication scheme.
> 2) I wouldn't specify the required authentication method at the token
> endpoint, i'd say something along the lines of "the client authenticates
> using its registered token endpoint authentication method". Albeit this is
> most likely going to be "none" but we shouldn't rule out dynamically
> registered clients with secrets or private_key_jwt auth.
> This is a good point as I'd like to look at how this works with dynamic
> client registrations using private_key_jwt. I'll see about updating the
> text accordingly.
> Furthermore any AS implementation probably has the "client authentication"
> abstracted before any specific grant type processing and it would be a
> hassle to add extra auth processing based on the grant type.
> Fair point.
> *dtto in 4.2*, the example should be a non-normative example
> *Section 4.3*
> > If that session expires, all refresh_tokens associated with it MUST be
> Regardless of present offline_access scope and consent on the initial
> authorization request?
> Good point. I need to address the 'offline_access' scope scenario. I
> believe that even in an offline_access scenario, the server can invalidate
> the refresh_token so the concept needs to stay. Effectively, there needs to
> be a way for the id_token to reference something for the long-lived
> "session" such that if that session is revoked, the token exchange failed.
> session_id could possibly be replaced with refresh_token_id for the
> offline_access case. Thoughts?
FS: I probably skimmed though 4.3 the first time, because point of it 2
says the token endpoint should verify binding between an id token and a
device secret. Where did id_token come from? The client doesn't send one,
just sends the refresh token and possibly? the device secret.
> *Section 4.4*
> - expires_in should be RECOMMENDED or OPTIONAL to be in line with the
> rest of the specifications
> - refresh_token REQUIRED? The point is to obtain an ID and Access
> tokens, i'd expect issuing a refresh token is left to the AS policy
> Yes, we should leave it up to the AS. What I'd like to describe is that if
> the new client SHOULD receive a refresh_token for it's own flows, then the
> AS needs to provide one because the refresh_token is issued to the new
> client_id. That said, there could be some flows (though unlikely in a
> mobile environment) where the AS does NOT issue refresh_tokens so it
> shouldn't be REQUIRED.
> Example response has a few nits
> - has "Issued_token_type" property name (capital I)
> - should be marked as non-normative
> - token_type although case-insensitive value should be "Bearer" to be
> inline with the rest of the specs
> *Section 6.1. *
> `sid` would already be registered by both front and back-channel specs,
> wouldn't it?
> *6.2.1. is probably a leftover copy-paste*
> *Thank you!! This is very helpful!*
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Openid-specs-ab