[Dtls-iot] AD review of draft-ietf-dice-profile-13

[Stephen Farrell <[e-mail]>]

Hiya,

I've done my AD review of draft-ietf-dice-profile-13.
It's a fine piece of work that's nearly but not quite
ready.

I have quite a few (13!) comments that I'd like to chat
about before starting IETF LC and at least the last of
those definitely needs a revised I-D. So I'll be marking
this as needing that shortly. If it helps to get out an I-D
before the Prague meeting starts just let me know and I
can ok that.

Most but not all of the things I'd like to chat about
before starting IETF LC are easy, but need checking I
think.

And since you chose to make this a 55 page document,
I've of course also got a bunch of nitty nits:-) Those
can be handled before or after IETF LC from my POV,
whatever's easier for the WG/authors.

Anyway, I don't think it ought take us too long to resolve
these issues and move this on to IETF LC and I bet people
will think this is a really fine RFC when it does pop out
from the RFC editor.

Cheers,
S.

Things I'd like to chat about before I start IETF LC:
-----------------------------------------------------

(1) 6.1: Is TLS_PSK_WITH_AES_128_CCM_8 still ok for
CoAP? We're discussing taking all the _8 ones out for
SRTP, i.e., from [draft-ietf-avtcore-srtp-aes-gcm]
because of the 8 octet tag being too short. I'd need to
go check if the same issues apply here, but maybe they
don't. What do we think? If this applies to any of the
ciphersuites here, it presumably applies to all so I
wanted to check just in case. (Kevin Igoe and Magnus
Westerlund are involved in that SRTP discussion in case
it helps.)

(2) 6.2: Saying that the client key pair "may, for
example, be configured during the manufacturing process
of the device" is not ok really. There have been too
many cases where the same key pair has been used in
whole production runs or worse. You need to add a 2119
instruction here that doing the quoted thing "REQUIRES a
different key pair for each device that SHOULD be
generated and installed in as-secure a manner as
possible." Or similar text. I don't care where that text
is added (6.2 might not be right) so long as it's not
hidden away on page 54:-)

(3) 6.3: How can you have a MUST for s/w update when
there is no well-defined interoperable way to do that. I
think s/MUST/need to/ would be right here. (I'd love to
see us define the IETF bits of a s/w update mechanism
that could be referenced via such a MUST but we don't
have that today so it's a bogus MUST as written.)

(4) 6.3: The Ed25519 reference should be removed or the
text modified.  CFRG are not selecting Ed25519 it seems
so that would be misleading as-written.

(5) 6.3: Forgetting CoAP for the moment, surely this
profile will be used with devices that only have
(possibly bogon) IP addresses and that want to have
those in certs. I do get that how to handle that well is
not very clear, esp. for certs for e.g. 192.168.0.1, but
shouldn't it really be covered by this profile?

(6) 13: sorry I don't get why a 1 second initial timer
is an issue because messages "need longer" (we're not in
a DTN, right:-) I don't think you've justified the 10s
initial value and that might be bad in fact e.g.  for
sleepy nodes one might want a much shorter initial
timeout so the node can go back to snoozing.  (It could
be that this 10s value was justified on the list, but
I'm not getting it from the text. Or is it appendix A
you're thinking of? If so, that seems overly specialised
for a generic 10s recommendation.)

(7) 14: Doesn't [Heninger] really cause many devices to
use one RSA prime factor? That's not the same as "same
keys again and again" and in any case you're not
recommending RSA keys on challenged nodes here.
Shouldn't you do the analysis of the impact of a dodgy
PRNG on populations of devices that follow these
profiles and not something else?

(8) 14: You are RECOMMENDING to follow
mathewson-no-gmtunixtime which is fine. That however
means that that's a normative reference on which you
need to wait. I'm fine with that, but the WG may or may
not be.

(9) 14: And also - if you RECOMMEND to sometimes use
ServerHello.Random for "secure time" then don't you have
to say that that's only to be used after one has
verified the server cert/knowledge-of-PSK or something?
Otherwise any old fake server could send me a time value
then bail on the handshake, and I might set my clock to
that. (The bad effect there is not related to
certificate revocation btw, but more e.g. DoS for sleepy
nodes or application layer nastiness.)

(10) 14: I want to check that the WG are really ok with
the "MUST" for h/w "quality" RNGs. I really like the
idea, but it's very odd for the IETF to have such a MUST
touching on the style of implementation.  (Say if my
entire implementation of TLS is in some kind of VM?
Then I couldn't meet this ever.) I think we have to
s/MUST/weasel-words/ there tbh, sadly - it's just not
our job to mandate implementation styles. The following
para though does the job nicely, so maybe s/MUST/ought
to/ is enough of a change.

(11) 21: Why not make RFC7539 a SHOULD or MUST right
now?  Doesn't it seem like doing so now in a profile
would be the right kind of timing? And that might be our
best bet for healing the CCM/GCM rift so I'd like to
check if the WG agree with that idea or not before we go
to IETF LC. (That might justify a separate thread.)

(12) 24: You need to mention that simply emitting a
packet can be privacy sensitive and that (D)TLS doesn't
help if so. For example, if lights turn on when I enter
the room and those packets can be detected then seeing
any packet says someone has entered the room.  Or if a
thing I carry about sends out nicely encrypted stuff
then seeing the destination IP for that might have the
privacy issue. Or if a sensor has a threshold and only
fires above/below that then seeing any packet means
we've crossed the threshold. Text that explains that and
that that's a system and/or application layer issue is
needed I think.

(13) Appendix C has an editor note that says that it's
not finished. That's gotta be wrong, no?

Minor/nitty comments:
---------------------

- Title: You are actually defining >1 profile but the
title is in the singular. Worth fixing?

- 55 pages! Sigh;-)

- So it seems like everything up to section 6 is just
introductory and not normative text (or at least pages
4-19 or so). IMO that is too much and if we were earlier
in the process I'd argue it'd be better reduced a lot or
put into another document.  As-is, I think you need to
add text in the intro that says "If you're familiar with
(D)TLS, then skip ahead to section 6" and then at the
start of section 6 you need to say that the normative
text really starts here. 6.1 is the first place I saw
any 2119 keywords - "required" (lowercase in the draft)
and a SHOULD NOT.

- section 2 (or somewhere): it might be worth saying
that TLS1.3 is being developed but that it is not
expected that this profile will "just work" for that due
to the significant changes being done to TLS for 1.3.
That'd imply there may be a need for another profile
later for TLS1.3 but that's ok, I'm not asking we start
that work now, just that we explicitly say that 1.3 is
underway and we're not expecting this profile to work
for that just as-is. You do sort of say that in section
8, but that's a bit obscure I think.

- section 3: Is RC4 really the only stream cipher
defined for TLS1.2? That surprised me but I've not
checked.

- 4.1.1.1 has quite a bit of text - I'd have said it
wasn't all needed and just saying EAP-TLS is relevant
and some RFC pointers would have been enough.

- Figure 4 spans 3 pages. That's too much. Can't you
split that into two figures each on one page?

- 4.1.1.2, last para before Figure 4: Are you implying
that DTLS-server-initiated resumption needs to be
supported? I don't know if that's reasonable or not.  Is
it? This is triggered by the combination of "After some
time" and "the already established" which could be read
to imply DTLS session resumption initiated by the
resource directory. Maybe this is addressed later in the
document though?

- 4.2, para just after figure 5: Is "have to be solved"
correct? There are really no other ways? I think
s/have to/can/ is probably better.

- 4.2: Something seems broken here "as described in
Section 2 of [RFC7452] these different communication
patterns.  "

- Figure 6 spans >1 page too and needn't.

- Section 5 duplicates RFC5246. I'd say delete the lot
would be better. I'll live if you don't:-)

- 6.1: "While the server is not required to initiate a
cookie exchange with every handshake, the client is
required to implement and to react on it when
challenged." Is that a piece of normative specification
or just a description of what's already documented
elsewhere? I think you should be clear about that.  (As
this'd be the 1st bit of normative spec in the draft I
think.)

- 6.1: "into the firmware" I think you should warn
against doing this, even in a passing reference like
this. Firmware is usally published and putting sensitive
values inside that is a really bad idea.

- 6.1, near end of p20: "bit-by-bit comparison" please
say to make that constant time, just in case it got into
a tight enough loop to expose information about what
ought be secret.  (I assume guessing the PSK identities
that a device has configured in might be sensitive if it
can be done remotely. I forget how mismatches in those
are handled.)

- 6.3: "is omitted from the chain" - I think a recent
DANE draft required that the root keys be included in
the chain. Just notin the potential mismatch in case
someone cares about these kinds of device and DANE.
(I'd say "ask Viktor" is the way to check that out.)

- 6.3.3, last para: that's not a good argument - trust
anchors do not get revoked anywhere near as often as end
entity (TLS client or server) certs - so for constrained
devices, I'd expect a higher revocation rate in theory.
(Though I agree, not in practice.)

- table 1: this has too much stuff that is syntactically
required and where there is no real choice. Shorten the
table by getting rid of all those where you've nothing
more to say. The table spans 3 pages too, so please
break it into two.

- table 1, serialNumber: for what length serial do you
require support? There was a time when longer serials
were an issue for some implementations and I could see
that happening again here maybe. If that is an issue,
then probably so would subject and issuer where you
might also want to add a bit more text.

- 6.3.6: typo: "during the DTLS exchange provides"

- section 8: Simplification is good but I wondered if
these restrictions were going to simplify very much. I'd
not be surprised if implementations had to be able to
handle lots of the errors you say aren't needed in any
case. So maybe doing this as non-normative guidance
would be better (and hence moving it before section 6).

- section 9 typo: s/server constrained/server is
constrained/

- 10: CRIME needs a reference. You could use the UTA
draft.

- 12: typo s/my expire/may expire/

- 12: I think you ought refer to heartbleed in some way
- some implementers might think that was a protocol
design flaw and not follow this section if you don't.

- 23: If false start is RECOMMENDED then it needs to be
a normative ref. Do you want to wait for that too?

- 25: I really like para 2, thanks. I'm not sure that
the OMA thing is the answer. (It might be, I don't
know.)

- Appendix A: I'm getting WAP-flashbacks :-( Are you
really recommending that A.3 be implemented? I assume
not, but it nearly says that so I thought I'd just
check.