Introduce Credential and IdCredential as successors to CredentialRpk#274
Closed
chrysn wants to merge 6 commits intolake-rs:mainfrom
Closed
Introduce Credential and IdCredential as successors to CredentialRpk#274chrysn wants to merge 6 commits intolake-rs:mainfrom
chrysn wants to merge 6 commits intolake-rs:mainfrom
Conversation
Member
Author
|
This needed updates after understanding the need for #281. |
Workaround-For: cryspen/hax#96 Workaround-For: cryspen/hax#15
This is required since learning that not all credentials are encoded,
when sent by value, in {key: 'bytestring'}, but some (such as CCS) are
sent directly as {/kccs/14: encoded_credential}. Consequently, the
generic builder for typed credentials goes away -- one can still have
arbitrarily kinds of credentials in an EDHOC implementation, but passing
them by value will require per-type information.
geonnave
reviewed
May 23, 2024
| /// If set, it enables using the credential by value. Setting this adds the requirement that | ||
| /// the value is well-formed according to the type. In particular, when set to KCCS or any | ||
| /// other Map-valued types, a value that is not CBOR can result in hard to debug parsing errors | ||
| /// on the other end, even before it reaches the credential validation at the peer. |
Collaborator
There was a problem hiding this comment.
s/at the peer/at the peer's application/
Collaborator
|
Yes I think this is in the right direction. |
geonnave
reviewed
Jun 4, 2024
| value: EdhocMessageBuffer, | ||
| /// Public authentication key (G_I or G_R) expressed in the credential | ||
| // For CCSs, this could be a reference into value, but Rust does not allow self-referential structs anyway. | ||
| public_key: BytesP256ElemLen, |
Collaborator
There was a problem hiding this comment.
can you help me understand why the value and public_key are not optional, while the kid is optional?
for context, I will have to add support for an experimental PSK credential (for the emerging edhoc-psk draft), and I am trying to see how things are going to fit here. for example, I was thinking that the PSK can be stored separately (like the private keys), and the credential type might hold only the kid that identifies the PSK.
Closed
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jun 21, 2024
will support more types of credentials,
including different types of COSE_Key's
the handling of id_cred's is inspired by chrysn's PR lake-rs#274
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jun 27, 2024
will support more types of credentials,
including different types of COSE_Key's
the handling of id_cred's is inspired by chrysn's PR lake-rs#274
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jul 3, 2024
borrowing code from @chrysn's PR lake-rs#274
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jul 3, 2024
borrowing code from @chrysn's PR lake-rs#274
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jul 3, 2024
borrowing code from @chrysn's PR lake-rs#274
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jul 31, 2024
will support more types of credentials,
including different types of COSE_Key's
the handling of id_cred's is inspired by chrysn's PR lake-rs#274
geonnave
added a commit
to geonnave/lakers
that referenced
this pull request
Jul 31, 2024
borrowing code from @chrysn's PR lake-rs#274
Collaborator
|
This PR served very well as inspiration to the credential-related updates in #284! Since that one is now merged, I am closing this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #272, #273
This is a first sketch of how I'd go about #272; the design includes some choices influenced by my assessment of #273. The two new types should eventually replace CredentialRpk completely (and also remove the need for BytesIdCred).
If I'm wrong about #273, Credential may still have a
.value_type, but it wouldn't be needed for.by_value()any more, only for.by_thumbprint()(which we don't have yet).@geonnave, is this a good direction?