SIOP v2 -06 review (pre implementer's draft)

From Section 6:

(based on the openid: custom scheme)

1. Recommend “(based on custom URL schemes or claimed “https” URIs)”

2. This section seems like where we would describe response_mode=post

3. This section needs reference to security considerations, e.g. “this has reduced security properties see security considerations for more information”

‌

From Section 9.1:

4. The authorization_endpoint should be specified as “openid://”

‌

From Section 9.2:

5. This basically specifies a recovery for no jwks_uri, which means it goes from a must not specify to a must ignore. If we want to support specifying a jwks_uri, we MUST have another metadata attribute to differentiate SIOP from non-SIOP processing behavior (e.g. an i_am_siop metadata property)

6. Note that if we have an i_am_siop metadata property, we no arguably no longer require an i_am_siop claim in the id_token.

‌

Section 9.2.2:

7. Should reference the jwk-thumbprint urn scheme for the thumbprint rather than just saying to b64url the value?

‌

Section 10.2:

1. Signed requests can either be OpenID Federation entity statements or other URI which can be resolved to a set of keys.
1. That clarification is needed to indicate that when the client_id represents a federation entity statement, registration parameters MUST NOT be present.

‌

Section 10.2.2.1:

When Relying Party's client_id is expressed as an https URI

10. this also needs to state that the request must be signed for federation entity statements

‌

Section 10.2.3:

If the RP uses more than one Redirection URI, the redirect_uris parameter would be used to register them.

11. 10.2 forbids redirect_uris in the `registration` and `registration_uri`, so we should probably just eliminate this text. Multiple redirect_uris are still allowed when using entity statements.

12. Also, subject_syntax_types_supported is atypical from other registration parameters, where the RP specifies exactly what configuration it wants (see https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#client-metadata ). Is there a reason this shouldn’t be subject_syntax_type?

‌

Section Section 11:

1. Scope is listed as required, but should be forbidden in cross-device flows
1. The registration and registration_uri parameter descriptions should just point to 10.2
1. “To prevent duplication…” - remove registration(_uri) from list, since described in 10.2

16. I’d recommend for a future edit - remove request/request_uri from the parameters list, since they describe _how_ to send the other parameters. Instead, describe them in the subsequent text of the section, along with the list of query parameters which MUST still be sent with them on the uri.

17. I put up also for consideration whether we should recommend against registration_uri when request_uri is being used.

Section 11.1:

For the following text:

Note that the authentication request might only include request parameters and not be targeted to a particular authorization_endpoint, in which case, the user must use a particular Self-Issued OP application to scan the QR code with such request.

I’d recommend:

18. “Note that the authentication request URI may be insufficient to target the intended SIOP using a platform or third-party QR code scanning system. In this case, the user will need to use the Self-Issued OP application to scan the QR code”

‌

Section 12.2:

Note that HTTP error codes do not work in the cross-device Self-Issued OP flows.

19. I’d recommend something more like
”When response_mode=post is used to submit the response directly to the RP redirect URI, the HTTP response is not used to convey information to the SIOP about success or failure and MUST be ignored”

20. Also, recommend removing user_cencelled as an error code as it seems to be a non-SIOP-specific error, and we do not define any usage criteria.

Section 12.3:

21. I’d recommend just making i_am_siop required for all SIOP id_tokens, rather than special-casing it.

‌

Section 13, Bullet (1):

The Relying Party (RP) MUST determine whether the ID Token has been issued by the Self-Issued OP. When static Self-Issued OP Discovery metadata has been used, iss MUST be https://self-issued.me/v2. When Self-Issued OP Discovery metadata has been obtained dynamically, an additional i_am_siop Claim MUST be present in the ID Token and iss MUST exactly match the issuer identifier specified in the Self-Issued OP Discovery Metadata MUST be included in the ID Token as a way for the RP to determine if the, when dynamic Self-Issued OpenID Provider discovery has been used.

22. Recommend changing this to something like:

Verify the issuer of the id_token. If the OP metadata, whether negotiated out-of-band or retrieved dynamically, has associated signing keys (such as those referenced via jwks_uri, the OP is not Self-Issued and processing should proceed using the verification steps defined by OpenID Core.
If the ID Token is encrypted, decrypt it using the keys and algorithms that the Client specified during Registration that the Self-Issued OP was to use to encrypt the ID Token. If encryption was negotiated with the Self-Issued OP at registration time and the ID Token is not encrypted, the RP SHOULD reject it.
The Issuer Identifier for the Self-Issued OpenID Provider from the request MUST exactly match the value of the iss (issuer) Claim.

Verify the id_token has an i_am_siop claim with value true, indicating it is intended to be processed as a Self-Issued ID Token

Section 13, Bullet (2):

The RP MUST validate that the aud (audience) Claim contains the value of the client_id that the RP sent in the Authentication Request as an audience. When the request has been signed, the value might be an HTTPS URL, or a Decentralized Identifier.

23. There is no need for that second sentence. We also need to define how to process audience as an array. Recommend:

“The RP MUST validate that the aud (audience) Claim contains the value of the client_id that the RP sent in the Authentication Request as an audience. This MUST be specified either as a string, or as an array containing a single string value.”

24. Section 13, Bullet (3):

“The RP MUST identify which Subject Syntax Type is used based on the URI of the sub Claim. Valid values defined in this specification are urn:ietf:params:oauth:jwk-thumbprint for JWK Thumbprint Subject Syntax Type and did: for Decentralized Identifier Subject Syntax Type.”

… and types with a “did:” prefix, indicating particular Decentralized Identifier methods.

25. Section 13, Bullet (4 & 5):

The RP MUST validate the signature of the ID Token. When Subject Syntax Type is JWK Thumbprint, validation is done according to JWS [RFC7515] using the algorithm specified in the alg header parameter of the JOSE Header, using the key in the sub_jwk Claim. The key MUST be a bare key in JWK format (not an X.509 certificate value). The RP MUST validate that the algorithm is one of the allowed algorithms (as in id_token_signing_alg_values_supported). When Subject Syntax Type is Decentralized Identifier, validation is performed against the key obtained from a DID Document. DID Document MUST be obtained by resolving a Decentralized Identifier included in the sub Claim using DID Resolution as defined by a DID Method specification of the DID Method used. Since verificationMethod property in the DID Document may contain multiple public key sets, public key identified by a key identifier kid in a Header of a signed ID Token MUST be used to validate that ID Token.

The RP MUST validate the sub value. When Subject Syntax Type is JWK Thumbprint, the RP MUST validate that the sub Claim value equals the base64url encoded representation of the thumbprint of the key in the sub_jwk Claim, as specified in Section 12. When Subject Syntax Type is Decentralized Identifier, the RP MUST validate that the sub Claim value equals the id property in the DID Document.

26. The ordering should be resolve, check sub value, validate signature.

27. We should define the format of sub_jwk separately so we do not need to define it here. We should also define the DID mechanism separately so we do not need to define it here.

28. The MUST on checking id_token_signing_alg_values_supported is stronger than OpenID Core. Is there a reason for this?

29. With these changes in mind, including describing sub_jwk and DID requirements elsewhere

30. 4: The RP MUST resolve an appropriate public key for the ID token signature, based on the information conveyed via the sub claim.

31. 4.a: When the subject type is a JWK thumbprint, the public material MUST be extracted from the sub_jwk element. The sub_jwk MUST only contain the required values for a given kty. The subject identifier MUST be validated to be a thumbprint matching the specified key, using the mechanism defined in [RFC7638].

32. 4.b: When the subject type is a decentralized identifier, the RP MUST resolve the DID to a DID document. The id of the DID document MUST exactly match the sub value. A key from the DID document MUST be specified by exact match with the kid protected header of the id_token. The key specified MUST be appropriate for assertion signing.

33. 5. The Client MUST validate the signature of the ID Tokens using the resolved subject key, according to JWS [JWS] using the algorithm specified in the JWT alg Header Parameter. The Client MUST NOT use any keys associated with the issuer.

‌

Section 13.1:

34. There is no reason that the nonce claim should be validated in any different manner from same-device flows. This likely makes this section redundant.

Section 14.1:

35. Title should call out it is specifically the security considerations for cross-device usage

Comments (7)