fix!: unsigned CoRIMs always tagged #187
Conversation
Update ToCBOR/FromCBOR for UnsignedCorim to add/expect tag 501. Current spec requires that unsigned CoRIMs are always tagged with tag 501, both, when appearing as stand-alone CBOR documents, and when embedded as payloads of signed CoRIMs. Note: There was already TaggedUnsignedCorim type registered under tagged 501, however it was not being used and its marshalling didn't work correctly due to the custom To/FromCBOR in UnsignedCorim used in lieu of the standard marshalling mechanism. Rather than (a) fixing its serialization, (b) forwarding all UnsignedCorim methods, and (c) replacing UnsignedCorim usaged with TaggedUnsignedCorim all over the place, TaggedUnsignedCorim was simply removed, and tagging was added to CBOR serialization of UnsignedCorim, as it's not really valid for it to appear untagged, and this minimizes API impact (and is also consistent with SignedCorim that serializes tagged). BREAKING CHANGE: serialized unsigned CoRIMs now start with tag 501. (This is an ABI-only break; all APIs remain the same). Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
| @@ -74,11 +76,15 @@ func UnmarshalSignedCorimFromCBOR(buf []byte) (*SignedCorim, error) { | |||
| // the data, they will be registered with the UnsignedCorim before it is | |||
| // unmarshaled. | |||
| func UnmarshalUnsignedCorimFromCBOR(buf []byte) (*UnsignedCorim, error) { | |||
| if !bytes.Equal(buf[:3], UnsignedCorimTag) { | |||
There was a problem hiding this comment.
Should the comment on Line 74 not clarify that the CBOR payload is now expected to be tagged?
// UnmarshalUnsignedCorimFromCBOR unmarshals a tagged UnsignedCorim from provided
// CBOR data.
There was a problem hiding this comment.
"tagged" is implied. It's not valid for an unsigned corim not to be tagged.
| Q7vjfy5hSzOu01PP8UKLFg== | ||
| 2: | ||
| - 0: ACME Ltd. | ||
| tag: 501 |
There was a problem hiding this comment.
line 1 presumably should say
minimalist tagged unsigned-corim that embeds a comid.PSARefValJSONTemplate
There was a problem hiding this comment.
"tagged" is implied. It's not valid for a CoRIM not to be tagged, so the distinction does not needs to be made ( just like we don't say "tagged signed corim".
| @@ -1,66 +1,68 @@ | |||
| # minimalist unsigned-corim that embeds comid.PSARefValJSONTemplate | |||
There was a problem hiding this comment.
line 1 presumably should say
minimalist tagged unsigned-corim that embeds a comid.PSARefValJSONTemplate
Update ToCBOR/FromCBOR for UnsignedCorim to add/expect tag 501.
Current spec requires that unsigned CoRIMs are always tagged with tag 501, both, when appearing as stand-alone CBOR documents, and when embedded as payloads of signed CoRIMs.
Note: There was already TaggedUnsignedCorim type registered under tagged 501, however it was not being used and its marshalling and didn't work correctly due to the custom To/FromCBOR in UnsignedCorim used in lieu of the standard marshalling mechanism. Rather than (a) fixing its serialization, (b) forwarding all UnsignedCorim methods, and (c) replacing UnsignedCorim usaged with TaggedUnsignedCorim all over the place, TaggedUnsignedCorim was simply removed, and tagging was added to CBOR serialization of UnsignedCorim, as it's not really valid for it to appear untagged, and this minimizes API impact (and is also consistent with SignedCorim that serializes tagged).
BREAKING CHANGE: serialized unsigned CoRIMs now start with tag 501. (This is an ABI-only break; all APIs remain the same).
This addresses #131