fix!: serialize tags as tagged bstr #203
Merged
Conversation
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
The CoRIM spec defines the tags field of corim-map as
&(tags: 1) => [ + $concise-tag-type-choice ]
where
$concise-tag-type-choice /= tagged-concise-swid-tag
$concise-tag-type-choice /= tagged-concise-mid-tag
$concise-tag-type-choice /= tagged-concise-tl-tag
and
tagged-concise-swid-tag = #6.505(bytes .cbor concise-swid-tag)
tagged-concise-mid-tag = #6.506(bytes .cbor concise-mid-tag)
tagged-concise-tl-tag = #6.508(bytes .cbor concise-tl-tag)
i.e. tags field is supposed to be an array of one or more tagged
byte string, which are themselves CBOR encodings of the corresponding
struct.
Our UnsignedCorim.Tags was defined as []Tag, where Tag was an alias of
[]byte. It is an array of serialized tag objects. However, since it is
essentially, an array of byte strings, it was serialized as such,
resulting in
[ + bstr .cbor TAG(OBJECT) ]
instead of the expected
[ + TAG(bstr .cbor OBJECT) ]
i.e. tag ended up being part of the bstr, where as it ought to have been
wrapping the bstr.
This commit changes the definition of Tag to a struct that associates a
tag number with a []byte (it is similar to cbor.Tag except the content
is always a []byte rather than any). In CBOR, this type is serialized
exactly as a cbor.Tag; in JSON, this includes the tag in the
base64-encoded data.
BREAKING CHANGE: tags (CoMID, CoSWID, etc) are now serialized as tagged
bstr's, rather than plain bstrs (that contained the tag). API-wise, the
Tag type has been changed to a struct that associates a tag number with
a []byte.
Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
thomas-fossati
approved these changes
Jun 19, 2025
| @@ -195,5 +195,5 @@ func Example_profile_marshal() { | |||
| fmt.Printf("corim: %v", hex.EncodeToString(buf)) | |||
|
|
|||
| // output: | |||
| // corim: d901f5a300f6018158d9d901faa40065656e2d474201a100676578616d706c650281a4006941434d45204c74642e01d8207468747470733a2f2f61636d652e6578616d706c65028101206f3132332046616b652053747265657404a1008182a100a300d90258582061636d652d696d706c656d656e746174696f6e2d69642d303030303030303031016941434d45204c74642e026e526f616452756e6e657220322e3081a200d90259a30162424c0465352e302e35055820acbb11c7e4da217205523ce4ce1a245ae1a239ae3c6bfd9e7871f7e5d8bae86b01a10281820644abcdef00037822687474703a2f2f6578616d706c652e636f6d2f6578616d706c652d70726f66696c65 | |||
| // corim: d901f5a300f60181d901fa58d6a40065656e2d474201a100676578616d706c650281a4006941434d45204c74642e01d8207468747470733a2f2f61636d652e6578616d706c65028101206f3132332046616b652053747265657404a1008182a100a300d90258582061636d652d696d706c656d656e746174696f6e2d69642d303030303030303031016941434d45204c74642e026e526f616452756e6e657220322e3081a200d90259a30162424c0465352e302e35055820acbb11c7e4da217205523ce4ce1a245ae1a239ae3c6bfd9e7871f7e5d8bae86b01a10281820644abcdef00037822687474703a2f2f6578616d706c652e636f6d2f6578616d706c652d70726f66696c65 | |||
There was a problem hiding this comment.
Not for this PR necessarily (as the bug was there before), but we have a problem with this CoRIM: it has a null tag-id, which is invalid. This should’ve been caught by Valid(), no?
There was a problem hiding this comment.
tag-id is being set on line 144. It has a null language, which is valid.
There was a problem hiding this comment.
no, wait, I've confused with comid. You're right. I'm not sure what is happening here...
There was a problem hiding this comment.
Ok, The issue is that UnsignedCorim.ToCBOR doen't validate. I'll submit that as a separate fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The CoRIM spec defines the tags field of corim-map as
where
and
i.e. tags field is supposed to be an array of one or more tagged byte string, which are themselves CBOR encodings of the corresponding struct.
Our
UnsignedCorim.Tagsis defined as[]Tag, whereTagwas an alias of[]byte. It is an array of serialized tag objects (with tag prefixed as part of serialization). However, since it is essentially, an array of byte strings, it was serialized as such, resulting ininstead of the expected
i.e. tag ended up being part of the
bstr, where as it ought to have been wrapping thebstr.This commit changes the definition of
Tagto a struct that associates a tag number with a[]byte(it is similar tocbor.Tagexcept the content is always a[]byterather thanany). In CBOR, this type is serialized exactly as acbor.Tag; in JSON, this includes the tag in the base64-encoded data.BREAKING CHANGE: tags (CoMID, CoSWID, etc) are now serialized as tagged bstr's, rather than plain bstrs (that contained the tag). API-wise, the Tag type has been changed to a struct that associates a tag number with a []byte.