Subject: [COSE] WGLC

From: Justin Richer <[email-masked]>
Date: Wed, 8 Jun 2016 10:50:02 -0500

Hi everyone,

Has anybody read the draft? Comments, thoughts, snide remarks?

 — Justin

---

From: Carsten Bormann <[email-masked]>
Date: Wed, 08 Jun 2016 18:07:18 +0200

Hi Justin,

expect my full review by the end of this week (and a pull request for
the editorial nits).

(Yes, there is a little work to do still, but so far I'm quite happy.)

Grüße, Carsten

Justin Richer wrote:
> _______________________________________________
> COSE mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/cose
> 
>

---

From: Francesca Palombini <[email-masked]>
Date: Thu, 9 Jun 2016 08:05:19 +0000

Hello,

Carsten and Jim, I just sent a pull request to fix some editorials (btw some were already fixed in the git compared to the ietf one), I may have left some of course.

I am happy with the draft, minor comments:
- the Base IV has been added to the COSE Key, but is not in the CDDL. Is there a reason or it was simply forgotten? (section 7.1)
- I'm not sure what you wanted to say in a sentence in section 11.2, which has some editorial, so I couldn't fix it: "(This practice means if algorithm A is broken and thus can is easier to find, the key derived for algorithm B will not be the same as the key for algorithm B.)"
- Same with this sentence in section 12.4.1 "Since the only the math is changed by changing the curve, the curve is not fixed for any of the algorithm identifiers we define." Maybe fixing the editorial will make it more clear, but right now I don't really understand it.
- Example C.2.1. is missing the comment for the signature

Otherwise, I am satisfied with the draft (including the appendices), thank you for the great work! I appreciated that you introduce the table with the parameters in the beginning of each section. I think it makes it easier to understand at what level those parameters should be used.

One question, maybe I missed it or maybe I lack experience: I see you define in section 15. "Application Profiling Considerations" that an application may define new header parameters; what would be the process to register labels for new parameters?

Francesca

-----Original Message-----
From: COSE [mailto:[email-masked]] On Behalf Of Carsten Bormann
Sent: den 8 juni 2016 18:07
To: Justin Richer <[email-masked]>
Cc: cose <[email-masked]>
Subject: Re: [COSE] WGLC

Hi Justin,

expect my full review by the end of this week (and a pull request for the editorial nits).

Grüße, Carsten

---

From: "Jim Schaad" <[email-masked]>
Date: Thu, 9 Jun 2016 11:33:36 -0700

Thanks for the review.  Comments interspersed below

> From: COSE [mailto:[email-masked]] On Behalf Of Francesca Palombini
> Sent: Thursday, June 09, 2016 1:05 AM
> To: Carsten Bormann <[email-masked]>; Justin Richer <[email-masked]>;
> [email-masked]
> Cc: cose <[email-masked]>
> Subject: Re: [COSE] WGLC
> 
> Hello,
> 
> Carsten and Jim, I just sent a pull request to fix some editorials (btw some were
> already fixed in the git compared to the ietf one), I may have left some of
> course.
> 
> - the Base IV has been added to the COSE Key, but is not in the CDDL. Is there a
> reason or it was simply forgotten? (section 7.1)

That was an oversight.

> - I'm not sure what you wanted to say in a sentence in section 11.2, which has
> some editorial, so I couldn't fix it: "(This practice means if algorithm A is broken
> and thus can is easier to find, the key derived for algorithm B will not be the
> same as the key for algorithm B.)"

The last B should be an A. 

> - Same with this sentence in section 12.4.1 "Since the only the math is changed
> by changing the curve, the curve is not fixed for any of the algorithm identifiers
> we define." Maybe fixing the editorial will make it more clear, but right now I
> don't really understand it.

Does this work better?

The math used to obtain the computed secret is based on the curve selected and not on the ECDH algorithm.
For this reason, a new algorithm does not need to be defined for each of the curves.

Note that I am planning on changing the title of the next bullet to "Computed Secret to Shared Secret" and update the text following accordingly.

> - Example C.2.1. is missing the comment for the signature

Your right - how odd.

> 
> Otherwise, I am satisfied with the draft (including the appendices), thank you for
> the great work! I appreciated that you introduce the table with the parameters
> in the beginning of each section. I think it makes it easier to understand at what
> level those parameters should be used.
> 
> One question, maybe I missed it or maybe I lack experience: I see you define in
> section 15. "Application Profiling Considerations" that an application may define
> new header parameters; what would be the process to register labels for new
> parameters?

This is done by the normal IANA processes.  That is one writes a document of some type and requests that an IANA registration is done in the appropriate registry.   An example of how this would be done can be found in section 16.9.
If one writes the document as an RFC, the request to IANA occurs automatically as part of the workflow of a document.  If one is writing the document outside of the IETF then one needs to send an email to IANA to request the registration.

As part of the IANA registration process, a designated export for the registry (to be determined by the IESG) would review the request and either approve it, suggest changes or deny the registration.   I have not created a template for the registration, but the set of fields to be included in the registration are defined.  Section 16.10 gives general guidance to the reviewers 

Jim

---

From: Francesca Palombini <[email-masked]>
Date: Fri, 10 Jun 2016 09:33:54 +0000

Hi Jim,

Answers inline.

Francesca

-----Original Message-----
From: Jim Schaad [mailto:[email-masked]] 
Sent: den 9 juni 2016 20:34
To: Francesca Palombini <[email-masked]>; 'Carsten Bormann' <[email-masked]>; 'Justin Richer' <[email-masked]>
Cc: 'cose' <[email-masked]>
Subject: RE: [COSE] WGLC

> Carsten and Jim, I just sent a pull request to fix some editorials 
> (btw some were already fixed in the git compared to the ietf one), I 
> may have left some of course.
> 
> - the Base IV has been added to the COSE Key, but is not in the CDDL. 
> Is there a reason or it was simply forgotten? (section 7.1)

That was an oversight.

> - I'm not sure what you wanted to say in a sentence in section 11.2, 
> which has some editorial, so I couldn't fix it: "(This practice means 
> if algorithm A is broken and thus can is easier to find, the key 
> derived for algorithm B will not be the same as the key for algorithm B.)"

[FP] Ok, then that "can is" should probably be "the key is", or something else? just an editorial again.

> - Same with this sentence in section 12.4.1 "Since the only the math 
> is changed by changing the curve, the curve is not fixed for any of 
> the algorithm identifiers we define." Maybe fixing the editorial will 
> make it more clear, but right now I don't really understand it.

Does this work better?

[FP] Yes it does, and I think this is a good title change too.

Your right - how odd.

> 
> Otherwise, I am satisfied with the draft (including the appendices), 
> thank you for the great work! I appreciated that you introduce the 
> table with the parameters in the beginning of each section. I think it 
> makes it easier to understand at what level those parameters should be used.
> 
> One question, maybe I missed it or maybe I lack experience: I see you 
> define in section 15. "Application Profiling Considerations" that an 
> application may define new header parameters; what would be the 
> process to register labels for new parameters?

[FP] Ok. thanks for the explanation!

Jim

---

From: Carsten Bormann <[email-masked]>
Date: Sun, 12 Jun 2016 20:58:57 +0100

Carsten Bormann wrote:
> expect my full review

Below.
This was one of the better drafts I have reviewed in my IETF life.

Grüße, Carsten

The COSE message syntax draft is mostly ready to implement.
Some further editorial work could improve the document.
(Pull request #158 contains some typos and other trivial editorial
improvements.)

# Process

8.2
Are we creating a dependency here on a document that might take
forever to complete?
Maybe mark this section with a "Sollbruchstelle" (~ toilet paper
perforation), so it can be detached into a separate document if that
happens.

# Technical

2. second item list, item 3/4.
The media type system REST adopted from MIME is represented by the
Content-Type header field in HTTP and the Content-Format (!) Option in
CoAP.  (The latter also covers what is Content-Encoding in HTTP.)
Editorial: The text here is about media types, and only then about how
these are represented in the REST protocols.
Technical: CoAP Content-Formats are represented by unsigned integers,
not integers in general (fix header parameter 3, content type).

Related in 3.1:
Technical: HTTP's Content-Type header fields and CoAP Content-Formats
can contain (media type) parameters (like COSE's cose-type), the
definition of header parameter 3 seems to allow this only on the
(unsigned) integer side (Content-Format), not for a tstr.

While discriminator strings such as "Signature", "Signature1", and
"CounterSignature" are cute, maybe a numeric label would be more
appropriate for processing by a constrained implementation.
(I know that these are just processed, not exchanged.  Still.)

If we don't mind spurious stuff in signer input, we should include
"protected" even for COSE_Sign1 (as an empty bstr), simplifying
implementation.

Why are kids always bstr?  It seems natural to allow tstrs here.

key_ops could more naturally be a bit field (CDDL .bits).
And start from 0.

I don't understand the effect of Appendix A.  Does this relieve the
statements earlier of the form "The 'alg' parameter MUST be present."?

(I'll add my ceterum censeo about private-use spaces and the X-Dash
effect.)

# Editorial/technical

While the changes discussed here do not change the technical
substance of the protocol, they may facilitate implementation.

The definition of the "protected" header map fields should use more of
what CDDL can do to define them, see #154.

Please use consistent spacing, "label: value" (preferred) or
consistently "label : value"; that makes searching so much simpler.

Hmm, Sign1, Encrypt1, and Mac0.  Sign1 contains 1 signature.  Encrypt1
contains 0 recipient structures.  Mac0 contains 0 recipient
structures.  Find the odd one out :-)
I think it would be less confusing to have Sign1, Encrypt0, and Mac0.

The counter signature parameter (parameter 7) contains one or more
counter signatures, not just one.

One of the nice things about JOSE is that the various labels have a
defined name, which is then re-used as a variable or field name in an
implementation.  We are not really providing the same level of
service.
(Also cf. the previous paragraph -- this is talking about a parameter
called "counter signature", but that doesn't stand out in that
sentence, so I also referenced the parameter number 7.)
A related problem are sentences such as
           o  The 'kty' field MUST be present and it MUST be 'Symmetric'.
Of course, we mean it must be 4 (and not the string 'Symmetric' in
bstr or tstr form), but it would not help to state this numerically.

10.2.1
          o  The key and nonce pair MUST be unique for every message
encrypted.
(for a single key, that is)

12.2
           o  At a minimum, the 'unprotected' field MUST contain the 'alg'
              parameter and SHOULD contain a parameter identifying the
shared
              secret.
Why is the first a MUST if we can have the second?
(The 'unprotected' field could be attacker-controlled; information
attached to the shared secret cannot be.)

# Editorial

1.1 and the rest of the document don't agree whether it is "MAC
messages" or "MACed messages".  Since the messages do not only contain
a MAC, the second phrasing is more accurate.

1.2 and 1.5 could be combined, as someone searching for terminology
might find one of them and then not the other.

1.4 could possibly be read as completely ruling out non int/tstr map
keys in any CBOR contained in COSE messages; maybe it could be more
explicit that this constraint is on the message structures defined by
this specification.

2., second item list
Maybe introduce local terms "untagged message" and "tagged message" in
item 1 and 2, resp.

3. "Two buckets..."
Maybe add example "(i.e., the byte string h'a0')" to clarify what a
CBOR-serialized empty map is.

I think I understand what the sentence with "finesses" is trying to
say, but it may be confusing. (Same with "vindicated" in 9.1 -- we
probably can agree on the gist of this statement, but at least the
dictionary sense "clear (someone) of blame or suspicion" is not
appropriate here.)

Some of the statements in 3 also are true for key structures, that is
not always clear.

"should perform the same checks that..." -> "should also
check that the same label does not occur in both the protected and
unprotected headers"

4.
"cannot be converted" -- of course they can.  They just lose something
then.  Can we be more explicit what is that what we lose?

4.2
"COSE Single signature messages" -- what is that?

4.3
"if the same relative numbering is kept" -- huh?
(Of course, you wouldn't.)

5.4
(define AE algorithm)

6.3
How is that "encode it as a binary value" done?

6.3
Maybe we can use names like ToBeMaced at the place where the input is
defined so the reference later is clearer. (Twice.)

10.2.1
"the portions encryption" -- Huh?

There are (fortunately few) cases of text that is repeated without
need. PartyVInfo could really just explained as analogous to
PartyUInfo.

12.1.2
unpredictable for whom?

12.2
The "MUST deal" is still not that clear.

12.4 second para, second sentence.
Hmm.  COSE is designed to be usable in that environment, but that
doesn't mean it never can be used in an "on-line" environment (we
actually plan to do this in CoAP "object security").

16.9
The cose-key media types are sometimes called cose-key+cbor and
sometimes cose-key.  If we don't have +cbor on application/cose
itself, why should it be on the key types?

# Editorial, covered in #158

1.
I prefer the abbreviation "IoT"
(IOT is interoperability testing to some of us)

4.4
s/consistent/well-defined/

5.4
s/absent/empty/g

# Other

Where the evolution from RFC 2633 to RFC 5751 is mentioned, maybe
there could be a brief mention that there was another stage (RFC 3851).

---

From: "Jim Schaad" <[email-masked]>
Date: Sun, 12 Jun 2016 19:26:06 -0700

Text changes can be found at https://github.com/cose-wg/cose-spec/pull/160

More comments inline

Jim

> Sent: Sunday, June 12, 2016 12:59 PM
> Are we creating a dependency here on a document that might take forever to
> complete?
> Maybe mark this section with a "Sollbruchstelle" (~ toilet paper perforation), so
> it can be detached into a separate document if that happens.

While it might be that it takes forever to finish, this is not the impression that I have of where the RG chairs want to be.   At worst this might stall the final publication of the RFC but I would be surprised if it was by far.  Also I would note that this is an Informative reference this means that it is possible to publish with a reference to the ID just like we are going to do with the CDDL document.

> The media type system REST adopted from MIME is represented by the Content-
> Type header field in HTTP and the Content-Format (!) Option in CoAP.  (The
> latter also covers what is Content-Encoding in HTTP.)
> Editorial: The text here is about media types, and only then about how these are
> represented in the REST protocols.
> Technical: CoAP Content-Formats are represented by unsigned integers, not
> integers in general (fix header parameter 3, content type).

I think you covered this in your pull request.  If not, what are you looking for.

> 
> Related in 3.1:
> Technical: HTTP's Content-Type header fields and CoAP Content-Formats can
> contain (media type) parameters (like COSE's cose-type), the definition of
> header parameter 3 seems to allow this only on the

Your right it makes sense to allow this.  It is a loss of functionality from the JOSE documents that I missed.

> "CounterSignature" are cute, maybe a numeric label would be more appropriate
> for processing by a constrained implementation.

This was the hardest one for me to think about and respond to.  I can understand why you are saying this.  I wish that could whole heartedly agree but I can't.  I can agree with shortening these strings, but going to integers just seems to me to be asking for potential problems in the future if we start having other structures created, or even the current structures being re-purposed for a different specific purpose.  I also think that strings are easier for people to recognize problems in general.

Not sure yet what to do about this.

> 
> If we don't mind spurious stuff in signer input, we should include "protected"
> even for COSE_Sign1 (as an empty bstr), simplifying implementation.

If we allow this, I would not want to allow it as an empty bstr but as something that would be totally different.  That being the case I don't think that absent is any harder than nil would be.  Making this an absent field might make it slightly harder to implement, but given that you need to deal with the fact that the sources of information is different I don't think that there is a real problem here.

> Why are kids always bstr?  It seems natural to allow tstrs here.

I don't see any real benefit to that.  Allowing for two types now means that you need to worry about what the type is if you are going to display, you need to keep the type for doing comparisons and so forth.  This would seem to make things complicated.  If one wishes to do a textual display one can see if it follows a text constrained format and then do the display if one wishes to.

> key_ops could more naturally be a bit field (CDDL .bits).

That would be true for the set of key ops that we have defined.  However, it would mean that future applications would potentially have a more difficult time creating a key op for their specific purpose.  I would worry that applications would only expect a small value for the bit field and have problems with larger values.  This is not a problem with the current method.

> 
> I don't understand the effect of Appendix A.  Does this relieve the statements
> earlier of the form "The 'alg' parameter MUST be present."?

That would be for a specific application to state.  But in the generic case it does not.

> While the changes discussed here do not change the technical substance of the
> protocol, they may facilitate implementation.
> 
> The definition of the "protected" header map fields should use more of what
> CDDL can do to define them, see #154.
> 
> Please use consistent spacing, "label: value" (preferred) or consistently "label :
> value"; that makes searching so much simpler.
Done - although I disagree on the preferred version.

> 
> Hmm, Sign1, Encrypt1, and Mac0.  Sign1 contains 1 signature.  Encrypt1 contains
> 0 recipient structures.  Mac0 contains 0 recipient structures.  Find the odd one
> out :-) I think it would be less confusing to have Sign1, Encrypt0, and Mac0.

Yes you are right

> 
> The counter signature parameter (parameter 7) contains one or more counter
> signatures, not just one.
> 
> One of the nice things about JOSE is that the various labels have a defined name,
> which is then re-used as a variable or field name in an implementation.  We are
> not really providing the same level of service.
> (Also cf. the previous paragraph -- this is talking about a parameter called
> "counter signature", but that doesn't stand out in that sentence, so I also
> referenced the parameter number 7.) A related problem are sentences such as
> Of course, we mean it must be 4 (and not the string 'Symmetric' in bstr or tstr
> form), but it would not help to state this numerically.

Not sure how to address this comment.  I can be more proactive about making sure that the single quotes are used everywhere that it is the name of a parameter field rather than a concept and mark that in the terminology section up top.  Is that going to be enough or do you have a more solid proposal?

> 
> 10.2.1
>           o  The key and nonce pair MUST be unique for every message encrypted.

I am not sure what you are asking for here.  It would be equally true to have the note "(for a single nonce, that is)".

>               parameter and SHOULD contain a parameter identifying the shared
>               secret.
> (The 'unprotected' field could be attacker-controlled; information attached to
> the shared secret cannot be.)

The first falls out from the requirement that an 'alg' parameter always be present in a message.   One could also use this in the absence of a 'kid' to filter out the set of keys to be used.

The second would be insufficient if the same secret could be used for multiple algorithms.  I.e. it could be used for both a key wrap algorithm in a recipient structure and as a CEK for an Encrypt0 message with a different algorithm.

In both cases you are going to end up in a situation of having a denial of service attack if either value was changed by an attacker.  

> 
> # Editorial
> 
> 1.1 and the rest of the document don't agree whether it is "MAC messages" or
> "MACed messages".  Since the messages do not only contain a MAC, the second
> phrasing is more accurate.

That makes sense

> 
> 1.2 and 1.5 could be combined, as someone searching for terminology might
> find one of them and then not the other.
> 
> 1.4 could possibly be read as completely ruling out non int/tstr map keys in any
> CBOR contained in COSE messages; maybe it could be more explicit that this
> constraint is on the message structures defined by this specification.

I think you are probably being overly paranoid, however I made some changes for this

> 
> 2., second item list
> Maybe introduce local terms "untagged message" and "tagged message" in item
> 1 and 2, resp.

I have identified both of these terms in the 2nd bullet.  This makes me wonder if I should swap the order of bullets 1 and 2.

> 
> 3. "Two buckets..."
> Maybe add example "(i.e., the byte string h'a0')" to clarify what a CBOR-
> serialized empty map is.

That makes sense

> 
> I think I understand what the sentence with "finesses" is trying to say, but it may
> be confusing. (Same with "vindicated" in 9.1 -- we probably can agree on the gist
> of this statement, but at least the dictionary sense "clear (someone) of blame or
> suspicion" is not appropriate here.)

The dictionary also has "to show that (someone or something that has been criticized or doubted) is correct, true, or reasonable" for this as well.  But I can see the point.

> 
> Some of the statements in 3 also are true for key structures, that is not always
> clear.

If this is true, then those statements should be echoed in section 7 as well.  Which statements do you think are missing there.

> 
> "should perform the same checks that..." -> "should also check that the same
> label does not occur in both the protected and unprotected headers"

Yes, that works better

> 
> 4.
> "cannot be converted" -- of course they can.  They just lose something then.
> Can we be more explicit what is that what we lose?

Done

> "COSE Single signature messages" -- what is that?

Done

> (Of course, you wouldn't.)

On a strictly naïve implementer basis, I am willing to bet that this might be one of the most common bugs that is going to pop up when testing.  If you copy over the correct set of options in the sender and then do the same in the validator 90+ percent of the time it is going to be just fine and work just great.  Only if the set of options actually gets changed during the testing process is it going to be a problem.

> (define AE algorithm)

Done

> How is that "encode it as a binary value" done?

Missed that one when the other two were re-written

> defined so the reference later is clearer. (Twice.)

done

> "the portions encryption" -- Huh?

Cleaned up

Section 11.2
> There are (fortunately few) cases of text that is repeated without need.
> PartyVInfo could really just explained as analogous to PartyUInfo.

Given that I pulled out the PartyInfo structure, it makes sense to cut this down even farther.

> unpredictable for whom?

I don't think it matters who it is unpredictable for, but I would consider there to be a distinction between random and a "predictable" but random method such as encrypting a counter which is incremented.

> The "MUST deal" is still not that clear.

The following text seems to be to be clear what this means.  Can you elaborate on what you think is not clear?

> Hmm.  COSE is designed to be usable in that environment, but that doesn't mean
> it never can be used in an "on-line" environment (we actually plan to do this in
> CoAP "object security").

I think of the way you are using it more as a store-and-forward without necessarily having a separate store entity.  After all, the protocol that you are outlining would work just as well in a true store-and-forward environment as well. However, I can also think of it as being an 'on-line' protocol as well.  These are the terms that I have normally used for distinguishing between message based protocols and something like TLS/DTLS which is more session based.  Do you have a suggestion for a better pair of contrasting terms for this?

> 
> 16.9
> The cose-key media types are sometimes called cose-key+cbor and sometimes
> cose-key.  If we don't have +cbor on application/cose itself, why should it be on
> the key types?

I had a reason for doing it this way, but I cannot seem to find a historical note that I left myself about why I removed the +cbor for one of them and not for the other.  It might have some vague reasoning that I was thinking of the possibility of a different encoding being used for the key structures but not for the message structures but I can't think of why that would have been.

> Where the evolution from RFC 2633 to RFC 5751 is mentioned, maybe there
> could be a brief mention that there was another stage (RFC 3851).
>

---

From: Ludwig Seitz <[email-masked]>
Date: Mon, 13 Jun 2016 07:58:11 +0200

Hello,

I spotted two typos in one sentence of 8.2.1.

"Signing and non-batch signature verification are deterministic 
operations and do not nee random nuber of any kind."

"need" and "number"

/Ludwig

-- 
Ludwig Seitz, PhD
SICS Swedish ICT AB
Ideon Science Park
Building Beta 2
Scheelevägen 17
SE-223 70 Lund

Phone +46(0)70-349 92 51
http://www.sics.se

---

From: Mike Jones <[email-masked]>
Date: Mon, 13 Jun 2016 06:44:30 +0000

I spotted those too.  They're actually already fixed in the GitHub version.

-----Original Message-----
From: COSE [mailto:[email-masked]] On Behalf Of Ludwig Seitz
Sent: Sunday, June 12, 2016 10:58 PM
To: [email-masked]
Subject: Re: [COSE] WGLC

Hello,

"Signing and non-batch signature verification are deterministic operations and do not nee random nuber of any kind."

"need" and "number"

/Ludwig

--
Ludwig Seitz, PhD
SICS Swedish ICT AB
Ideon Science Park
Building Beta 2
Scheelevägen 17
SE-223 70 Lund

Phone +46(0)70-349 92 51
http://www.sics.se