Drop top level tagging requirement #337

deeglaze · 2024-10-23T18:12:02Z

Tagged type choices are not typical.
I would go so far as to drop the 500 tag as the entrypoint to CoRIM altogether. NVIDIA is creating CoRIMs this way, but they are using a different content-type in the protected header. I think we can drop it in an follow-up.

This patch drops

the need to tag the type choice
the extensibility of concise-rim-type-choice, since extensibility is governed by a profile, and the profile is not known at this point in parsing.
the need to tag the signed corim, since it is a COSE-sign1 with an unambigiuous content-type, and COSE-sign1 already has its own tag.

Addresses Issue #333, but 500 removal is TBD.

Tagged type choices are not typical. I would go so far as to drop the 500 tag as the entrypoint to CoRIM altogether. NVIDIA is creating CoRIMs this way, but they are using a different content-type in the protected header. I think we can drop it in an follow-up. This patch drops * the need to tag the type choice * the extensibility of concise-rim-type-choice, since extensibility is governed by a profile, and the profile is not known at this point in parsing. * the need to tag the signed corim, since it is a COSE-sign1 with an unambigiuous content-type, and COSE-sign1 already has its own tag. Addresses Issue ietf-rats-wg#333, but 500 removal is TBD. Signed-off-by: Dionna Glaze <[email protected]>

Fixed PR so that it builds.

nedmsmith · 2024-10-23T18:53:43Z

This PR probably should have some examples that tests the new functionality.

The section "Concise Reference Integrity Manifest (CoRIM)" may need to be updated to describe the possibility for unsigned payloads.

deeglaze · 2024-10-25T03:54:18Z

Added a small change to the section about the specific #6.501 tag.

thomas-fossati · 2024-10-30T10:59:37Z

Note that this PR would allow four different tags (500, 501, 502, 18) instead of one (500) to represent a top-level CoRIM.

deeglaze · 2024-10-30T12:38:25Z

Note that this PR would allow four different tags (500, 501, 502, 18) instead of one (500) to represent a top-level CoRIM.

I think this is the pattern for CBOR though. There’s no single top level tag for COSE objects either.

500 and 502 are for theoretical encoding options that I don’t understand the justification for either.

thomas-fossati · 2024-10-30T12:54:16Z

Note that this PR would allow four different tags (500, 501, 502, 18) instead of one (500) to represent a top-level CoRIM.

I think this is the pattern for CBOR though. There’s no single top level tag for COSE objects either.

500 and 502 are for theoretical encoding options that I don’t understand the justification for either.

In fact, I'd drop 500 for sure, and may also be convinced to drop 502 if offered one or two pints of Guinness :-)

nedmsmith · 2024-10-30T13:45:07Z

Would the proposed solution allow both 500(signed-corim) and 502(signed-corim)? Would that be confusing?

corim = (tagged-concise-rim-type-choice / concise-rim-type-choice)
tagged-concise-rim-type-choice = #6.500(concise-rim-type-choice)
tagged-signed-corim = #6.502(signed-corim)
concise-rim-type-choice /= tagged-corim-map
concise-rim-type-choice /= tagged-signed-corim
concise-rim-type-choice /= signed-corim
tagged-corim-map = #6.501(corim-map)
tagged-signed-corim = #6.502(signed-corim)
signed-corim = #6.18(COSE-Sign1-corim)
COSE-Sign1-corim = [
  protected: bstr .cbor protected-corim-header-map
  unprotected: unprotected-corim-header-map
  payload: bstr .cbor (tagged-corim-map / corim-map)
  signature: bstr
]

If #6.500 is dropped then wouldn't corim be defined as:

corim = concise-rim-type-choice
concise-rim-type-choice /= #6.501(corim-map)
concise-rim-type-choice /= #6.502(signed-corim)

If 502 is used, then the cose payload would use corim-map instead of tagged-corim-map?

deeglaze · 2024-10-30T14:07:18Z

We can say 502 is for signing envelopes that don't have their own CBOR tag or need further disambiguation.

500 is generally a confusing representation choice, so I think we've lost that one..
I can be convinced to have the payload always be 501-tagged for simplicity, it'll just invalidate cocli-signed CoRIMs. That's fine.

thomas-fossati · 2024-10-30T14:25:21Z

We can say 502 is for signing envelopes that don't have their own CBOR tag or need further disambiguation.

500 is generally a confusing representation choice, so I think we've lost that one.. I can be convinced to have the payload always be 501-tagged for simplicity, it'll just invalidate cocli-signed CoRIMs. That's fine.

The easiest, and most consistent, would be to always require 501 (even when found in 502) and 502.

deeglaze · 2024-10-30T14:53:07Z

Am I to understand that the MUST language in sec-corim-signed is not intended, and 502 is an anything-goes tag so long as the protected payload is a 501-tagged CBOR-encoded corim-map?

thomas-fossati · 2024-10-30T15:02:07Z

Am I to understand that the MUST language in sec-corim-signed is not intended,

Which MUST-language are you referring to?

and 502 is an anything-goes tag so long as the protected payload is a 501-tagged CBOR-encoded corim-map?

Not sure, but at the moment we seem to have an inconsistency between CDDL and prose. In S4.2 we say: "payload: A CBOR encoded tagged CoRIM.", whilst the CDDL allows a "naked" corim-map alongside the tagged one. Besides, "CoRIM" should be "corim-map".

yogeshbdeshpande · 2024-10-30T15:43:25Z

It was concluded in CoRIM Meeting 30th october to drop tag for #6.500 and propose the same change in TCG.

Also remove type extensibility from top level $concise-rim-type-choice, i.e. make it
only: concise-rim-type-choice

deeglaze · 2024-11-20T15:28:50Z

We should return to this when the DICE errata has been accepted to remove the top level tags as well.

deeglaze · 2024-12-18T15:21:12Z

cddl/cbor-tags.txt

 tagged-corim-map = #6.501(corim-map)
-tagged-signed-corim = #6.502(signed-corim)
+tagged-signed-corim = #6.502($signed-corim)


I think we've agreed to drop the extensibility of the signing format.

deeglaze · 2024-12-18T15:27:03Z

draft-ietf-rats-corim.md

@@ -259,7 +259,7 @@ For more detail, see {{sec-corim-profile-types}}.

 A CoRIM can be signed ({{sec-corim-signed}}) using COSE Sign1 to provide end-to-end security to the CoRIM contents.
 When CoRIM is signed, the protected header carries further identifying information about the CoRIM signer.
-Alternatively, CoRIM can be encoded as a CBOR-tagged payload ({{sec-corim-map}}) and transported over a secure channel.
+Alternatively, CoRIM can be encoded as a #6.501 CBOR-tagged payload ({{sec-corim-map}}) and transported over a secure channel.


TODO: Drop 500, 502, deprecate 501 but allow it for the TCG spec conformance.

We change the IANA request to have a single content type, application/rim+cbor.

TODO: Drop 500, 502, deprecate 501 but allow it for the TCG spec conformance.

We change the IANA request to have a single content type, application/rim+cbor.

IIUC, the gist of the proposed way forward is:

media type CBOR tag data item

application/rim+cbor 501 tagged-corim-map

application/rim+cbor 18 signed-corim

This looks asymmetrical because in the case of 18, absent a media type indication from the transport/wrapping channel, the parser needs to unpack the potential signed-corim, unpack the protected header, fetch the content-type element value, and make sure it's application/rim+cbor.

I suggest instead (as I did in #337 (comment)) to keep both 501 and 502.

The media type for signed-corim would be application/cose. The unprotected header for the payload's media type would be application/rim+cbor.

Tagged type choices are not typical. I would go so far as to drop the 500 tag as the entrypoint to CoRIM altogether. NVIDIA is creating CoRIMs this way, but they are using a different content-type in the protected header. I think we can drop it in an follow-up. This patch drops the need to tag the type choice the extensibility of concise-rim-type-choice, since extensibility is governed by a profile, and the profile is not known at this point in parsing. the need to tag the signed corim, since it is a COSE-sign1 with an unambigiuous content-type, and COSE-sign1 already has its own tag. Addresses Issue ietf-rats-wg#333, but 500 removal is TBD.

deeglaze · 2024-12-18T20:02:48Z

Discussed in our meeting that we're going with 1 content type and dropping 500 and 502.

CI doesn't have envsubst or openssl tools.

deeglaze · 2024-12-18T23:22:52Z

@nedmsmith @henkbirkholz @thomas-fossati @yogeshbdeshpande @andrew-draper please take a look.

thomas-fossati · 2025-01-03T11:57:33Z

cddl/Makefile

+# Commented since CI doesn't have openssl
+#examples/sig-structure.diag: examples/sig-structure.diag.tmpl examples/payload-corim-4.diag examples/protected-header-map.diag
+#	payload="$$(cat examples/payload-corim-4.diag)" \
+#	protected="$$(cat examples/protected-header-map.diag)" \
+#	envsubst < examples/sig-structure.diag.tmpl > examples/sig-structure.diag
+
+#examples/testkey.pem:
+#	openssl ecparam -name secp384r1 -genkey -noout -out examples/testkey.pem
+
+#examples/corim-4.sig: examples/sig-structure.cbor examples/testkey.pem
+#	openssl dgst -sha384 -sign examples/testkey.pem -out examples/corim-4.sig examples/sig-structure.cbor
+
+#examples/corim-4.diag: examples/corim-4.sig examples/corim-4.diag.tmpl examples/payload-corim-4.diag examples/protected-header-map.diag
+#	payload="$$(cat examples/payload-corim-4.diag)" \
+#	protected="$$(cat examples/protected-header-map.diag)" \
+#	signature="h'$$(cat examples/corim-4.sig | xxd -p -c 128)'" \
+#	envsubst < examples/corim-4.diag.tmpl > examples/corim-4.diag
+


This bit is important: we need complete, signed examples too.

Please make this conditional instead.

I suggest adding a check for the openssl CLI in cddl/tools.mk and then a corresponding ifeq ($(strip $(openssl)),) here.

(You probably need a check for envsubst too.)

Besides, add the autogenerated files to the CLEANFILES variable.

PS: maybe this is worth a separate PR.

thomas-fossati · 2025-01-03T12:02:19Z

cddl/cbor-tags.txt

@@ -16,3 +14,4 @@ tagged-cert-thumbprint-type = #6.559(digest)
 tagged-bytes = #6.560(bytes)
 tagged-cert-path-thumbprint-type = #6.561(digest)
 tagged-pkix-asn1der-cert-type = #6.562(bstr)
+tagged-integral-predicate= #6.563(integral-predicate)


This looks spurious.

thomas-fossati · 2025-01-03T12:06:12Z

cddl/examples/corim-4.diag

+  / protected / << / header_map / {
+  / alg: / 1 : -35,
+  / content-type: / 3 : "application/rim+cbor",
+  / corim-meta: / 8 : << / corim-meta / {
+    / signer: / 0 : / corim-signer-map / {
+      / signer: / 0 : "ACME Ltd."
+    }
+  } >>,
+  / kid: / 4 : h'f8ccd2b49fdba32cd94498030fdc8e5010358919'
+} >>,


Indentation needs some care

thomas-fossati · 2025-01-03T12:09:25Z

cddl/examples/payload-corim-4.diag

@@ -0,0 +1,46 @@
+501(/ corim-map / {


send the corim-map to L2

Also, ts=2 et

I don't understand your second comment. ts=2?

thomas-fossati · 2025-01-03T12:17:16Z

draft-ietf-rats-corim.md

-
-Subtype name:
-: `corim-unsigned+cbor`
+: `rim+cbor`


Two observations:

The registration is now invalid: we cannot just drop all the registration template fields except the subtype name;

Let's call it corim rather than rim - we don't want to create confusion with TCG's "Reference Integrity Manifest (RIM)."

I didn't see that application/rim+cbor was taken. What made it invalid?

Let's call it corim rather than rim - we don't want to create confusion with TCG's "Reference Integrity Manifest (RIM)."

There are a variety of TCG specs that refer to themselves as "rim", but I don't believe any have registered media types. The DICE Endorsement Architecture spec (that defines corim) includes "application/rim+cbor" which is published (but didn't yet request a media type - this could be done immediately if necessary).
TCG regards the published spec as having priority over subsequent specs. But it really comes down to what's registered by IANA.
Pragmatically, should the TCG corim and the IETF corim have different media types. For any given release of the specs one spec may be a subset of the other's functionality (due to processing idiosyncrasies). Can they both have the same media type name? Conversely, when the specs are aligned, but they have different media type names - is that desirable?
Note that TCG DICE Endorsement appendices are not normative. The normative expressions are cddl-like (but not cddl). The intent was the I-D would make the cddl normative. If there are separate media types for each, then that seems to say the TCG should create normative cddl.

draft-ietf-rats-corim.md

…rats-corim into pr/337

nedmsmith

I basically approve but need to resolve thread related to which media type string to request.

Readd trailing whitespace.

Change 4 spaces to \t

deeglaze requested review from yogeshbdeshpande, nedmsmith and henkbirkholz October 23, 2024 18:12

Update corim.cddl

630b205

Fixed PR so that it builds.

deeglaze force-pushed the topleveltag branch 2 times, most recently from 00f2fe3 to 74cd2da Compare October 25, 2024 03:36

deeglaze requested a review from thomas-fossati as a code owner October 25, 2024 03:36

deeglaze force-pushed the topleveltag branch from 74cd2da to b450f6d Compare October 25, 2024 03:53

deeglaze force-pushed the topleveltag branch from b450f6d to f0c91e5 Compare October 25, 2024 04:07

deeglaze commented Dec 18, 2024

View reviewed changes

deeglaze added 3 commits December 18, 2024 19:29

Fix CoRIM examples with CDDL changes.

d401fbf

Create signed example in Makefile.

ae65445

deeglaze force-pushed the topleveltag branch from 7f35ed6 to ae65445 Compare December 18, 2024 20:02

Remove signing from Makefile

e11242b

CI doesn't have envsubst or openssl tools.

deeglaze mentioned this pull request Dec 24, 2024

Content Type in COSE_Sign1 out of spec veraison/corim#132

Open

thomas-fossati reviewed Jan 3, 2025

View reviewed changes

thomas-fossati mentioned this pull request Jan 3, 2025

Cut #6.500, #6.502 in FromCOSE veraison/corim#133

Merged

nedmsmith reviewed Jan 7, 2025

View reviewed changes

draft-ietf-rats-corim.md Outdated Show resolved Hide resolved

Merge branch 'topleveltag' of https://github.com/deeglaze/draft-ietf-…

cfba300

…rats-corim into pr/337

nedmsmith approved these changes Jan 7, 2025

View reviewed changes

deeglaze added 5 commits January 8, 2025 07:12

Update cbor-tags.txt

89e5419

Update corim-4.diag

6e77393

Update payload-corim-4.diag

a4c2f02

Update draft-ietf-rats-corim.md

9dc0fa8

Readd trailing whitespace.

Update draft-ietf-rats-corim.md

67aaaa4

Change 4 spaces to \t

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop top level tagging requirement #337

Drop top level tagging requirement #337

deeglaze commented Oct 23, 2024

nedmsmith commented Oct 23, 2024 •

edited

Loading

deeglaze commented Oct 25, 2024

thomas-fossati commented Oct 30, 2024

deeglaze commented Oct 30, 2024

thomas-fossati commented Oct 30, 2024

nedmsmith commented Oct 30, 2024

deeglaze commented Oct 30, 2024 •

edited

Loading

thomas-fossati commented Oct 30, 2024

deeglaze commented Oct 30, 2024

thomas-fossati commented Oct 30, 2024

yogeshbdeshpande commented Oct 30, 2024

deeglaze commented Nov 20, 2024

deeglaze Dec 18, 2024

deeglaze Dec 18, 2024

thomas-fossati Jan 3, 2025 •

edited

Loading

deeglaze Jan 8, 2025

deeglaze commented Dec 18, 2024

deeglaze commented Dec 18, 2024

thomas-fossati Jan 3, 2025

thomas-fossati Jan 3, 2025

deeglaze Jan 8, 2025

thomas-fossati Jan 3, 2025

deeglaze Jan 8, 2025

thomas-fossati Jan 3, 2025

deeglaze Jan 8, 2025

thomas-fossati Jan 3, 2025 •

edited

Loading

nedmsmith Jan 6, 2025

nedmsmith Jan 6, 2025 •

edited

Loading

nedmsmith left a comment

media type	CBOR tag	data item
`application/rim+cbor`	501	`tagged-corim-map`
`application/rim+cbor`	18	`signed-corim`

Drop top level tagging requirement #337

Are you sure you want to change the base?

Drop top level tagging requirement #337

Conversation

deeglaze commented Oct 23, 2024

nedmsmith commented Oct 23, 2024 • edited Loading

deeglaze commented Oct 25, 2024

thomas-fossati commented Oct 30, 2024

deeglaze commented Oct 30, 2024

thomas-fossati commented Oct 30, 2024

nedmsmith commented Oct 30, 2024

deeglaze commented Oct 30, 2024 • edited Loading

thomas-fossati commented Oct 30, 2024

deeglaze commented Oct 30, 2024

thomas-fossati commented Oct 30, 2024

yogeshbdeshpande commented Oct 30, 2024

deeglaze commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deeglaze commented Dec 18, 2024

deeglaze commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nedmsmith Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

nedmsmith left a comment

Choose a reason for hiding this comment

nedmsmith commented Oct 23, 2024 •

edited

Loading

deeglaze commented Oct 30, 2024 •

edited

Loading

thomas-fossati Jan 3, 2025 •

edited

Loading

thomas-fossati Jan 3, 2025 •

edited

Loading

nedmsmith Jan 6, 2025 •

edited

Loading