-
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
base: feat/new-cat-signed-doc
Are you sure you want to change the base?
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
coset
until that's merged and only then fully migrated tominicbor
(having been tested)?