Hi Russ, Inline. On Wed, Aug 17, 2022 at 10:54:04AM -0400, Russ Housley wrote: > Ben: > > Thanks very much for the very careful review. > > You caught one very significant mistake. Thanks very much. > > > On Wed, Jul 20, 2022 at 09:18:16AM -0700, The IESG wrote: > >> > >> The IESG has received a request from the CBOR Object Signing and Encryption > >> WG (cose) to consider the following document: - 'CBOR Object Signing and > >> Encryption (COSE): Countersignatures' > >> <draft-ietf-cose-countersign-06.txt> as Internet Standard > >> > >> The IESG plans to make a decision in the next few weeks, and solicits final > >> comments on this action. Please send substantive comments to the > >> last-call@xxxxxxxx mailing lists by 2022-08-10. Exceptionally, comments may > >> be sent to iesg@xxxxxxxx instead. In either case, please retain the beginning > >> of the Subject line to allow automated sorting. > >> > >> The file can be obtained via > >> https://datatracker.ietf.org/doc/draft-ietf-cose-countersign/ > > > > This draft fills a claer need, providing "actual countersignature" > > semantics for COSE. However, it's also intended to play a second role, > > namely to deprecate the original "countersignature" functionality from RFC > > 8152 (which did not always provide "actual countersignature" semantics). > > > > I think we probably need to expound further on what exactly is done to > > achieve this deprecation: the current version uses the stem "deprecate" > > only twice, once in text that is essentially duplicated from > > draft-ietf-cose-rfc8152bis-struct, and the final time in the IANA > > considerations, where IANA is to add "(Deprecated by [[This Document]]" to > > the existing registrations. (Note: the mismatched parentheses are in the > > original.) There is no discussion of what the deprecation means in > > practice for implementations, which is a rather serious omission. > > First, the missing ")" was corrected in the -07 version. > > Second, the codepoints for "counter signature" and "CounterSignature0" are not removed from the IANA registry so that old counter signatures can be handled. Right, and that seems correct to me. > I suggest that we add the following: > > With the publication of this document, implementers are encouraged to > migrate used of the previous countersignature algorithm to the one > specified in this document. In particular, uses of > "CounterSignature" will migrate to "CounterSignatureV2", and uses of > "CounterSignature0" will migrate to "CounterSignature0V2". Of > course, the validation of "CounterSignature" and "CounterSignature0" > might be supported to handle previously generated counter signatures. The general sense of this looks good to me. For transparency, RFC-to-be 9052 appears to have all needed approvals (including Paul's as AD), and says this (in the context of "crit"): Additionally, the header parameter "counter signature" (label 7) defined by [RFC8152] must be understood by new implementations, to remain compatible with senders that adhere to that document and assume all implementations will understand it. with lowercase "must". So I think the text in this document would better align with 9052 if we s/might be supported/must be supported/. > > > In particular, the current state of affairs gives the COSE header parameter > > with label 7 ("counter signature") privileged treatment in that senders are > > free to assume that recipients can understand and process it, even if it is > > not listed in the "crit" header's value. While a reasonable reader might > > assume that being marked as "deprecated" directs a sender to not omit the > > parameter (though more concrete guidance on that point seems apropos to > > me), I do not think that there is a clear implication about whether an > > impelementation must continue to implement support for processing this > > header parameter. In order to ensure a smooth deprecation process, I think > > this document needs to make a specific statement about whether the > > requirement on implementations to understand this parameter remains. > > (I am currently grappling with how to describe this situation for RFC-to-be > > 9052, that does not discuss "counter signature" but does retain RFC 8152's > > guidance that labels 0-7 SHOULD be omitted from the "crit" value. I > > believe that the requirement on implementations to understand and be able > > to process "counter signature" must remain for the purposes of RFC 9052.) > > We could also warn that over time implementations are expected to stop recognizing "CounterSignature" and "CounterSignature0". That said, i would probably be good to have RFC-to-be 9052 and this document handle this the same way. I support warning that over time we expect implementations to phase out support for the legacy structures. I mostly expect that we would have a new document in a year or two that formally Updates 9052 to ease this requirement (but I also wouldn't mind if we decided to put that change into the current -countersign document). I'm not sure if that aligns with your indicated preference to handle things the same way with 9052 and -countersign. > > > Since I'm posting to last-call, I did make an attempt to read the rest of > > the document, and have some additional remarks. > > > > Prior to sending to the RFC Editor, please do a pass to compare the specific > > wordings used for definitions and other shared text between this document and > > RFC 9052, to avoid needless divergence in phrasing. A few specific areas > > stuck out to me as meriting this comparison (but this should not be assumed > > to be an exhaustive list): the introduction in general, but most notably > > around "to be a schema-free decoder"; the security considerations, and the > > document terminology section. > > Will do. > > The Security Consideration in this document primarily points to RFC-to-be 9052, and then includes a few things explicitly related to counter signatures. Please tell me where you see misalignment. It looks like I was looking at -countersign-05 when I wrote this; in the -05, the first ~80% of the security considerations looked like a copy/paste job from 9052, and I was pointing out that the original source had been edited since the copy was made. Looking at the -06 and -07, that concern seems to no longer be an issue. > > > Section 1 > > > > countersignature, were those that the working group desired, the > > decision was made to remove all of the countersignature text from > > [I-D.ietf-cose-rfc8152bis-struct] and create a new document to both > > deprecate the old countersignature algorithm and to define a new one > > with the desired security properties. > > > > We should probably specifically indicate that the "new document" is this > > one, in some fashion. > > This was reworded a bit, and I think the last sentence addresses your point: > > During the process of advancing COSE to an Internet Standard, it was > noticed that while the description of the security properties of > countersignatures was accurate, the associate specification of their > implementation in Section 4.5 of [RFC8152] was incorrect for the > COSE_Sign1 structure. To remedy this situation, the working group > decided to remove all of the countersignature text from > [I-D.ietf-cose-rfc8152bis-struct], which obsoletes [RFC8152]. This > document defines a new countersignature with the desired security > properties. Yes, that looks good. > > > The new algorithm is designed to produce the same countersignature > > value in those cases where the cryptographic computed value was > > already included. This means that for those structures the only > > thing that would need to be done is to change the value of the header > > parameter. > > > > It's not entirely clear to me on first read that this is a feature. > > Creating ambiguity about how a given ciphertext was created or should be > > processed has historically given rise to vulnerabilities. That said, since > > in this particular case the actual cryptographic content is the same and > > this is the scenario where the construction being deprecated did provide > > the intended "actual countersignature" semantics, maybe there is not really > > any grounds for "confusion" here. > > These are Jim's words, and I would not like to change them unless there is a real need. I cannot supply a "real need" at this time, and thus defer to your preference. > > > Section 3 > > > > timestamp, a time point at which the signature exists. When done on > > a COSE_Sign, this is the same as applying a second signature to the > > payload and adding a parallel signature as a new COSE_Signature is > > the preferred method. > > > > I don't think I understand what this is trying to say, as what it seems to say > > to me doesn't make sense. In particular, this seems to say that the act of > > applying a countersignature to a COSE_Sign structure is functionally > > equivalent to just appending to the "signatures" array of the COSE_Sign > > object, and indeed that appending to "signatures" is preferred. But that > > seems to provide semantics that are not the (desired) semantics of a true > > countersignature, which would cover the existing (primary) signature in > > addition to the content! > > Wow. This error has been here a very long time. Thanks for noticing! > > ... That is, the countersignature makes a > statement about the existence of a signature and, when used as a > timestamp, a point in time at which the signature exists. When a > timestamp is not used, a countersignature on a COSE_Sign is the same > as applying a second signature to the payload; therefore, adding a > parallel signature as a new COSE_Signature is the preferred method. I had to read this several times to convince myself that it makes sense, so let me write out some description of how I resolved it, to try to cross-check my reasoning. Just as a baseline/refresher (since I've confused myself about it in the past), the primary COSE structures for signatures are COSE_Sign and COSE_Sign1; the COSE_Signature structure is a substructure that holds "the signature and information about the signature". When we construct a countersignature on a COSE_Sign, the procedures have us pull into the to-be-signed bytes both the payload and "all bstr fields after the second" (among other things), but since COSE_Sign has only two bstr fields (protected_headers and payload), that means the "other_fields" element is omitted. In particular, the "signatures" member of COSE_Sign is an array and not a bstr, so the other existing signatures in the COSE_Sign are not included in other_fields and thus not covered by the countersignature -- we'd need to apply the countersignature on the COSE_Signature object itself in order to do that. Having made that insight, the focus in the new text about timestamp/no-timestamp does make more sense, and in the no-timestampp case the countersignature's to-be-signed bytes basically match those of a primary signature ("new COSE_Signature [in the COSE_Sign]"), and the recommendation here does make more sense. That said, this seems to be the only place in the document where we mention "timestamp", so I wonder if we want to put some additional discussion somewhere about the mechanics of using a countersignature to add a timestamp (e.g., in external_aad?). > > > NITS > > > > Section 1.2 > > > > CBOR grammar in the document is presented using CBOR Data Definition > > Language (CDDL) [RFC8610]. > > > > singular/plural mismatch > > > > The collected CDDL can be extracted from the XML version of this > > document via the following XPath expression below. (Depending on the > > XPath evaluator one is using, it may be necessary to deal with > > > as an entity.) > > > > //sourcecode[@type='CDDL']/text() > > > > Carsten pointed out for 9052 that the type needs to be lowercase in order to > > work. > > Fixed. > > > > Section 2 > > > > Generic_Headers /= ( > > ? TBD10 => COSE_Countersignature / [+COSE_Countersignature] > > ; V2 Countersignature > > ? TBD11 => COSE_Countersignature0 ; V2 Countersignature0 > > ) > > > > Since only one of the comments fits to the right, I'd suggest making both be > > on their own line, and appearing before the content they describe. > > How about this? > > Generic_Headers /= ( > ; V2 Countersignature > ? TBD10 => COSE_Countersignature / [+COSE_Countersignature] > ; V2 Countersignature0 > ? TBD11 => COSE_Countersignature0 > ) Just what I was imagining. Thanks! -Ben -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
