Updates the SignedCorim Sign method to include certificates #168
Conversation
Signed-off-by: Akhilesh Kr. Yadav <[email protected]>
|
Hi @thomas-fossati, I am still working on the corresponding tests. |
There was a problem hiding this comment.
Thanks!
I have left a few comments inline.
From an API design perspective, instead of changing the Sign's signature (apologies for the pun) I think it'd be better to add two new methods on the SignedCorim object to configure the end-entity cert and intermediate(s):
func (o *SignedCorim) AddSigningCert(der []byte) error { /* */ }
func (o *SignedCorim) AddIntemediateCerts(der []byte) error { /* */ }| // populated. | ||
| func (o *SignedCorim) Sign(signer cose.Signer) ([]byte, error) { | ||
| // The target SignedCorim must have its UnsignedCorim field correctly populated. | ||
| func (o *SignedCorim) Sign(signer cose.Signer, leafCert, intermediateCert []byte) ([]byte, error) { |
There was a problem hiding this comment.
We cannot make any assumptions on the depth of the cert chain, i.e., the number of intermediate certs could be > 1.
There was a problem hiding this comment.
Techinically, it could be 0 as well. Though unlikely, its possible the signing cert will be signed by the root CA cert.
| if leafCert == nil || intermediateCert == nil { | ||
| return nil, errors.New("nil certs") | ||
| } |
There was a problem hiding this comment.
We want to continue supporting the case of signed CoRIMs that rely on out-of-band means for communicating the verification key material.
Therefore, we shouldn't return an error here; continue as usual, just avoid adding the header.
| protectedHeadersCBOR, err := cbor.Marshal(protectedHeaders) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed CBOR encoding of protected headers: %w", err) | ||
| } |
There was a problem hiding this comment.
At this stage, this needs to remain an array - see https://www.rfc-editor.org/rfc/rfc9360.html#tab-1
The COSE library will take care of CBOR encoding.
| type ProtectedHeader struct { | ||
| X5Chain [][]byte `cbor:"33,keyasint,omitempty"` | ||
| } |
There was a problem hiding this comment.
It looks like this isn't needed.
| o.message.Headers.Protected.SetAlgorithm(alg) | ||
| o.message.Headers.Protected[cose.HeaderLabelContentType] = ContentType | ||
| o.message.Headers.Protected[HeaderLabelCorimMeta] = metaCBOR | ||
| o.message.Headers.Protected[33] = protectedHeadersCBOR |
There was a problem hiding this comment.
Rather than 33, you can use cose.HeaderLabelX5Chain
Attempts -- #159