openid / OpenID4VP

52 stars 18 forks source link

what metadata goes into client_metadata parameter? #17

Closed OIDF-automation closed 5 days ago

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/1814

Original Reporter: KristinaYasuda

currently we just say that any client metadata can go into a client_metadata parameters, but I think parameters like redirect_uris, response_types, grant_types should be prohibited to be included.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: tlodderstedt

The parameters you listed don’t make sense as they are typically used by the AS to check whether the actual request parameters match. It might be worthwhile to go through the list of metadata parameters and whether there are more constraints we need to defined. For example, does it make sense to pass multiple kyes?

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: dwc8

I think if we specify the trust model that we are using for both issuing and presenting VCs then it will become much clearer which parameters are trustworthy and which are not - and the latter can therefore introduce vulnerabilities.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

I made the first pass to what I think should be allowed/prohibited/conditional:

For both OID4VP and SIOPv2

Only SIOPv2

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

default_acr_values
this may depends by each presented VC. Let’s suppose that we may have different credentials of the same type but with different LoA (eg: different european member state issues the same document with different LoA). Given LoA as requirement, it should be in the presentation_definition

another chapter should be open for the presentation_definition that to date we can submit only in the authorization request, while we really need it also in the metadata, allowing a RP to use the one it gives in its metadata, without explicitly specify this in the auzh request. WDYT?

Another capther would be for aliased presentation definition in scopes. The metadata would define the presentation_definition_scope_alias.
this would allow to define presentation defintion and scope aliases in the metadata, and then using the aliases in the scope parameter of the request

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

What about the proposal below?

Please note that:

  1. It’s a federation entity configuration, where the metadata type defines the entity type we’re dealing with: Verifier or wallet relying party.
  2. in my team there is a certain sensitivity not to define this entity as a relying party but rather to specialize it with the name of wallet verifier, this to not collide with metadata type openid_relying_party , an alternative could be wallet_relying_party or any other suggested in this WG.
  3. redirect_uris and request_uris are required for the security of the solution, since all the resources listed there should be attested by a trusted third party. Why these are marked as **Prohibited** by Kristina?
  4. presentation_definitions is still unconventional and needs discussions. The requirement to have it in the metadata is the same of the previous point.
  5. PE with sd-jwt input descriptor needs to be discussed here: https://bitbucket.org/openid/connect/issues/1580/openid4vps-add-sd-jwt-example
{
    "alg": "RS256",
    "kid": "2HnoFS3YnC9tjiCaivhWLVUJ3AxwGGz_98uRFaqMEEs",
    "typ": "entity-statement+jwt"
}
.
{
    "exp": 1649590602,
    "iat": 1649417862,
    "iss": "https://rp.example.it/",
    "sub": "https://rp.example.it/",
    "jwks": {
        "keys": [
            {
                "kty": "RSA",
                "n": "5s4qi …",
                "e": "AQAB",
                "kid": "2HnoFS3YnC9tjiCaivhWLVUJ3AxwGGz_98uRFaqMEEs"
            }
        ]
    },
    "metadata": {
        "wallet_relying_party": {
            "application_type": "web",
            "client_id": "https://rp.example.it",
            "client_name": "Name of an example organization",
            "jwks": {
                "keys": [
                    {
                        "kty": "RSA",
                        "use": "sig",
                        "n": "1Ta-sE …",
                        "e": "AQAB",
                        "kid": "YhNFS3YnC9tjiCaivhWLVUJ3AxwGGz_98uRFaqMEEs",
                        "x5c": [ ... ]
                    }
                ]
            },

            "contacts": [
                "ops@rp.example.it"
            ],

            "request_uris": [
                "https://rp.example.it/authz"
            ],
            "redirect_uris": [
                "https://rp.example.it/cb"
            ],

            "default_acr_values": [
                "https://www.spid.gov.it/SpidL2",
                "https://www.spid.gov.it/SpidL3"
            ],

            # not sure that's really required anymore here ...
            # "client_metadata": {
              "vp_formats": {
                 "jwt_vp_json": {
                    "alg": [
                       "EdDSA",
                       "ES256K"
                    ]
                }
              },
              "presentation_definition": [
                  {
                    "id": "SD-JWT-sample",
                    "name": "Person Identification Data",
                    "purpose": "User authentication",
                    "input_descriptors": [
                        {
                            "id": "sd-jwt",
                            "format": {
                                "jwt": {
                                    "alg": [
                                        "RS256",
                                        "ES256",
                                        "PS256"
                                    ]
                                },
                                "constraints": {
                                    "limit_disclosure": "required",
                                    "fields": [
                                        {
                                            "path": [
                                                "$.sd-jwt.type"
                                            ],
                                            "filter": {
                                                "type": "string",
                                                "const": "PersonIdentificationData"
                                            }
                                        },
                                        {
                                            "path": [
                                                "$.sd-jwt.cnf"
                                            ],
                                            "filter": {
                                                "type": "object",
                                            }
                                        },
                                        {
                                            "path": [
                                                "$.sd-jwt.family_name"
                                            ],
                                            "intent_to_retain": "true"
                                        },
                                        {
                                            "path": [
                                                "$.sd-jwt.given_name"
                                            ],
                                            "intent_to_retain": "true"
                                        },
                                        {
                                            "path": [
                                                "$.sd-jwt.unique_id"
                                            ],
                                            "intent_to_retain": "true"
                                        }
                                    ]
                                }
                            }
                        }
                    ]
                  },
                  {
                    "id": "mDL-sample-req",
                    "input_descriptors": [
                        {
                            "id": "mDL",
                            "format": {
                                "mso_mdoc": {
                                    "alg": [
                                        "EdDSA",
                                        "ES256"
                                    ]
                                },
                                "constraints": {
                                    "limit_disclosure": "required",
                                    "fields": [
                                        {
                                            "path": [
                                                "$.mdoc.doctype"
                                            ],
                                            "filter": {
                                                "type": "string",
                                                "const": "org.iso.18013.5.1.mDL"
                                            }
                                        },
                                        {
                                            "path": [
                                                "$.mdoc.namespace"
                                            ],
                                            "filter": {
                                                "type": "string",
                                                "const": "org.iso.18013.5.1"
                                            }
                                        },
                                        {
                                            "path": [
                                                "$.mdoc.family_name"
                                            ],
                                            "intent_to_retain": "false"
                                        },
                                        {
                                            "path": [
                                                "$.mdoc.portrait"
                                            ],
                                            "intent_to_retain": "false"
                                        },
                                        {
                                            "path": [
                                                "$.mdoc.driving_privileges"
                                            ],
                                            "intent_to_retain": "false"
                                        }
                                    ]
                                }
                            }
                        }
                    ]
                }
            ],
            "default_max_age": 1111,

            # SIOPv2 related
            "subject_type": "pairwise",
            "require_auth_time": true,
            "id_token_signed_response_alg": [
                "RS256",
                "ES256",
                "PS256"
            ],
            "id_token_encrypted_response_alg": [
                "RSA-OAEP",
                "RSA-OAEP-256"
            ],
            "id_token_encrypted_response_enc": [
                "A128CBC-HS256",
                "A192CBC-HS384",
                "A256CBC-HS512",
                "A128GCM",
                "A192GCM",
                "A256GCM"
            ],
        },
        "federation_entity": {
            "federation_resolve_endpoint": "https://rp.example.it/resolve/",
            "organization_name": "OpenID Wallet Verifier example",
            "homepage_uri": "https://rp.example.it/home",
            "policy_uri": "https://rp.example.it/policy",
            "logo_uri": "https://rp.example.it/static/logo.svg",
            "contacts": [
               "tech@example.it"
             ]
        }
    },
    "authority_hints": [
        "https://registry.eudi-wallet.example.it/"
    ]
  }
}

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

instead of presentation_definitions what about having in the wallet RP metadata presentation_definition_supported ?

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

default_acr_values

this may depends by each presented VC.

Does acr apply to VCs? verifier will ask for a specific value and wallet will return? will verifier trust wallet’s acr value?

another chapter should be open for the presentation_definition that to date we can submit only in the authorization request, while we really need it also in the metadata,

Right now, if you want to publish a static presentation_definition, you would use presentation_definition_uri.

Another capther would be for aliased presentation definition in scopes. The metadata would define the presentation_definition_scope_alias.

interesting.. might be worth considering..

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

Does acr apply to VCs?

good one, it should, since there are some legacy credentials with LoA substantial and not High

Right now, if you want to publish a static presentation_definition, you would use presentation_definition_uri.

good one but unfortunately this doesn’t fit the requirements that an accreditation body have in validating RP metadata.
I assume, as a requirement in a federative context, that the metadata should be signed by a Trusted Third PArty or at least a verifiable attestation is issued containing the relevant metadata properties and policy

I’m asking to have the presentation definition in the metadata and optionally in the request, since at this moment we just have these in the request and this doesn’t fit a federative approach

presentation_definition_scope_alias

regarding this one, there the will in my context to use scope anyway and have aliased presentation definitions as scope. In the discussion also Takahiko raised his concerns, I agree that’s an hot topic on the table that requires some effort. I’m on it, let’s try to discuss and find the way to go together

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

during the SIOP call we discussed there are three potential actions here:

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

parameter availability motivation
application_type REQUIRED we have the proximity and remote flow. If the RP is remote this MUST set to web. if proximity .. proposal required here.
client_id REQUIRED to uniquely identify the RP
client_name REQUIRED for UX on the consent page
jwks CONDITONAL REQUIRED if jwks_uri or signed_jwks_uri are not present.
contacts RECOMMENDED for UX and the right to establish a direct contact to the assistance or to an organization technical or administartive representative
request_uri note: there are some cases where for security reason this endpoint could be extended with session unique identifiers needed in the cross device flows. for this reason we should mention that the FQDN of the request_uri should be the equal to RP’s but we cannot mandate a static definition of it in the metadata CONDITIONAL REQUIRED if application_type is web and when the RP supports that. If the RP request a uri that is not defined in this list or this parameter is not present, the wallet instance should reject the request. The presence of this parameter allows an accreditation body to apply policies on the allowed request_uris.
default_max_age OPTIONAL Default Maximum Authentication Age. Specifies that the End-User MUST be actively authenticated if the End-User was authenticated longer ago than the specified number of seconds. The max_age request parameter overrides this default value. If omitted, no default Maximum Authentication Age is specified. See Section 2 of OpenID Connect Dynamic Client Registration 1.0
subject_type OPTIONAL SIOPv2 related
require_auth_time OPTIONAL SIOPv2 related
id_token_signed_response_alg RECOMMENDED SIOPv2 related
id_token_encrypted_response_alg RECOMMENDED SIOPv2 related
id_token_encrypted_response_enc RECOMMENDED SIOPv2 related
authorization_signed_response_alg OPTIONAL JARM related
authorization_encrypted_response_alg OPTIONAL JARM related
authorization_encrypted_response_enc OPTIONAL JARM related
attested_security_context_supported_values OPTIONAL non-IANA. In harmonization with attested_security_context represented here https://vcstuff.github.io/draft-looker-oauth-attestation-based-client-auth/draft-looker-oauth-attestation-based-client-auth.html#appendix-A.1; and defined in the Italian EUDI Wallet Solution, for both wallet provider and wallet relying party here: https://github.com/italia/eudi-wallet-it-docs/blob/versione-corrente/docs/en/wallet-solution.rst#payload-wallet_provider; and for wallet instance attestation here: https://github.com/italia/eudi-wallet-it-docs/blob/versione-corrente/docs/en/wallet-instance-attestation.rst#payload-1
redirect_uris CONDITIONAL the same motivation given for request_uris
default_acr_values OPTIONAL There are some cases where the RP made known the supported LoA. If not respected by the presenter, the presentation should be rejected.
vp_formats REQUIRED this makes known to the wallet instance if it has the capabilities to present a digital credential to the RP
presentation_definition REQUIRED this makes known which are the presentation definitions that the RP may request. Having it in the metadata this allows an accreditation body to configure policy on the metadata, or it issues verifiable_attestations with the entire list or a subset of the presentation definition supported. There are some cases where a wallet solution discover in the entire network (trusted Lists, Federation …) which are the RP and then their metadata to survey the technical capabilities required for the best interop. There are some cases where the request contains just the scope parameter (or an alternative presentation_definitions_ids parameter) containing the presentation definition ids, made available in the rp’s metadata. IF this is true, then we should define in OpenID4VP that the presentation_definition id MUST NOT contain space characters in theis ID values.
default_max_age OPTIONAL Default Maximum Authentication Age. Specifies that the End-User MUST be actively authenticated if the End-User was authenticated longer ago than the specified number of seconds. The max_age request parameter overrides this default value. If omitted, no default Maximum Authentication Age is specified. See Section 2 of OpenID Connect Dynamic Client Registration 1.0

paulbastian commented 10 months ago

@Sakurann @peppelinux @tlodderstedt I would like to pick up this topic again.

If I compare this with what we have in OpenID4VCI, I see a disparity.

Developers at Bundesdruckerei stumbled over this and were confused as there is little guidance how DCR client metadata fits into OpenID4VP. I would like to see more normative parameters inside the OpenID4VP spec.

peppelinux commented 10 months ago

@paulbastian this issue is related to anything assuming the role of client, then it would be the Wallet Instance in the issuance phase, or the RP in the presetation phase

for the italian model we have defined at least these base claims

key value
authorization_endpoint URL of the SIOPv2 Authorization Endpoint.
response_types_supported JSON array containing a list of the OAuth 2.0 response_type values.
response_modes_supported JSON array containing a list of the OAuth 2.0 "response_mode" values that this authorization server supports. RFC 8414 section 2
vp_formats_supported JSON object with name/value pairs, identifying a Credential format supported by the Wallet.
request_object_signing_alg_values_supported JSON array containing a list of the JWS signing algorithms (alg values) supported.
presentation_definition_uri_supported Boolean value specifying whether the Wallet Instance supports the transfer of presentation_definition by reference.
Sakurann commented 7 months ago

@peppelinux, the metadata parameters you quote in the table above are AS metadata parameters, not client metadata parameters..

peppelinux commented 7 months ago

@Sakurann as mentioned in the previous issues intro text (and sorry for the out-of-topic) those are the claims related to the Wallet Instance Technical Capabilities. It was included here according to a chat/meet as a reference, to be not confused with the purpose of this issue related to the RP's metadata

Sakurann commented 6 months ago

138 is related.

Sakurann commented 5 months ago

bumping this up as there have been additional inquiries what can client_metadata be used for

Sakurann commented 4 months ago

WG call

jogu commented 4 months ago

I think it would be worth asking if we actually need client_metadata_uri. Given we have federation entity_ids and a suggestion for a client_id_scheme that includes pulling metadata from a .well-known and the ability to pass the entire request via a uri I'm not sure I see a use case for client_metadata_uri. (Removing client_metadata_uri would make figuring out which meta data can be used / in which cases simpler I think, hence why I'm bringing it up on this issue.)

David-Chadwick commented 4 months ago

what if jwks (or any other parameter) is duplicative between client_metadata and whatever wallet obtained using trust chain. (@awoie )

The value from the trust chain should take precedence, because the wallet is dependent upon the trust infrastructure in order to retrieve reliable information. The metadata should not take precedence, because a dishonest RP may send "false" metadata in order to trick the wallet into believing untrustworthy information.

David-Chadwick commented 4 months ago

client_id and client_name are both mandatory, but what if they are inconsistent? E.g. the client_id is mallory but the client_name is Mr Nice Guy. The user only sees the latter on the consent page. The client_name should be optional, and it should be replaceable by whatever the trust infrastructure returns for the client_name of the client_id. In this way a lowly trusted client_id wont be able to present the name of a highly trusted client_name to the user.

tlodderstedt commented 4 months ago

This is the list of allowed client metadata parameters we agreed on in the WG call on the 21st of May:

Open: define the list in the spec? OIDF cannot establish a new IANA registry. Leaning towards defining in the spec. Open: use different parameter name for passing the ephm encryption key? Leaning towards changing the name. Open: does it make a difference whether the request is signed or not? Leaning towards there is a difference for some parameters, e.g. jwks

bc-pi commented 4 months ago

Open: use different parameter name for passing the ephm encryption key? Leaning towards changing the name.

Note that the existing section on using JARM will need to be updated to account for a name change.
https://openid.net/specs/openid-4-verifiable-presentations-1_0-20.html#section-6.3-10 has text such as:

"The key material used for encryption and signing SHOULD be determined using existing metadata mechanisms.

To obtain Verifier's public key for the input to the key agreement to encrypt the Authorization Response, the Wallet MUST use jwks or jwks_uri claim within the client_metadata request parameter,... "

awoie commented 4 months ago

jwks is currently used in two different ways in the context of the client_metadata parameter:

  1. conveying ephemeral keys used for DH to derive a JARM encryption key
  2. conveying ephemeral keys for DH to derive MAC keys

Both 1. and 2. typically require the wallet to do DH key agreement and using key derivation function, e.g., HKDF/ConcatKDF. There are specifications that may reuse the same key for 1. and 2., e.g., ISO 18013-5 (although they allow for dedicated keys as well). I don't know if there is a specific problem with reusing the same key for 1. and 2. (same verifier key but with different wallet keys -> one is ephemeral, the other one is the key the credential is bound to; verifier key is ephemeral per transaction) but I'd prefer having it clearly separated by having distinct parameters.

Today, one option how keys for 1. can be distinguished from keys for 2. within jwks is to set the JWK alg value to ECDH-ES for encryption/JARM.

Note that key usages for 1. and 2. would set the JWK use value to enc as per JWK:

The "enc" value is also to be used for public keys used for key agreement operations.

Also note that for 1., the key is not directly used as a content encryption key, and for 2., the key is not directly used as a MAC key because both keys are used for DH to derive other keys. This is why, IMHO, I think key_ops = encryption is not appropriate and it should probably be deriveKey in both cases, but I would be curious what other implementations would normally do?

Option A: This just shows that it would be better to have distinct metadata parameters to resolve potential ambiguity which is why I'm in favor of having distinct metadata parameters. For example, encryption_epks/jwks, mac_epks/jwks (names to be bikeshedded). Perhaps the mac_epks/jwks one could be format specific since not all VPs in all formats will require this.

Option B: Alternatively, we could define a new use/key_ops value specifically for DH-negotiated MAC operations but this is not my preference.

Sakurann commented 4 months ago

also a note from PR #165. we should clarify whether client_metadata parameters can be used with any client_id_schemes or not. originally, there was a text saying that client_metadata should be not be used with pre-registered clients but in the course of the discussion in the PR and issue #96 it was brought up that ephemeral encryption key for example can be passed in the client metadata even for preregistered clients.

peppelinux commented 4 months ago

@tlodderstedt

I believe that it is important to also include presentation definition in cases where the RP does not necessarily have to modify the set of credentials/attributes required for each request, and that in addition these can be enabled by a federation authority through verifiable metadata.

I would not underestimate also contacts and client_id

David-Chadwick commented 4 months ago

For many RP services the presentation definition will be fixed regardless of the transaction e.g. consider buying anything from eBay or Amazon, regardless of the purchase the requested credentials are always the same. Thus the presentation definition should be verifiable by the trust infrastructure.

peppelinux commented 3 months ago

Here is a proposal for the creation of a new parameter name designed to address the federative challenges introduced by some trust frameworks in implementations:

https://github.com/openid/OpenID4VP/issues/189

peppelinux commented 3 months ago
  • jwks

  • vp_formats

  • authorization_signed_response_alg

  • authorization_encrypted_response_alg

  • authorization_encrypted_response_enc

I would not understimate the importance of client_name for a good UX during the Consent/SD

David-Chadwick commented 3 months ago

I would not understimate the importance of client_name for a good UX during the Consent/SD

As I mentioned earlier, there needs to be some way of validating the client_name against the client_id, otherwise a bad RP could use a non-matching client_id and client_name.

peppelinux commented 3 months ago

I would not understimate the importance of client_name for a good UX during the Consent/SD

As I mentioned earlier, there needs to be some way of validating the client_name against the client_id, otherwise a bad RP could use a non-matching client_id and client_name.

The client_id is often an HTTPS URL value or an opaque string, while the client_name provides a human-readable representation of the RP's name. From my perspective, these are different parameters with distinct purposes, and their values do not necessarily need to match.

David-Chadwick commented 3 months ago

I think you are missing the point. We can have a client_id https://x.com with a matching client name of My Company, and another client_id of https://y.com with a matching client name of Your Company. There must be some way to stop the latter from saying client_id https://y.com with client name of My Company since it the client name that is shown to the user, but the client_id is checked for trustworthiness by the wallet. Thus the trust infrastructure should return the valid client name for the trusted client_id

peppelinux commented 3 months ago

@David-Chadwick yes I was missing the point, got it.

openid federation uses metadata and or metadata_policy in the subordinate statements to fix the leaves metadata to the only possible and allowed data.

The point you mentioned can be formalized as a functional requirement of the trust infrastructure I agree.

peppelinux commented 3 months ago

here my current proposal, where @tlodderstedt 's proposal is included as well

Claim Description Reference
client_id It MUST contain an HTTPS URL that uniquely identifies the RP. RFC 7591 Section 3.2.1 and OpenID Connect Dynamic Client Registration 1.0 Section 3.2
client_name Human-readable string name of the RP. RFC 7591 Section 2
authorization_signed_response_alg String representing the JWS alg algorithm that MUST be used for signing authorization responses. The algorithm none MUST NOT be used. RFC 7515 and oauth-v2-jarm-03 Section 3
authorization_encrypted_response_alg
authorization_encrypted_response_enc
vp_formats JSON object defining the formats and proof types of Verifiable Presentations and Verifiable Credentials the RP supports. OPENID4VC-HAIP Draft 00 Section 7.2.7 and OpenID4VP Draft 20 Section 9.1
jwks JSON Web Key Set document, passed by value, containing the protocol specific keys for the Relying Party. oauth-v2-jarm-03 Section 3, OIDC-FED Draft 36 Section 5.2.1, and JWK
jogu commented 3 months ago

Discussed on today's call; Joseph to do a PR based on Torsten's comment recording the previous WG consensus, https://github.com/openid/OpenID4VP/issues/17#issuecomment-2123350290

jogu commented 3 months ago

And just to respond to @peppelinux's comment above; the main two differences between his proposal and Torsten's were:

  1. Addition of client_id - this isn't needed because client_id is already present at the top level of the authorization request
  2. Addition of client_name - based on previous comments on this issue this seems contentious and I think we should discuss it in a separate issue if someone has a good use case for it. I am not yet clear in what situation it is useful for a client to provide a self-attested client_name, nor how that might be done without allowing the client to lie about its client_name. OAuth is generally based around the principle that one client_id has a single name, and that name is normally obtained from a trusted source rather than provided in the request.