Skip to content
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

Change OID name and add clarification notes #11

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

shnwc
Copy link
Contributor

@shnwc shnwc commented Feb 23, 2023

No description provided.

@shnwc shnwc assigned shnwc and unassigned shnwc Feb 23, 2023
@shnwc
Copy link
Contributor Author

shnwc commented Feb 24, 2023

This also closes issue #9. @dimakuv, @KB5201314 please review.

docs/Interoperable-RA-TLS-cert-edvidence-formats.md Outdated Show resolved Hide resolved
- CBOR tag: a CBOR tag registered with IANA or some other registry serves as evidence format ID
- CBOR data: this data covered by the tag contains the evidence (including custom claims). Its format is defined by the CBOR tag.
- CBOR data: this data covered by the tag contains the evidence (including custom claims). Its format is defined by the CBOR tag.
- Note: the DICE specification states that the criticality flag of this extension SHOULD be marked critical, but popular TLS libraries (such as openssl and mbedtls) will fail in verification of an X.509 cert with this extension if its criticality flag is set. To overcome this limitation, the criticality flag will be cleared.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you say will be cleared? Shouldn't it be more like Until most popular TLS libraries start recognizing this extension, the crticiality flag of this extension should be cleared.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented above, let's take another look to see if the criticality flag can be set.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my experiments with mbedTLS, no, we can't set the criticality flag. See #9 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review the latest commit.

Why do you say will be cleared? Shouldn't it be more like Until most popular TLS libraries start recognizing this extension, the crticiality flag of this extension should be cleared.


# Evidence Custom Claims
The evidence extension must include a `pubkey-hash` claim, and can optionally include a `nonce` claim:
The evidence extension must include a `pubkey-hash` claim, and can optionally include a `nonce` claim. All other custom claims will be ignored. How the custom claims are stored in the evidence is dependent on the specific evidence format definition.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How the custom claims are stored in the evidence is dependent on the specific evidence format definition.

Do we want this sentence? It doesn't really describe anything, and it only confused me on the first read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How the custom claims are stored in the evidence is dependent on the specific evidence format definition.

Do we want this sentence? It doesn't really describe anything, and it only confused me on the first read.

Thanks! I'll remove it.

@shnwc shnwc merged commit ea1df31 into CCC-Attestation:main Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants