Subject: [Perc] review of draft-ietf-perc-srtp-ekt-diet

From: Sean Turner <[email-masked]>
Date: Thu, 8 Dec 2016 10:48:08 -0500

I volunteered in Seoul to review the draft.  Three bigger picture questions, then a bunch of nits:

0) I suspect the answer to this question is "yeah we know but we don’t want to wait”: DTLS1.3 is going to include a “Key and IV update handshake message" so aren’t we kind of setting up a situation where we’re burning a TLS content type code point for a DTLS1.2-specific protocol? Put another way are you going to redo this for TLS1.3?

1) Based on the DoS security consideration, it appears as if you’re okay with the EKT field not be protected by SRTP.  (he asks without having dug into SRTP in a while) There’s really no way to get at integrity protection of the EKT field?

2) s2.2.2, #1: I gotta ask why the MessageType byte isn’t first? Wouldn’t putting the MessageType first allow you avoid buffering everything before proceeding?  It seems like you’re counting on nobody sending a string of zeros as the first byte of the EKT field to mess with implementations.

NITS:

0) s2, last paragraph

What happens if an implementation does use SRTP's MKI or with SRTP's <From, To>?  Is an error thrown?

1) s2.1, EKTPlaintext

Probably over thinking this, but is the concatentation like so:

+--------------+--------+--------+
| SRTP MK | SSRC | ROC |
+-------------+---------+--------+

I’d be afraid somebody would think it’s the other way around without being more specific.  Maybe it’s as easy as: putting something like "SRTP MK || SSRC || ROC, where || denotes concatenation” somewhere in the paragraph.

2) s2.1, EKTMsgLength

Should this be “MUST”: All EKT message other that ShortEKTField must have ...

3) s2.1, MessageType

Maybe it’s late but I’m having trouble parsing this sentence:

  Values less than 64 are mandatory to
  understand and the whole EKTField SHOULD be discarded if it
  contains message type value that is less than 64 and is not
  implemented.

Can “and is not implemented” be dropped?

4) s2.2.1

r/needs to be sent/is sent
r/is to be sent/is sent

5) s2.2.1, #4

“EKTMEsgTypeFull” doesn’t appear anywhere else in the draft.  I think it’s supposed to “ … Message Type using the FullEKTField format."

also: r/EKT Ciphertext/EKTCiphertext

6) s3.2

To ensure we tick all the coordination boxes, it’s probably worth making sure that this draft also gets sent to the TLS WG during WGLC because it’s defining an extension and a content type.

7) s3.2:

struct {
  EKTCipherType ekt_ciphers<0..254>;
} SupportedEKTCiphers;

0..254?  Should it be 1..256?

8) s3.2:

r/When the server responds in the
   "srtp_ekt_key_transport" in its ServerHello message, it must include
/When the server responds in the
   "srtp_ekt_key_transport" in its ServerHello message, it MUST include

9) s5

Should the registries be part of the SDP or RTP parameters group?  I.e., collected together with the other SDP parameters?

10) s5.2

I’m assuming the AESKW256 value is 3 on purpose?

Cheers,

spt