Subject: [Lake] review of edhoc-12

From: Stephen Farrell <[email-masked]>
Date: Thu, 11 Nov 2021 00:38:32 +0000

Hiya,

At our last interim I also promised to review edhoc. This
is my late (apologies;-) review. This is all review by me
as just another participant, not as co-chair. (So please
do feel entirely free to ignore bits or just say no.)

Generally, I guess I could implement from this, were I
fluent in CBOR/COSE etc, so I think it's in good shape.
All but my first comment are editorial. I assumed that
we'll have others who do crypto analysis and implementers
who'll provide yet more detailed feedback as we go, so
we're not quite there yet but getting pretty close IMO.
Good job!

Cheers,
S.

- Connection identifiers (which can be byte-strings) are
sent in clear which could enable various network observer
attacks for protocols that later send values obviously
derived from connection IDs in clear. (I see from A.3 that
one main use case does expose these values anyway.). To
given an example of how this could be concerning, if some
proxy (that just muxes packets) sits between I and R then
those cleartext identifiers could allow an observer on that
link to more easily do traffic analysis of a specific
initiator's traffic. Was any consideration given to deriving
such identifiers in a less obvious manner? I'm not claiming
that that'd be a great improvement, just asking.

Editorial:

- 80 pages is still big, I understand its hard to delete
   stuff but hope we keep trying:-)

- 1.1, CWT, CSS and C509 could do with expansion here on
   1st use.  (Perversely, X.509 is probably sufficiently
well known and disliked to not need such:-) That might also
make the text before (or caption of) Figure 1 easier to
read.

- section 1.2: last para - this is repetitive, suggest
   making these points just once

(Aside on figures/captions: I always find it a useful
exercise to ensure that a figure+caption can, by itself,
make a meaningful slide to present with no additions. And
then I remove most text that's already in the caption from
elsewhere in the document. Might be worth a try.)

- Figure 1: I don't get how the "Figure 1 shows..."
   sentence results in those example message sizes. I'm not
doubting the numbers, but the text could be improved
(maybe, as suggested above via caption text.)

- 1.4: are "EDHOC authenticated with digital signatures"
   and "EDHOC authenticated with signature keys" different
things? If not, using one term is likely better. If so,
using terms that are more clearly different would be good.

- 1.5: which is normative, CDDL or English language text?
   We seem to have a bit of a mixture.

- Figure 2: I see why message 2 doesn't also use an AEAD(),
   but probably no harm to say that more explicitly here.
Maybe moving some of the relevant text from section 8 to
here would work.

- Figure 2: consider showing the AAD as an input to the
   AEAD() construct in the figure. That might be too
cumbersome or it may help, not sure.

- 3.5.1, 3.5.2 and 3.5.3: this might be some text to
   shorten a lot - how much is it really needed? Could it be
cut down to the stuff with 2119 terms and a bit more? (I'd
keep the examples though.)

- 3.6: does EDHOC *really* support hash based sigs? What'd
   be the consequence for EDHOC of using a private key too
many times or loss of state?  (Are you missing a reference
to rfc8778 there too or is one embedded in COSE stuff
somewhere?)

- 4.1: Be good to clarify that the PRK_<foo> labels in
   4.1.x are basically local key names. (Same for others in
4.2.)

- 4.1.2/3: You need to define R and I in each of these as
   (I guess) the static DH secret and not as the identities
of the initiator and recipient which were (I thought) the
previous uses of I and R. Might be better to use different
names though.

- 5.2.2: What does "truncated after the cipher suite
   selected for this session" mean? (Also be good to say
order of preference means first in network byte order is
most-preferred.) I'm also puzzled by "but all cipher suites
which are more preferred than the selected cipher suite in
the list MUST be included in SUITES_I."

- 5.3.2: This seems to be the first occurrence of the
   "<<..>>" symbolism. A forward ref to C.1 would be good.

- 5.3.3: is it ok to pass EAD_2 to the application before
   checking authentication?

- section 6: I found this v. hard to follow. Suspect a
   re-write for clarity would be good.

- section 8: "EDHOC provides a minimum of 64-bit
   security..." could do with a reference to wherever that
conclusion is derived. And 64-bit security isn't quite "in
line" with TLS is it?

- 8.7: stating that a "truly random seed MUST" be provided
   isn't a sensible use of 2119, and isn't entirely under
the control of an implementer (maybe the section title
could be re-thought?).

- 8.7 (or somewhere): if some random values are visible
   (connection identifiers?) then it can make sense to
derive those from a different random stream compared to
that used for randomly picking secrets. That way the
publicly visible random numbers are less likely to leak
information about the state of the PRNG used for secrets.
Could be worth a mention.

- 8.7: discarding a message_1 because of G_X
   collision/reflection should also be stated earlier - it
could very easily be missed here.

- 8.7: TEE => "cannot" be tampered is a little optimistic
   IMO.

- 9.10: The well-known URI is not mentioned at all in the
   body of the spec but only here and then in an appendix. A
forward ref to A.3 from 9.10 is probably enough to fix. The
same may apply to other IANA registrations I guess.

- 10.1: are we confident that all the normative I-Ds will
   become RFCs in a sufficiently timely manner? I've no
specific reason to think they won't, and they're all far
along the process, but sometimes transitive dependencies
can cause a lot of delay.  (Very sadly, we can't just ask
Jim about this.)

- A.1: "byte string shaped"? what's that mean?

- Appendix D: Point 6 mentions an EUI-64. I'm not clear how
   that'd be used in edhoc.

- The URL for SIGMA can use https so change to [1]
   [1] https://webee.technion.ac.il/~hugo/sigma-pdf.pdf