Jim - I have updated https://github.com/cose-wg/cose-spec/pull/161 to remove
the two proposed changes that you disagreed with. Unless you weren't done
reading the pull request changes, this should now be ready to merge.
-- Mike
From: Mike Jones
Sent: Sunday, June 12, 2016 11:43 PM
To: [email protected]
Subject: WGLC comments from Mike Jones on draft-ietf-cose-msg-12
I'll say up front that this spec is much improved since the last time I did a
full read through. Good progress.
Substantive comments meriting working group visibility are in this note.
Corrections to grammatical, punctuation, and consistency nits are contained in
the accompanying GitHub pull request,
https://github.com/cose-wg/cose-spec/pull/161, which (lightly) touches 160
lines.
====
Document Title:
Change "CBOR Encoded Message Syntax" to "CBOR Object Signing and Encryption
(COSE)" so that the title contains and explains the intended acronym for the
work.
1.4. CBOR Related Terminology
Please change "Applications can either fail processing or process messages with
incorrect labels, however they MUST NOT create messages with incorrect labels"
to "Applications MUST fail processing for messages with incorrect labels and
they MUST NOT create messages with incorrect labels". The current wording is a
major security hole in a security specification. Please close the hole!
Also, please consider moving this important semantic requirement to a section
other than the terminology section, in which it is likely to be overlooked.
2. Basic COSE Structure
Readers could be misled by the very general wording of this section into
thinking that COSE_Key structures are COSE Messages, as defined in this
section. Saying explicitly that a COSE_Key is not a COSE Message would help
eliminate this potential source of confusion.
One of the places that I was confused in this way was the text "The following
CDDL fragment identifies all of the top level messages defined in this
document". And yet even though COSE_Key is a top-level data structure, it
wasn't in the list.
3. Header Parameters
In "Applications SHOULD perform the same checks that the labels appearing in
the protected and unprotected headers are unique as well", change the "SHOULD"
to "MUST". This is the same security hole as in 1.4, which needs to be closed.
Then delete the then-superfluous sentence: "If the message is not rejected as
malformed, attributes MUST be obtained from the protected bucket before they
are obtained from the unprotected bucket."
The following sentence seems garbled or as if it's missing word(s) near
"generic": "It uses forward references to a group definition of headers for
generic and algorithms." Please revise.
In the following text, I can't tell what "; Algorithm_Headers," is intended to
mean. Is it a comment on either the previous or subsequent line? If so, maybe
move it onto the line.
header_map = {
Generic_Headers,
; Algorithm_Headers,
* label => values
}
4.4. Signing and Verification Process
In the following sentence, I can't work out what "the appropriate authorization
is done" refers to: "In addition to performing the signature verification, one
must also perform the appropriate checks to ensure that the key is correctly
paired with the signing identity and that the appropriate authorization is
done." Please clarify.
5.3. Encryption Algorithm for AEAD algorithms
This instruction is ambiguous because it doesn't say how the key is
represented: "For recipients of the message, recursively perform the encryption
algorithm for that recipient, using the encryption key as the plain text". I
suspect "encryption key" should be changed to "encryption key represented as a
COSE_Key structure". This same language also occurs in 5.4.
7.1. COSE Key Common Parameters
Change either the order of the parameters in Table 3 or the order of their
descriptions afterwards so that they are presented in the same order in both
places.
11.2. Context Information Structure
In the AlgorithmID description, it should probably say that the AlgorithmID is
represented in the same manner as the 'alg' header parameter.
12.2. Key Wrapping
Similarly to the ambiguity in 5.3 and 5.4, the text "The plain text to be
encrypted is the key from next layer down" doesn't say how the key is
represented. In this case, I think it's a raw symmetric key that's encrypted,
right? Please say what it is.
A.2. Counter Signature Without Headers
Should the "counter signature w/o headers | 9 | bstr" value in Table 27 be
added to the appropriate registry? If the answer is "no", please change "9" to
"TBD".
Thanks,
-- Mike
_______________________________________________
COSE mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/cose
