```suggestion - The Wallet uses the `attestation` proof type in the Credential Request with the key attestation without a signature by key(s) themselves as specified in (#attestation-proof-type). ```

```suggestion * `key_attestations_required`: OPTIONAL. Object that describes the requirement for key attestations as described in (#keyattestation), which the Credential Issuer expects the Wallet to send within the proof of the Credential Request. If the Credential Issuer does not require a key attestation, this parameter MUST NOT be present in the metadata. If both `key_storage` and `user_authentication` parameters are absent, the `key_attestations_required` parameter may be empty, indicating a key attestation is needed without additional constraints. ```

```suggestion * `key_storage`: OPTIONAL. Array defining values specified in (#keyattestation-apr) accepted by the Credential Issuer. ```

```suggestion "key_storage": [ "iso_18045_moderate" ], ```

```suggestion * `key_storage` : OPTIONAL. Array of case sensitive strings that assert the attack potential resistance of the key storage component and its keys attested in the `attested_keys` parameter. This specification defines initial values in (#keyattestation-apr). ```

How can one know that `the value does not map to a well-known specification`? Some IETF specs (e.g. Web Linking and link relations) mandate that *any* non-standard value must be an URI (e.g. https://www.rfc-editor.org/rfc/rfc8288.html#section-2.1.2). I suggest a similar approach here.

If `key_storage_type` and `user_authentication` now use APRs, then perhaps: - Rename `key_storage_type` to just `key_storage` (`user_authentication` does not have the `type` suffix`). - Make `key_storage` also be an array.

```suggestion Depending on the Wallet's implementation, the `attestation` may avoid unnecessary End-User interaction during Credential issuance, as the key itself does not necessarily need to perform signature operations. ```

```suggestion A Wallet MAY provide key attestations to inform the Credential Issuer about the properties of the cryptographic public keys, such as those used in proof types sent in the Credential Request. Credential Issuers SHOULD evaluate these key attestations to ensure the cryptographic keys meet their security requirements, based on the trust framework, regulatory requirements, laws, or internal policies. A Credential Issuer MUST communicate the need to evaluate key attestations through its metadata or via an out-of-band mechanism. ```

```suggestion If an adversary obtains a key proof (as outlined in #proof-types), they could potentially have a Credential issued that is linked to a key pair controlled by the victim. The `c_nonce` parameter serves as the main defense against replay attacks involving key proofs. It is RECOMMENDED that Credential Issuers utilize the Nonce Endpoint as specified in [](nonce-endpoint). A Wallet MAY continue using a given nonce until it either expires or is rejected by the Credential Issuer, or request a new one. The Credential Issuer determines how frequently a particular nonce can be used. Servers SHOULD establish a clear policy on whether the same key proof can be reused and for how long, or if each Credential Request requires a new key proof. ```

```suggestion **Key attestation** is a mechanism where the key storage component or Wallet Provider asserts the cryptographic public keys and their security policy. The Wallet MAY provide this data in the Credential Request to allow the Credential Issuer to validate the cryptographic key management policy. This requires the Credential Issuer to rely on the trust anchor of the key attestation and the respective key management policy. While some existing platforms have key attestation formats, this specification introduces a common key attestation format that may be used by Credential Issuers for improved interoperability, see [](#keyattestation). ```

```suggestion * `key_attestations_required`: OPTIONAL. Object that describes the requirement for key attestations as described in (#keyattestation), which the Credential Issuer expects the Wallet to send within the proof of the Credential Request. If the Credential Issuer does not require a key attestation, this parameter MUST NOT be present in the metadata. If both `key_storage_type` and `user_authentication` parameters are absent, the `key_attestations_required` parameter may be empty, indicating a key attestation is needed without additional constraints. ```

duplicated here: https://github.com/openid/OpenID4VCI/pull/389/files#r1841798940

```suggestion The Credential Issuer SHOULD issue a Credential for each cryptographic public key specified in the `attested_keys` claim within the `key_attestation` parameter. ```

```suggestion When ISO 18045 is not used, ecosystems may define their own values. If the value does not map to a well-known specification, it is RECOMMENDED that the value is a URL that gives further information about the attack potential resistance and possible relations to level of assurances. ``` suggested during the WG call

`apr` was not removed here and the references are not matching the new section ```suggestion * `key_attestations_required`: OPTIONAL. Object that describes the requirement for key attestations as described in (#keyattestation), which the Credential Issuer expects the Wallet to send within the proof of the Credential Request. If the Credential Issuer does not expect a key attestation, this object is absent. If neither of the `key_storage_type` and `user_authentication` parameters are present, this object may be empty, indicating that a key attestation without further constraints is required. * `key_storage_type`: OPTIONAL. Array defining values specified in (#keyattestation-apr) accepted by the Credential Issuer. * `user_authentication`: OPTIONAL. Array defining values specified in (#keyattestation-apr) accepted by the Credential Issuer. ```

Should we preserve an example that uses the x5c parameter too given it is still a header parameter declared in the definition of the proof syntax here https://github.com/openid/OpenID4VCI/pull/389/files#diff-1f424614b35a9899813079f1b1f6218631a2aedd993368ccb89bb81a9eda0289R884

Could the guidance be more general here e.g ```suggestion The Credential Issuer SHOULD return a Credential for each of the keys provided in the `attested_keys` claim of the attestation. ``` Im not sure we need to caveat the normative statement to only imply this is the behaviour when the attestation is validated and or signed by one of the attested keys do we? Or perhaps I'm missing something, is there a case where we cont intent to return a credential for each of the keys provided in the `attested_keys` claim of the attestation.

```suggestion * `typ`: REQUIRED. MUST be `wallet-key-attestation+jwt`, which explicitly types the key proof JWT as recommended in Section 3.11 of [@!RFC8725]. ``` as suggested at IIW

```suggestion The latter may avoid unnecessary End-User interaction during the Credential issuance, as the key itself is not performing a signature operation. ```

```suggestion Key attestations are used by various Credential Issuers with different trust frameworks and requirements, so a common approach is needed for interoperability. Therefore, key attestations SHOULD follow a common format. This helps Credential Issuers create consistent evaluation processes, reducing complexity and errors. Common formats also simplify compliance with regulatory requirements across jurisdictions and support the creation of shared best practices and security standards. ``` It looks more like an implementation consideration, it would make sense moving in that section.

```suggestion A key attestation, as defined by this specification, is a verifiable statement that demonstrates the authenticity and security properties of a key and its storage component to the Credential Issuer. Keys can be stored in various key storage components, which vary in their ability to protect the private key from extraction and duplication, as well as in the methods used for User authentication to enable key operations. These key storage components can be software-based or hardware-based and may reside on the same device as the Wallet, on external security tokens, or on remote services that facilitate cryptographic key operations. Key attestations are issued either by the Wallet's key storage component itself or by the Wallet Provider. When the Wallet Provider creates the key attestation, it MUST verify the authenticity of its claims about the keys, possibly using platform-specific key attestations. A Wallet MAY provide key attestations to inform the Credential Issuer about the properties of the cryptographic public keys, such as those used in proof types sent in the Credential Request. Credential Issuers SHOULD evaluate these key attestations to ensure the keys meet their security requirements, based on the trust framework, regulatory requirements, laws, or internal policies. A Credential Issuer MUST communicate the need to evaluate key attestations through its metadata or via an out-of-band mechanism. ```

```suggestion If an adversary obtains a key proof (as outlined in #proof-types), it could potentially have a Credential issued that is linked to a key pair controlled by the victim. The `c_nonce` parameter serves as the main defense against replay attacks involving key proofs. It is RECOMMENDED that Credential Issuers utilize the Nonce Endpoint as specified in [](nonce-endpoint). A Wallet can continue using a given nonce until it either expires or is rejected by the Credential Issuer. The Credential Issuer can determine how frequently a particular nonce can be used. Servers MUST establish a clear policy on whether the same key proof can be reused and for how long, or if each Credential Request requires a new key proof. ```

```suggestion **Key attestation** is a mechanism where the key storage component or Wallet Provider asserts the keys and their security policy. The Wallet MAY provide this data in the Credential Request to allow the Credential Issuer to validate the key management policy. This requires the Credential Issuer to rely on the trust anchor of the key attestation and the respective key management policy. While some existing platforms have key attestation formats, this specification introduces a common key attestation format that may be used by Credential Issuers for improved interoperability, see [](#keyattestation). ```

```suggestion A Wallet MAY provide key attestations to inform the Credential Issuer about the properties of the provided cryptographic public keys, e.g. for proof types sent in the Credential Request. Credential Issuers may want to evaluate these key attestations to determine whether the keys meet their own security requirements, based on the trust framework in use, regulatory requirements, laws, or internal design decisions. A Credential Issuer MUST communicate this requirement to evaluate key attestations through its metadata or using some sort of out-of-band mechanism. ```

```suggestion If an `attestation` is provided, successfully validated by the Credential Issuer, and the JWT used as proof was signed by one of the attested keys, the Credential Issuer SHOULD return a Credential for each of the keys provided in the `attested_keys` claim of the attestation. ```

would like to see change in this structure so mapping between key_type and user_authn can be communicated?

where the proof of association (POA) is made evident in this jwk?

```suggestion "typ": "wallet-key-attestation+jwt", ``` may we use the name pattern also used in other specs prepending "wallet"?

feedback from a Funke team: it would be much more actionable if this information is communicated per credential type, and not in general, so "i need HSM for PID, but for diploma strongbox is ok" as opposed to "I am ok with HSM or strongbox"

```suggestion Specifications that extend this list MUST choose collision-resistant values. ```

```suggestion This specification defines the following values for `key_storage_type`: ```

```suggestion * `key_attestations_required`: OPTIONAL. Object that describes the requirement for key attestations as described in (#keyattestation), which the Credential Issuer expects the Wallet to send within the proof of the Credential Request. If the Credential Issuer does not expect a key attestation, this object is absent. If neither of the `key_storage_type`, `user_authentication` and `apr` parameters are present, this object may be empty, indicating that a key attestation without further constraints is required. ```

```suggestion "key_storage_type": "strong_box", ```

I line with the proposal of Oliver ```suggestion * `key_storage_type`: OPTIONAL. Array defining values specified in (#keyattestation-keytypes) accepted by the Credential Issuer. ```

```suggestion * `x5c`: OPTIONAL. JOSE Header containing a certificate or certificate chain corresponding to the key used to sign the JWT. ```

Should OID4VCI also say something about how these key attestations might relate to encryption? We had the discussion in the past that the encryption is not adding a lot on top of TLS because the wallet keys are not trusted necessarily. If the key attestations would also contain keys the wallet provides for encryption, wouldn't this solve this specific problem we had with encryption?

I know it is impossible to come up with an exhaustive list at this point, but there are other authentication factors missing, e.g., knowledge-based authentication. Perhaps we should already make room for an extension mechanism? ```suggestion Specifications that extend this list MUST choose collision-resistant values. ```

Wouldn't it make sense to allow to convey the key attestations also in the in the Pushed Authorization and/or Token Request, i.e., using OAuth2 attestation-based client authentication? This way, the AS could also make a decision if they want to issue an access/refresh token to the wallet? Or is this mechanism out of scope of OID4VCI while still being possible because an AS can use the OAuth2 client authentication extension mechanism to support this use case anyways?

Are the methods defined in `user_authentication` evaluated as OR or AND? If they are OR, then how would you encode MFA?

Perhaps `key_storage_type` is the better term? ```suggestion * `key_storage_type` : OPTIONAL. Case sensitive string that asserts the key storage component of the keys attested in the `attested_keys` parameter. This specification defines initial values in (#keyattestation-keytypes). ```

is the indent a bit off here on the bullet for `x5c`, I cannot tell

Stupid comment, but this `kid` is a bit confusing for me here in this form, is this referring to a particular key from the attestation or is it just a random value for the sake of the example?

```suggestion * `nonce`: OPTIONAL. String that represents a nonce provided by the Issuer to prove that a key attestation was freshly generated. ```

is a word missing here, or am I missing something? ```suggestion When a key attestation is used as a proof type, it MUST contain the `c_nonce` value provided by the Credential Issuer in its `nonce` parameter. ```

Why only SHOULD?

Why only SHOULD?

Is this object also used for proof type "attestation"? If so, this statement should be conditional.

```suggestion * `attestation`: A JWT [@!RFC7519] representing a key attestation without using a proof of possession of the cryptographic key material that is being attested. When a `proof_type` parameter in a `proof` object is set to `attestation`, the object MUST also contain an `attestation` parameter that includes a JWT as defined in (#attestation-proof-type). There are two ways to convey key attestation(s) of the cryptographic key material during Credential issuance. For details, see (#keyattestation). ```

Wondering it this could be an JWKS (https://www.rfc-editor.org/rfc/rfc7517.html#section-5) instead of directly an array if JWK. Having an object here is an extra indirection however it is a standard representation and provides "extra space" to add more common properties to this set of keys.

```suggestion There are two ways to convey key attestations during Credential issuance: ```

```suggestion Since key attestations may be used for different Credential Issuers from different trust frameworks and varying in their requirements, it is necessary to use a common approach to facilitate interoperability. Therefore, key attestations SHOULD use a common format, allowing Credential Issuers to develop consistent evaluation processes, reducing complexity and potential errors. Common formats make it easy for Credential Issuers to demonstrate compliance with regulatory requirements across different jurisdictions and facilitate the development of shared best practices and security benchmarks. ```

```suggestion A key attestation defined by this specification is an interoperable, verifiable statement that provides evidence of authenticity and security properties of a key and its storage component to the Credential Issuer. Keys can be stored in various key storage components, which differ in their ability to protect the private key against extraction and duplication, as well as in the methods used for End-User authentication to unlock key operations. These key storage components may be software-based or hardware-based, and can be located on the same device as the Wallet, on external security tokens, or on remote services that enable cryptographic key operations. Key attestations are issued by the Wallet's key storage component itself or by the Wallet Provider. When the Wallet Provider creates the key attestation, it needs to verify the authenticity of the claims it is making about the keys (e.g., by using the platform-specific key attestations). ```

```suggestion * `key_type` : OPTIONAL. Case sensitive string that asserts the security mechanism of all of the keys being attested in the `attested_keys` parameter. This specification defines initial values in (#keyattestation-keytypes). ``` probably better to make it clear that the same mechanism is used for all of the attested keys, right? was also trying to simplify "the key storage component and its security mechanism"...

```suggestion * `user_authentication` : OPTIONAL. Array of case sensitive strings that assert the security mechanisms used to authorize usage of the private key from `keys`. This specification defines initial values in (#keyattestation-auth). ``` is this what is intended? sorry, could not really understand the original sentence.. typo?

where is this claim name coming from..? and what are the potential values of the URNs..?

in JWT proof type, we only defined iat, and did not define exp, because we said it is up to the issuer's policy to decide how long to accept the proofs. why do we need exp here?

```suggestion * `key_attestations_required`: OPTIONAL. Object that describes the requirement for key attestations as described in (#keyattestation), which the Credential Issuer expects the Wallet to send within the proof of the Credential Request. If the Credential Issuer does not expect a key attestation, this object is absent. If neither of the `key_type`, `user_authentication` and `apr` parameters are present, this object may be empty, indicating that a key attestation without further constraints is required. ``` think there is a copy paste error?

```suggestion When a key attestation is used as a proof type, it MUST contain the `c_nonce` value provided by the Credential in its `nonce` parameter. ```

sorry for going back to this... I still think it should be ok to use x5c chain as key attestation, since key attestation in `key_attestation` parameter is a JWT and does not have an option to put x5c in the header.

```suggestion The latter may avoid unnecessary user interaction during the Credential issuance, as the key itself is not performing a signature operation. ```

```suggestion * `attestation`: A JWT [@!RFC7519] representing a key attestation without using a proof of possession of the cryptographic key material that is being attested. When a `proof_type` parameter in a `proof` object is set to `attestation`, the object MUST also contain an `attestation` parameter that includes a JWT as defined in (#attestation-proof-type). ```

```suggestion * `status`: OPTIONAL. JSON Object representing the supported revocation check mechanisms, such as the one defined in [@!I-D.ietf-oauth-status-list] ```

incorrect reference? Should probably be `[@!I-D.draft-ietf-oauth-status-list]` ```suggestion * `status`: OPTIONAL. JSON Object representing the supported revocation check mechanisms, such as the one defined in [@!I-D.draft-ietf-oauth-status-list] ```

Typos and using defined terms. ```suggestion Since the key attestations may have large audience as many Credential Issuers that not necessarily uses the same trust framework or internal design decisions, it is required to use a common approach to facilitate interoperability. Therefore, key attestations SHOULD use a common format, allowing Credential Issuers to develop consistent evaluation processes, reducing complexity and potential errors. Common formats makes easy for Credential Issuers to demonstrate compliance with regulatory requirements across different jurisdictions, they also facilitate the development of shared best practices and security benchmarks. ```

Same as above. `Issuer` on its own is not a defined term in this spec. ```suggestion A Wallet MAY provide key attestations to inform the Credential Issuer about the properties of the provided cryptographic public keys, e.g. for proof types sent in the Credential Request. Credential Issuers may want to evaluate these key attestations to determine whether the keys meet their own security requirements, based on the trust framework in use, regulatory requirements, laws, or internal design decisions. A Credential Issuer SHOULD communicate this requirement to evaluate key attestations through its metadata or using some sort of out-of-band mechanism. ```

`Issuer` on its own is not a defined term in this spec. ```suggestion A key attestation defined by this specification is an interoperable, verifiable statement that provides to the Credential Issuer, the evidence of authenticity and security properties of a key and its storage component. Keys can be stored in various key storage components, which differ in their ability to protect the private key against extraction and duplication, as well as in the methods used for End-User authentication to unlock key operations. These key storage components may be software-based or hardware-based, and can be located on the same device as the Wallet, on external security tokens, or on remote services that enable cryptographic key operations. Key attestations are issued by the Wallet's key storage component itself or by the Wallet Provider. When the Wallet Provider creates the key attestation, it needs to verify the authenticity of the claims it is making about the keys (e.g., by using the platform-specific key attestations). ```

```suggestion "user_authentication": [ "system_pin", "system_biometry" ], ```

```suggestion * `secure_element`: It MUST be used when the Wallet uses a Secure Element for key management. * `external`: It MUST be used when the Wallet uses a local external hardware token for key management. ```

```suggestion * `hsm`: It MUST be used when the Wallet uses Hardware Security Module (HSM) for key management. * `remote_hsm`: It MUST be used when the Wallet uses Hardware Security Module (HSM) that is not directly connected to the Wallet for key management. ```

```suggestion * `software`: It MUST be used when the Wallet uses software-based key management. * `tee`: It MUST be used when the Wallet uses the Trusted Execution Environment for key management. * `secure_enclave`: It MUST be used when the Wallet uses the Secure Enclave for key management. * `strong_box`: It MUST be used when the Wallet uses the Strongbox for key management. * `secure_element`: It MUST be used when the Wallet uses a Secure Element for key management. ```

```suggestion * `key_type` : OPTIONAL. Case sensitive string that asserts the key storage component and its security mechanism of attested keys from the `attested_keys` parameter. This specification defines initial values in (#keyattestation-keytypes). ```

```suggestion "apr" : [ "https://trust-list.eu/apr/high" ], ```

```suggestion * `apr` : OPTIONAL. Array of case sensitive strings that assert attested resistance to specified attack potentials for the given keys. The string values contain URNs that identify the given attack potentials. ```

```suggestion "user_authentication": [ "system_pin" ], ```

```suggestion The Credential Issuer SHOULD return a Credential for each of the keys provided in the `attested_keys` claim of the `attestation`. ```

```suggestion A key attestation defined by this specification is an interoperable, verifiable statement that provides to the Issuer, the evidence of authenticity and security properties of a key and its storage component. Keys can be stored in various key storage components, which differ in their ability to protect the private key against extraction and duplication, as well as in the methods used for End-User authentication to unlock key operations. These key storage components may be software-based or hardware-based, and can be located on the same device as the Wallet, on external security tokens, or on remote services that enable cryptographic key operations. Key attestations are issued by the Wallet's key storage component itself or by the Wallet Provider. When the Wallet Provider creates the key attestation, it needs to verify the authenticity of the claims it is making about the keys (e.g., by using the platform-specific key attestations). ```

```suggestion * `x5c`: OPTIONAL. JOSE Header containing a certificate or certificate chain corresponding to the key used to sign the JWT. * `key_attestation`: OPTIONAL. JOSE Header containing a key attestation as described in (#keyattestation). ```

```suggestion * `system_biometry`: It MUST be used when the key usage is authorized by the key storage component itself or the associated operating system using a biometric factor, such as the one provided by mobile devices. * `system_pin`: It MUST be used when the key usage is authorized by the key storage component itself or the associated operating system using personal identification number (PIN). * `remote_biometry`: It MUST be used when the key usage is authorized by a remote system using a biometric factor. * `remote_pin`: It MUST be used when the key usage is authorized by a remote system using a PIN. ```

```suggestion ```

```suggestion Since the key attestations may have large audience as many Credential Issuers that not necessarly uses the same trust framework or internal design decisions, it is required to use a common approach to facilitate interoperability. Therefore, key attestations SHOULD use a common format,allowing Issuers to develop consistent evaluation processes, reducing complexity and potential errors. Common formats makes easy for Issuers to demonstrate compliance with regulatory requirements across different jurisdictions, they also facilitate the development of shared best practices and security benchmarks. ```

```suggestion * `attestation`: A JWT [@!RFC7519] representing a key attestation without proof of possession is used. When a `proof_type` parameter in a `proof` object is set to `attestation`, it MUST also contain an `attestation` parameter that includes a JWT as defined in (#attestation-proof-type). ```

What about "cloud" secure area (where keys are managed on the server)?

Actually prototyping an implementation of this. If there are multiple keys, do we create multiple credentials? Can this be made clear in the spec one way or another?

Where does a (Cloud) HSM PIN fit in here?

We might want to consider combinations (as provided by mobile operating systems). This allows for better UX in certain situations (similar to Apple/Google/Samsung Pay allowing to fall-back from to biometry to device pin). Examples: - **Biometry **OR** PIN**: `.biometryAny` OR `.devicePin` on iOS, `AUTH_BIOMETRIC_STRONG` OR `AUTH_DEVICE_CREDENTIAL` on Android - **Biometry **AND** PIN**: When using a (Cloud) HSM, the user might need to use biometry to access the key in the TEE/SecureEnclave and then a PIN for the HSM.

```suggestion "kid": "0", ``` This header parameter is a string as per Section 4.1.4 of RFC 7515 https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.4

```suggestion * `exp`: REQUIRED (number). Integer for the time at which the key attestation and the key(s) it is attesting expire, using the syntax defined in [@!RFC7519]. ```

I would put this paragraph in the beginning of this section. this is super important to explain upfront why platform attestation is not used, and generic mechanism is used instead

umm... I don't think this really reflects what we discussed in the WG: https://github.com/openid/OpenID4VCI/pull/389#issuecomment-2386849999 I think key attestation proof type should be adjusted to both options documented in the comment above, to support both key attestation proof type with nonce in key attestation and key attestation proof type with nonce in PoP. and key attestation in jwt proof type being removed. because if we go the current path. an option to do key attestation without a nonce is only limited to key attestation per Proof, right? because i don't think jwt proof type has been extended to include more than one key.

```suggestion A key attestation defined by this specification is an interoperable, verifiable statement that provides to the Issuer, the evidence of authenticity and security properties of a key and its storage component. Keys can be stored in various key storage components, which differ in their ability to protect the private key against extraction and duplication, as well as in the methods used for End-User authentication to unlock key operations. These key storage components may be software-based or hardware-based, and can be located on the same device as the Wallet, on external security tokens, or on remote services that enable cryptographic key operations. Key attestations are signed by the Wallet Provider or the Wallet's key storage component itself. When it is the Wallet Provider that creates the key attestation, it needs to check the authenticity of the claims it is making about the keys, for example, by using the platform-specific attestations. ``` I think there needs to be a differentiation between key attestation defined by this specification and platform provided key attestations - people seem to be confusing the two. we also using the term "wallet provider" without really defining it... wouldn't "wallet backend" be more generic?

```suggestion When a key attestation is used as a proof type, it MUST contain the `nonce` claim if a `c_nonce` was provided by the Credential Issuer. ``` second half of the sentence feels redundant.

```suggestion * `attestation`: A JWT [@!RFC7519] representing a key attestation is used instead of a proof of possession of an individual cryptographic key material. When a `proof_type` parameter in a `proof` object is set to `attestation`, it MUST also contain an `attestation` parameter that includes a JWT as defined in (#attestation-proof-type). ``` attestation can contain proof of possession so `instead of a PoP` sounded a little off to me.

```suggestion "attestation": "<key attestation in JWT format>" ```

This shouldn't be an array ```suggestion "key_attestation": <key attestation in JWT format> ```

why is `x5c` being deleted? `key_attestation` parameter should be added in addition to `x5c`, not instead of it

```suggestion A key attestation is an interoperable, verifiable statement that provides evidence of the authenticity and security properties of a key and its storage component. Keys can be stored in various key storage components, which differ in their ability to protect the private key against extraction and duplication, as well as in the methods used for End-User authentication to unlock key operations. These key storage components may be software-based or hardware-based, and can be located on the same device as the Wallet, on external security tokens, or on remote services that enable cryptographic key operations. Key attestations are signed by the Wallet Provider or the Wallet's key storage component itself. ```

```suggestion * `key_type` : OPTIONAL. String that asserts the key storage component and its security mechanism of attested keys from the `attested_keys` parameter. This specification defines initial values in (#keyattestation-keytypes). ```

```suggestion * `apr` : OPTIONAL. String that asserts the resistance to a specified attack potential. The value contains an URL that identifies the given attack potential. ```

```suggestion * `attested_keys` : REQUIRED. Array of attested keys from the same key storage component using the syntax of JWK as defined in [@!RFC7517]. ```

```suggestion "attested_keys": [ ```

This is just a detail, but how about calling the `keys` `attested_keys` ? I think this would make it perfectly clear what this parameter is.

```suggestion "typ": "keyattestation+jwt", ```

I suggest to also add an optional parameter that represents the key lifetime (like maximum duration for example). The Wallet solution provider might have reasons to enforce extra constraints and therefore he would need to specify to the Issuer to reduce the initially forseen validity of the EAA.

maybe point to the definitions of these parameters in `jwt` proof type section https://openid.github.io/OpenID4VCI/openid-4-verifiable-credential-issuance-wg-draft.html#section-7.2.1.1

I think the text can be more explicit about that the comparison here is with using platform specific key attestations (google/apple).

what is the mechanism an issuer uses to communicate key attestation requirement in the issuer metadata..?

```suggestion A Wallet MAY provide key attestations to inform the Credential Issuer about the properties of the provided cryptographic public keys, e.g. for proof types sent in the Credential Request. Credential Issuers may want to evaluate these key attestations to determine whether the keys meet their own security requirements, based on the trust framework in use, regulatory requirements, laws, or internal design decisions. An Issuer SHOULD communicate this requirement to evaluate key attestations through its metadata or using some sort of out-of-band mechanism. ```

we already have a definition of a key attestation here. could we simply point there? https://openid.github.io/OpenID4VCI/openid-4-verifiable-credential-issuance-wg-draft.html#section-12.1-4

```suggestion * `system_biometry`: It MUST be used when the key usage is authorized by the operating system using a biometric factor, such as the one provided by mobile devices. ``` I make this suggestion because nothing prevents to have a wallet installed on a laptop or provided via Cloud, while biometry should not necessarly depend by mobile device manufacturers

it seems to me that we definitively, deliberately, have decided to not include `sub` anymore for these kind of attestations

```suggestion This is an example of a Wallet Attestation: ``` we need to take a final decision if call it Wallet Instance Attestation or Wallet Attestation alone. I have the same problem when I write a spec. We also need a specification for the standardization of the wallet attestation, we have other similar non-normative examples in the oauth attestation based client authentication

```suggestion * `nonce`: OPTIONAL. String that represents a nonce provided by the Issuer to proof that a key attestation was freshly generated. * `status`: OPTIONAL. JSON Object representing the supported revocation check mechanisms, such as the one defined in [status list] ```

```suggestion Since the key attestations may have large audience as many Credential Issuers that not necessarly uses the same trust framework or internal design decisions, it is required to use a common approach to facilitate interoperability. Therefore, key attestations SHOULD use a common format,allowing Issuers to develop consistent evaluation processes, reducing complexity and potential errors. Common formats makes easy for Issuers to demonstrate compliance with regulatory requirements across different jurisdictions, they also facilitate the development of shared best practices and security benchmarks. ```

```suggestion A Wallet MAY provide key attestations to inform the Credential Issuer about the properties of the provided cryptographic public keys. Credential Issuers may want to evaluate these key attestations to determine whether the keys meet their own security requirements, which can result from the trust framework in use, regulatory requirements, laws, or internal design decisions. An Issuer SHOULD communicate this requirement to evaluate key attestations through its metadata or using some sort of out-of-band mechanism. ```

```suggestion A key attestation is an interoperable, verifiable statement that provides evidence of the authenticity and security properties of a key and its storage component. Keys can be stored in various key storage components, which differ in their ability to protect the private key against extraction and duplication, as well as in the methods used for End-User authentication to unlock key operations. These key storage components may be software-based or hardware-based, and can be located on the same device as the Wallet, on external security tokens, or on remote services that enable cryptographic key operations. ```

```suggestion * `software`: It MUST be used when the Wallet uses software-based key management. ```

isn't hardware somewhat redundant with the following values that are also hardware based? Or is hardware supposed to be something like an external device (like a FIDO authenticator)?

Should we be more explicit for `biometry`, allowing specific biometric factors like video / fingerprint?

Do you think it makes sense to remove `openid4vci` here? ```suggestion * `typ`: REQUIRED. MUST be `keyattestation+jwt`, which explicitly types the key proof JWT as recommended in Section 3.11 of [@!RFC8725]. ```

I would add a section here that the key attestation is created by the Wallet Provider or key storage component (as mentioned below). I think this should be introduced here and not in the JWT specific parts.