TBD54566975 / known-customer-credential

3 stars 1 forks source link

Clarify use of JWT-Secured Authorization Request in KCC README #33

Open frankhinek opened 3 weeks ago

frankhinek commented 3 weeks ago

Context

@ethan-tbd and I were troubleshooting an issue with DIDPay throwing an exception during the KCC issuance sequence with the YC PFI.

DIDPay expects to receive a JWT-Secured Authorization Request (JAR):

  /// Parses a SiopV2AuthRequest from a URL query string.
  /// Example [here](https://openid.net/specs/openid-connect-self-issued-v2-1_0.html#section-5-5)
  static Future<SiopV2AuthRequest> fromUrlParams(String queryParams) async {
    final authRequestParams = Uri.splitQueryString(queryParams);
    if (!authRequestParams.containsKey('request')) {
      throw Exception();
    }

    // per https://www.rfc-editor.org/rfc/rfc9101.html
    final decodedRequestJwt = await Jwt.verify(authRequestParams['request']!);

    return SiopV2AuthRequest.fromMap(decodedRequestJwt.claims.misc!);
  }

Here's an example of what DIDPay expects to receive:

client_id=did:web:192.168.4.23%3A8892:ingress&request=eyJhbGciOiJFZERTQSIsImtpZCI6ImRpZDp3ZWI6MTkyLjE2OC40LjIzJTNBODg5MjppbmdyZXNzIzAifQ.eyJjbGllbnRfaWQiOiJkaWQ6d2ViOjE5Mi4xNjguNC4yMyUzQTg4OTI6aW5ncmVzcyIsImNsaWVudF9tZXRhZGF0YSI6IiIsImlzcyI6ImRpZDp3ZWI6MTkyLjE2OC40LjIzJTNBODg5MjppbmdyZXNzIiwibm9uY2UiOiIxNjhlNDQyODkzZjcyMzkwYmJlNDc3OGI5NDg0ODAxMSIsInJlc3BvbnNlX21vZGUiOiJkaXJlY3RfcG9zdCIsInJlc3BvbnNlX3R5cGUiOiJpZF90b2tlbiB2cF90b2tlbiIsInJlc3BvbnNlX3VyaSI6Imh0dHA6Ly8xOTIuMTY4LjQuMjM6ODg5Mi9pbmdyZXNzL2tjYyIsInNjb3BlIjoib3BlbmlkIn0.jdt-I6Doi4fziRcNCo3N1T0LONHjQFv7bccVUUrcKXzrXB_t9Ea0b30U9vINiiiYkL0rJO0NlZkXPgyIUOvGBQ

YC followed the kcc-prototype-exemplar so their endpoint is returning the JSON payload directly:

{
  "client_id": "did:dht:ho3axp5pgp4k8a7kqtb8knn5uaqwy9ghkm98wrytnh67bsn7ezry",
  "response_type": "id_token",
  "nonce": "494e2f95308aceadee91e7cf0d119b05",
  "scope": "openid",
  "response_mode": "direct_post",
  "response_uri": "https://pfi.yellowcard.engineering/auth-response",
  "client_metadata": {
    "subject_syntax_types_supported": "did:dht did:jwk did:web"
  }
}

Proposal

We should:

Unanswered Questions

  1. Should we make JWT-Secured Authorization Request (JAR) mandatory such that URL-encoded JSON auth requests are rejected? It seems like this is the most reasonable approach both to reduce variability / decisions for implementers and because accepting JSON means that the Auth Request from the RP (PFI Issuer in this case) is unsigned and could be tampered with. FWIW, although we don't use this spec due to its lack of support for DIDs, HAIP also mandates usage of RFC 9101 JARs for auth requests.

  2. Should we enable support for returning the authorization request by reference in addition to (or instead of) by value? The benefit to doing this is that it makes it feasible (fewer bytes to QR encode a URI vs. signed JWT) to present the auth request as a QR code, which could then be scanned by a mobile device in the case that the wallet that contains the user's DID+keys+VCs are a different device from the one they are using to interact with a PFI. In other words, should we return an authorization request that looks like this:

    client_id=https%3A%2F%2Fclient.example.org%2Fcb
    &request_uri=https%3A%2F%2Fclient.example.org%2Frequest%2FGkurKxf5T0Y-mnPFCHqWOMiZi4VS138cQO_V7PZHAdM
EbonyLouis commented 3 weeks ago

I love that we have YC to go through this flow, which in turn provokes these important changes. My thoughts to your unanswered questions:

  1. Yes, we should for exactly the reasons you mentioned. We want to avoid implementation issues and reduce the number of decisions implementers need to make to ensure this is as simple for them as possible. Additionally, we should be showcasing the safest way of doing things. Since the current method of sending the Auth Request as JSON means it can be tampered with, we shouldn't be advising this approach.

  2. I think we should support the authorization request by reference in addition to, not instead of, by value. That way we have that flexibility there for them to decide and I don't see the harm in having both approaches.

Once we all reach a conclusion on this, I'll be sure to update the KCC guides.

tomdaffurn commented 3 weeks ago
  1. I agree. The raw JSON option is equivalent to request by value, but without signing.

  2. I don't see much of a use case for request-by-reference, because the wallet has just made a GET request and expects a response. However, I'm comfortable adding it in addition to by-value. In general I think we shouldn't restrict what SIOPv2 allows without good reason.

KendallWeihe commented 3 weeks ago

Good catch, I glossed over this in the first go at kcc-prototype-exemplar.

  1. Yes.
  2. The example shared is ~650 bytes, and the lower bound for maximum QR Code allowance is ~1200 bytes, so we're already pushing > 50% already. However, everything within the request is constrained, except for response_uri which I guess I could imagine a situation, especially in the enterprise world, where that alone could blow up to something like 1k bytes. So yeah, in favor of adding in-addition-to; definitely a case of solution without a known problem but existing use case > 50% seems a reasonable threshold to get in front of.
decentralgabe commented 3 weeks ago

given that HAIP requires request_uri I believe we should as well - let's remove optionality where we can

tomdaffurn commented 3 weeks ago

HAIP is not a spec we've used in KCC so far. We aren't doing most of what it requires: SD-JWT VC, authorization code flow, sender-constrained tokens, S256 code challenge method. If we're going to take it's recommendations then it should be all of them.

I'm fine with using just 1 of request or request_uri though. I think request_uri is the more flexible of the 2

decentralgabe commented 3 weeks ago

If we're going to take it's recommendations then it should be all of them.

We should understand the choices they've made and why, and apply them where they make sense.

A few clear security benefits for using request_uri:

  1. We reduce the risk of an unsigned request by value (yes, we can prohibit this, but still a risk).

  2. Even if the request parameter is signed, the authorization request parameters are still exposed in the URL when using request. This could lead to sensitive info like client_id leaking to the User Agent -- browser histories, server logs, referrer headers, etc. request_uri keeps the request parameters completely out of URLs.

  3. The HAIP spec requires the use of Pushed Authorization Requests (PAR), which works well with request_uri. The client can directly push the request object to the Auth Server using PAR, and then use the returned request_uri in the authorization request.

I wasn't too familiar with PAR, but I believe it could be a good security addition to the KCC spec. This would improve the security of Step 3 in the Initiate Application Flow, so instead of

PFI URI-encodes SIOPv2 Authorization Request and returns in HTTP response

We could introduce a PAR endpoint which surfaces a request_uri (instead of the current response_uri) for the client to send the SIOPv2 Auth Response to. This would make sure the request parameters are never exposed to any intermediaries, since they're removed from the URI itself. Even if the request_uri is intercepted it is meaningless without access to the PAR endpoint.

So tl;dr -- yes let's go with request_uri, and let's consider using PAR.

frankhinek commented 2 weeks ago

I wasn't too familiar with PAR, but I believe it could be a good security addition to the KCC spec. This would improve the security of Step 3 in the Initiate Application Flow, so instead of

PFI URI-encodes SIOPv2 Authorization Request and returns in HTTP response

We could introduce a PAR endpoint which surfaces a request_uri (instead of the current response_uri) for the client to send the SIOPv2 Auth Response to. This would make sure the request parameters are never exposed to any intermediaries, since they're removed from the URI itself. Even if the request_uri is intercepted it is meaningless without access to the PAR endpoint.

So tl;dr -- yes let's go with request_uri, and let's consider using PAR.

@decentralgabe I might have misread the spec, but I believe the issue is that this PAR cannot be easily combined with SIOPv2, particularly in cross-device scenarios.

As noted in the diagram below (excerpt from KCC README) the RP is the PFI and the IDP/Authorization Server is the mobile app. In order to use PAR, the mobile app would need a publicly accessible endpoint for the PFI to push its authorization request to. I've searched and thus far I can't find any good example of SIOPv2 combined with PAR when the IDP/AS doesn't have publicly accessible endpoints.

I'm

sequenceDiagram
autonumber

participant W as Webview
participant D as Mobile App
participant P as PFI

D->>+P: GET
P->>P: Construct SIOPv2 Authorization Request
P-->>-D: SIOPv2 Authorization Request
D->>D: Construct SIOPv2 Authorization Response
D->>+P: SIOPv2 Authorization Response
P->>P: Construct IDV Request
P-->>-D: IDV Request
D->>D: Verify IDV Request
D->>W: Load URL in IDV Request
decentralgabe commented 2 weeks ago

my mistake - let's ignore my suggestion for PAR

EbonyLouis commented 1 week ago

JAR update PR for KCC-wallet and KCC-issuer guides. I have a question on there for y'all -thanks!