openid / OpenID4VCI

66 stars 19 forks source link

Encrypted responses in Batch and Deferred Endpoints #286

Open vafeini opened 6 months ago

vafeini commented 6 months ago

I am trying to understand how the responses of Batch and Deferred Endpoints should work in case encryption is required

Batch Endpoint Spec defines that a batch credential response is (besides c_nonce info) a list of single credential responses (credential_responses attribute).

What happens though if the individual credential requests (or even worse only some of them) require encrypted responses (credential_response_encryption included in the requests)?

One could assume that since batch credential response is an accumulator of multiple single credential responses it could be as follows

{
    "credential_responses": [
        "eyJraWQiOiI2.....retJxnnPyaeUzNbwPZZAiA", // JWE encrypted response of a request X that demands encrypted response
        "OiI2NjBmNjQx.....ahTLjjEidA_V6g2u-ppGg", // JWE encrypted response of a request Y that demands encrypted response
        {                                                               
        "credential": "LUpixVCWJk0eOt4CXQe1NXK....WZwmhmn9OQp6YxX0a2L", // Non encrypted response of request Z that does not demand encryption
    }                                                               
    ],
    "c_nonce": "ERE%@^TGWYEYWEY",
    "c_nonce_expires_in": 34
}

This seems thought rather complicated. Why need separate encryption per individual response instead of having the whole batch credential response encrypted.

In the case that the whole batch credential response should be encrypted, we are missing from batch credetial request a way to define the encryption information. Property credential_response_encryption can be used in batch credential requests as in single ones:

{
   "credential_requests":[ ... ],
   "credential_response_encryption": {
        "jwk": { ... },
        "alg": "ES256",
        "enc": "A128GCM"
   }
}

Deferred Endpoint In Section 9.2 spec defines that

"Credential Response MUST be sent using the application/json media type"

This does not align with the case where the corresponding credential request defines that encrypted response is needed. In this case response media type should be application/jwt. Section 9.2 needs to be enriched to reflect how case of encrypted responses should be handled.

babisRoutis commented 6 months ago

For batch request, I think that credential_response_encryption needs to be added as a top-level optional attribute and in addition require that individual credential_requests must not be set credential_response_encryption. That way, it would be clear that response encryption is about batch response as a whole.

babisRoutis commented 6 months ago

For deferred issuance, there is the obvious problem that a wallet can receive a transaction_id with an encrypted response but it cannot exchange it (via deferred endpoint) to a credential nested in an encrypted deferred response. This is odd.

Should a request to deferred endpoint contain also a credential_response_encryption ?

TakahikoKawasaki commented 6 months ago

FYI: It may not be specified in the OID4VCI specification, but as a natural technical consequence for me, I thought that the deferred credential endpoint should be able to accept the credential_response_encryption parameter as a top-level property and return application/jwt when the response is encrypted. Our sample implementation of the deferred credential endpoint returns application/jwt like below (from java-oauth-server: DeferredCredentialEndpoint.java#L159).

case OK_JWT:
    return ResponseUtil.okJwt(content, headers);
babisRoutis commented 6 months ago

FYI: It may not be specified in the OID4VCI specification, but as a natural technical consequence for me, I thought that the deferred credential endpoint should be able to accept the credential_response_encryption parameter as a top-level property and return application/jwt when the response is encrypted. Our sample implementation of the deferred credential endpoint returns application/jwt like below (from java-oauth-server: DeferredCredentialEndpoint.java#L159).

case OK_JWT:
    return ResponseUtil.okJwt(content, headers);

Dear @TakahikoKawasaki

I totally agree about introducing a credential_response_encryption to the deferred request. It makes no sense to be able to get a transaction_id encrypted but not the issued (deferred) credential. I think though that this should be explicitly stated in the specification - I don't believe that it's a natural consequence.

For instance, one may think that the issuer should "remember" that the initial request (to the credential endpoint) had a credential_response_encryption parameter and accordingly adjust the deferred response.

To avoid such ambiguities, I think that:

In 11.2.3

credential_response_encryption: OPTIONAL. Object containing information about whether the Credential Issuer supports encryption of the Credential and Batch Credential Response on top of TLS.

Should be updated to add Deferred Credential Response

In 8.1

The credential_response_encryption should be added as an optional top-level parameter.

credential_requests: REQUIRED. Array that contains Credential Request objects, as defined in Section 7.2.

Also, the above text should be updated to exclude the credential_response_encryption parameter from each item of the credential_requests

In 8.2

The description should be updated to present the application/jwt case.

In 9.1

The credential_response_encryption should be added as an optional parameter.

In 9.2

The Deferred Credential Response uses the credential parameter as defined in Section 7.3.The Deferred Credential Response MUST be sent using the application/json media type

Description should document also the encrypted case (using application/jwt)

In 9.3

It is unclear to me, what the error response should be in case the wallet requests credential_response_encryption.

babisRoutis commented 5 months ago

I would appreciate if there could be some response from the specification authors

The issue, to my understanding, can be summarized to whether

jogu commented 5 months ago

We discussed on yesterday's WG call; though people probably need more time to think about it.

A single top-level attribute credential_response_encryption is expected to be send with request to the batch endpoint

There seemed to be agreement on this and it was proposed to raise a PR to make this clear in the specification; @c2bo agreed to raise that PR

A credential_response_encryption is expected to be send with a request to the deferred endpoint

We weren't clear on this. It felt unnecessary. The deferred credential endpoint currently only receives transaction_id, and it is assumed that all other necessary information was passed to the credential/batch credential endpoint originally and has been stored against the transaction_id. It was not obvious to us why credential_response_encryption would be an exception to that mechanism.

It was also further noted that it's not clear that encryption in this way (over an already encrypted transport) is actually adding any value as the keys aren't attested in anyway, though that applies equally to the encryption support already in the spec at the credential endpoint.

Sakurann commented 3 months ago

please comment on the path forward regarding the deferred endpoint .

babisRoutis commented 3 months ago

please comment on the path forward regarding the deferred endpoint .

Dear @Sakurann

If adding an credential_response_encryption attribute feels unnecessary, then there is the need for both the issuer & the wallet to "remember" whether the initial request included this and adjust accordingly deferred response creation and processing, respectively, (as per @jogu last comment).

I think that spec should explicitly describe this to the deferred response section. To my understanding, deferred response should be encrypted in case

In case of an "invalid_transaction_id" error, the deferred response cannot be encrypted, simply because issuer cannot trace back to the initial request.