- Notifications
You must be signed in to change notification settings - Fork 0
feat(rust/signed-doc): CBOR encoder using minicbor #351
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
Changes from all commits
9db5fe2 88f75d7 617a07f 1192b90 9a5e9d0 322fcf6 a4b821b 843299c b78ed40 53cb828 63208f7 57f2e23 52fc974 fcd6b54 d397a3a 2eb553b e97bd3c 1515bb7 File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -9,7 +9,7 @@ use std::{ | |
| }; | ||
| | ||
| use anyhow::Context; | ||
| use catalyst_signed_doc::{Builder, CatalystId, CatalystSignedDocument}; | ||
| use catalyst_signed_doc::{CatalystId, CatalystSignedDocument, CoseSignBuilder}; | ||
| use clap::Parser; | ||
| | ||
| fn main() { | ||
| | @@ -66,7 +66,7 @@ impl Cli { | |
| // Possibly encode if Metadata has an encoding set. | ||
| let payload = serde_json::to_vec(&json_doc)?; | ||
| // Start with no signatures. | ||
| let signed_doc = Builder::new() | ||
| let signed_doc = CoseSignBuilder::new() | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just am not seeing the need for a builder. I think its a huge over-complication to build a generic COSE builder when we only need a way to serialize catalyst signed documents. If we can encode the payload and metadata, this builder is literally just encoding: which is just a single byte defining an array of 3 entries. Which does seem overkill for a general COSE builder in our use case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing that @Mr-Leshiy pointed out to me is that an unrestricted builder could be handy in tests. In other words, a generic builder allows for making a valid COSE that isn't a valid Catalyst Signed Doc. For example, as you mentioned, we want to keep field ordering deterministic. This can be checked trivially if there's some abstraction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't have to be a builder, it could as well be an extension trait, which would allow to encode a type as a protected header. Or at least something that abstracts enough, so that devs don't have to look up the RFC. For example, this seems to me like a misuse of an encoder. And this would have to be replaced (so could as well be abstracted for both the The links point to the new PR, where abstractions over minicbor encoder aren't used. | ||
| .with_decoded_content(payload) | ||
| .with_json_metadata(metadata)? | ||
| .build(); | ||
| | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| use minicbor::{ | ||
| bytes::{ByteArray, ByteSlice}, | ||
| data::Tagged, | ||
| Encode as _, | ||
| }; | ||
| | ||
| use super::VecEncodeError; | ||
| | ||
| /// Encode headers using the provided cbor-encoded key-value pairs, | ||
| /// conforming to the [RFC 8152 specification](https://datatracker.ietf.org/doc/html/rfc8152#autoid-8). | ||
| pub fn encode_headers<'a, I>(iter: I) -> Vec<u8> | ||
| where I: IntoIterator<Item = (&'a [u8], &'a [u8]), IntoIter: ExactSizeIterator> { | ||
| let mut encoder = minicbor::Encoder::new(vec![]); | ||
| | ||
| let iter = iter.into_iter(); | ||
| let map_len = u64::try_from(iter.len()).unwrap_or(u64::MAX); | ||
| encoder.map(map_len); | ||
| | ||
| for (encoded_key, encoded_v) in iter { | ||
| // Writing a pre-encoded field of the map. | ||
| encoder.writer_mut().extend_from_slice(encoded_key); | ||
| encoder.writer_mut().extend_from_slice(encoded_v); | ||
| } | ||
| | ||
| encoder.into_writer() | ||
| } | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given we have a fixed set of headers, and we will only be using those, seems overcomplex to maintain a general COSE header builder which already requires the keys and values to be encoded. its just complication for no gain. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I agree on this. This function exists, because @Mr-Leshiy and I used to want the generic builder, and since these were all immediately encoded and stored in a map, I had to make a concatenating function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the generic builder is deprecated, I'm sure this can be done way simpler and better. | ||
| | ||
| /// Encode a single protected `kid` header for the COSE Signature. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// - If encoding of the `kid` fails. | ||
| pub fn encoed_kid_header(kid: &[u8]) -> Result<Vec<u8>, VecEncodeError> { | ||
| /// The KID label as per [RFC 8152 3.1 section](https://datatracker.ietf.org/doc/html/rfc8152#section-3.1). | ||
| pub const KID_LABEL: u8 = 4; | ||
| | ||
| let mut encoder = minicbor::Encoder::new(vec![]); | ||
| // A map with a single `kid` field. | ||
| encoder.map(1u64)?.u8(KID_LABEL)?.bytes(kid)?; | ||
| Ok(encoder.into_writer()) | ||
| } | ||
| | ||
| /// Create a binary blob that will be signed. No support for unprotected headers. | ||
| /// | ||
| /// Described in [section 2 of RFC 8152](https://datatracker.ietf.org/doc/html/rfc8152#section-2). | ||
| pub fn encode_tbs_data( | ||
| protected_headers: &[u8], signature_header: &[u8], content: Option<&[u8]>, | ||
| ) -> Result<Vec<u8>, VecEncodeError> { | ||
| /// The context string as per [RFC 8152 section 4.4](https://datatracker.ietf.org/doc/html/rfc8152#section-4.4). | ||
| const SIGNATURE_CONTEXT: &str = "Signature"; | ||
| | ||
| minicbor::to_vec(( | ||
| SIGNATURE_CONTEXT, | ||
| <&ByteSlice>::from(protected_headers), | ||
| <&ByteSlice>::from(signature_header), | ||
| ByteArray::from([]), // no aad. | ||
| <&ByteSlice>::from(content.unwrap_or(&[])), // allowing no payload (i.e. no content). | ||
| )) | ||
| } | ||
| | ||
| /// Encode COSE signature. | ||
| /// | ||
| /// Signature bytes should represent a cryptographic signature. | ||
| pub fn encode_cose_signature( | ||
| protected_header: &[u8], signature_bytes: &[u8], | ||
| ) -> Result<Vec<u8>, VecEncodeError> { | ||
| minicbor::to_vec([ | ||
| <&ByteSlice>::from(protected_header), | ||
| <&ByteSlice>::from(signature_bytes), | ||
| ]) | ||
| } | ||
| | ||
| /// Encode an array from an iterator of pre-encoded COSE Signature items. | ||
| fn encode_cose_signature_array<S>(signatures: S) -> Result<Vec<u8>, VecEncodeError> | ||
| where S: IntoIterator<Item: AsRef<[u8]>, IntoIter: ExactSizeIterator> { | ||
| let iter = signatures.into_iter(); | ||
| let array_len = u64::try_from(iter.len().saturating_add(1)).unwrap_or(u64::MAX); | ||
| let mut encoder = minicbor::Encoder::new(vec![]); | ||
| encoder.array(array_len)?; | ||
| for signature in iter { | ||
| encoder.bytes(signature.as_ref())?; | ||
| } | ||
| Ok(encoder.into_writer()) | ||
| } | ||
| | ||
| /// Make cbor-encoded tagged [RFC9052-CoseSign](https://datatracker.ietf.org/doc/html/rfc9052). | ||
| pub fn encode_cose_sign<W: minicbor::encode::Write, S>( | ||
| e: &mut minicbor::encode::Encoder<W>, protected: &[u8], payload: Option<&[u8]>, signatures: S, | ||
| ) -> Result<(), minicbor::encode::Error<W::Error>> | ||
| where S: IntoIterator<Item: AsRef<[u8]>, IntoIter: ExactSizeIterator> { | ||
| /// From the table in [section 2 of RFC 8152](https://datatracker.ietf.org/doc/html/rfc8152#section-2). | ||
| const COSE_SIGN_TAG: u64 = 98; | ||
| | ||
| let tagged_array = Tagged::<COSE_SIGN_TAG, _>::new(( | ||
| <&ByteSlice>::from(protected), | ||
| ByteArray::from([]), // unprotected. | ||
| payload.map(<&ByteSlice>::from), // allowing `NULL`. | ||
| encode_cose_signature_array(signatures).map_err(minicbor::encode::Error::custom)?, | ||
| )); | ||
| tagged_array.encode(e, &mut ()) | ||
| } | ||
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.
remove all use of coset.
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.
But we have the decoding side still. Shouldn't it come form
cosetuntil that's merged and only then fully migrated tominicbor(having been tested)?