-
Notifications
You must be signed in to change notification settings - Fork 15
prohibit self-signed certificates for x509_hash #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| * The Wallet and Verifier MUST support at least one of the following Credential Format Profiles defined in (#vc-profiles): IETF SD-JWT VC or ISO mdoc. Ecosystems SHOULD clearly indicate which of these formats, IETF SD-JWT VC, ISO mdoc, or both, are required to be supported. | ||
| * The Response type MUST be `vp_token`. | ||
| * For signed requests, the Verifier MUST use, and the Wallet MUST accept the Client Identifier Prefix `x509_hash` as defined in Section 5.9.3 of [@!OIDF.OID4VP]. | ||
| * For signed requests, the Verifier MUST use, and the Wallet MUST accept the Client Identifier Prefix `x509_hash` as defined in Section 5.9.3 of [@!OIDF.OID4VP]. The X.509 certificate of the trust anchor MUST NOT be included in the `x5c` JOSE header of the signed request. The X.509 certificate signing the request MUST NOT be self-signed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are another 2 places that we mention x5c and don't mention "The X.509 certificate of the trust anchor MUST NOT be included in the x5c JOSE header of the signed request" - should we just fix them all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we want to prohibit self signed certifcates. Issue #236 also does not explain the rationale. So I suggest to remove the second sentence.
|
Discussed on this morning's WG call: Torsten is going to add a comment to the issue as he's not sure why we're preventing self-signed certs. |
tlodderstedt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we want to prohibit self signed certifcates. Issue #236 also does not explain the rationale. So I suggest to remove the second sentence.
@tlodderstedt Here are some of the reasons and does this make sense?
|
|
to clarify, |
|
thanks @awoie these are good reasons to prohibit self signed certs in the HAIP. |
tlodderstedt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I support the change given the fix @jogu asked for is applied.
paulbastian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but agree that we should apply this to all occurrences of x5c
Aren't these all effectively ecosystem policies? In many other parts of HAIP, ecosystems are left with responsiblity to supply policy which HAIP defers to. I don't see any of the above points being any different. Unless there are specific interoperability concerns that are being addressed, I think it should be a recommendation with the ecosystem ultimately deciding if they want to allow self-signed certificates. Trust management is explicitly out of scope for HAIP. There also seems to be the assumption that there are central CAs that will be responsible for oversight, however it is completely valid for each verifier to have their own CA. This comes down to ecosystem policy. While I am not dead set against forbidding self-signed certificates, I do want to make sure it's not stepping into trust management territory without there being valid interopability concerns that are getting addressed. |
|
@awoie please add this text to all occurrences of |
c2bo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved given the changes Joseph suggested (making sure this applies to all places that use x5c)
… in x5c for all x5c occurrences.
|
After WG discussion, there are better ways to solve concerns which rely on self-signed certificates. Prohibiting self-signed certificates doesn't impact trust management enough to be a meaningful reason to be against these changes. Happy with changes. |
|
Consensus on WG call to merge this once the conflicts are resolved. |
Fixes #236