[Lake] spt review of echoc-12

[Sean Turner <[e-mail]>]

Like others I agreed to review the edhoc I-D at the last meeting. I did not have a chance to make sure that I am not repeating comments from others. To save the WG some time, I tried to categorize these along the lines of questions/nits where discuss might need some f2f time and nits, I believe, can be dealt with by the authors as they see fit.

Disclaimers: 0) I am not a cryptographer or security researcher. 1) It is entirely possible I am entirely missing the plot/point. 

Apologies: I ran out of time to submit these as Issues/PRs. I can do so after the interim, including those that got rejected at the 12/15 interim for posterity sake. 

s1.2 (nit): If s1.2 is supposed to be an applicability statement in the context of RFC 2026, can we call it that. i.e., r/Use of EDHOC/Applicability Statement. 

revised: And then, I see section 3.9. Maybe instead of renaming s1.2, a forward pointer to s3.9 is all that is needed to say that an applicability statement is required?

s2, 2nd para (nit): refer to both because it’s (D)TLS and not just TLS?  I.e.,
s/(D)TLS 1.3 [RFC8446]/(D)TLS 1.3 [RFC8446] [I-D.ietf-tls-dtls13]

s2, 2nd para (nit): I think you can safely drop the 2nd reference to IKEv2
s/like IKEv2 [RFC7296],/like IKEv2,

s3.2/s3.5.2 (question): Smarter people than I have devised plans to allow mixing of different METHODs. How does this impact the key usage extension present in certificates? In other words, are METHODS 1 and 2 suggesting that a DS and DH key be used together and does that violate any kind of restrictions on the key usage settings for certificates?

s3.4 (nit): Is transport of error messages, as noted by s6 (2nd para), needed in the list.

s3.5 para after the bullets (nit): I think you are trying to say the credentials have to be the same type, i.e., you can’t have PGP one one side and x.509 on the other. I am not sure that the following is the best wording:

s/Identical authentication credentials need to be established in both endpoints to be able to verify integrity./The same type of authentication credentials are needed baby the endpoints to be able to verify integrity.

s3.5.1, 1st para (nit): At first I thought for sure this had to be a MUST, but it says or application so I am not sure:

The EDHOC implementation or the application must enforce information about the intended endpoint …

s3.5.1 (nit): expand on first use NAI & EUI

s3.5.1, 1st bullet (nit): Often we refer to this a Proof-of-Possession so maybe you could say that:

s/EDHOC provides
      proof that the other party possesses the private authentication
      key corresponding to the public authentication key in its
      certificate./EDHOC provides
      proof that the other party possesses the private authentication
      key corresponding to the public authentication key in its
      certificate, which is also known as proof-of-possesion.

s3.5.1, 1st bullet (question): This seems like the “mixing” discussed in s3.2, relies on the identity provide in the certificate?

    The certification path provides proof that the
    subject of the certificate owns the public key in the certificate.

s3.5.2 (question): related to comment on s3.2.

s3.5.3, 2nd para (nit): Could point to RFC 3279/5480 for some key usages for ECDSA and RFC 8410 for x25519/x448.

In
   X.509 and C509 certificates, signature keys typically have key usage
   "digitalSignature" and Diffie-Hellman public keys typically have key
   usage "keyAgreement" [RFC3279][RFC8410].

s3.6 (nit): refer to both?:
s/TLS 1.3 [RFC8446]/
(D)TLS 1.3 [RFC8446] [I-D.ietf-tls-dtls13]

s3.5.3 (question): EDHOC seems to assume you only provide the reference for the initiator's/responder’s. Was there any thought it to providing references for the rest of the certification path up to but not including the Root CA’s certificate? I mean technically they can also be in there certificate provided so not necessarily needed.

s3.6, CNSA ref (nit): I made a similar comment against the ZRTP spec when they said their alg suites were compliant with Suite B … probably best to say Suite 24 and 25 use algorithms in CNSA and leave the word “compatible” for “them” to say.

s3.8 (comment): Wow .. so you can send in the CSR in EAD, if it works then you can use the cert in the later messages (e.g., kind of like EAP)? Sure hope there’s no kind of CSR-related error. And, you know that the CA gets to decide the name right. Sorry just trying to digest it all.

s4.3 (nit): s/kan/can

s4.4/s5.1 (question): Do you need to provide advice on when to delete the old PRK_4x3m? I.e., does the peer that sent this need to wait for some kind of confirmation before deleting it?

s5.1 (question): Is a state diagram needed? One thing people clamored for from TLS was a state machine. Maybe a diagram isn’t needed because there are so few states?

s5.1 (comment, no action required): Cute. Deduplication is how to deal with message reply ;)

s5.2.1, s.5.3.1, s5.4.1, s5.5.1 (nit): is the “,” before the closing bracket right? e.g.,

OLD:

    ? EAD_1 : ead,
}

NEW:

    ? EAD_1 : ead
}

s5.2.2 (question): If this this initiator processing section, how does the initiator know to truncate?

  …, truncated after the cipher suite selected for this
  session.

s5.2.3, s5.3.3, s5.4.3, s5.5.3, last sentence (question - probably being pedantic): If there is an error but an error message is not sent, is the session discontinued? How does the peer know it was discontinued if an error is not sent?

s5.2.3, s5.3.3, s5.4.3, s5.5.3 (nit, which I am sure the RFC editor fix better than I could, but I couldn’t help myself): Instead of the e.g., in middle of the MAY sentence maybe:

OLD:

   Sending error
   messages is essential for debugging but MAY e.g., be skipped if a
   session cannot be found or due to denial-of-service reasons, see
   Section 8.6.

NEW:

   Sending error
   messages is essential for debugging but MAY be skipped if, for
   example, a
   session cannot be found or due to denial-of-service reasons, see
   Section 8.6.

s6.1 (nit): To stop loops, should any peer that receives an ERR_CODE=0 discontinue the session? I.e., is that worth stating in the I-D?

s6.2 (nit): It’s really more “Freeform” than unspecified right? I mean the string is required so it’s definitely not unspecified per se.

s6.3.2, 2nd to last para (nit): s/shall/SHALL

s6.3.2/s5.2.1 (nit): Might be worth noting earlier that SUITES_R is not important. Also, s5.2.1, mentioned SUITES_I, but s5.2.2 does not mention SUITES_R.

s7, last para (nit): s/To enable as much interoperability as we can
   reasonably achieve,/To enable as much interoperability as possible,

s8 (question): Isn’t some kind of consideration needed to address the use of PSKs shared by more than two? See RFC 8773 for some considerations wrt to group key sharing and that sharing impact on authentication.

s8.3 (nit): You can informatively refer to RFC 6194 to slam the door on SHA-1.

s9 and onwaard (apologies): I ran out of steam.

s? (question): Is a peer supposed to check status information for its peer’s certificate/key? 

I-D Nit complaints (not all of them because the DOWNREFs (mostly) looked intentional):

== Missing Reference: 'RFC9052' is mentioned on line 2411, but not defined

Cheers,
spt