Re: [Perc] AD Evaluation of draft-ietf-perc-srtp-ekt-diet-08

[Ben Campbell <[e-mail]>]

Thanks!.  I will add this to my list of things to look at after this week’s telechat.

Ben.

> On Oct 9, 2018, at 2:06 PM, Richard Barnes <[e-mail]> wrote:
> 
> Thanks for the review, Ben.  I've posted a PR with responses; notes inline below...
> 
> https://github.com/ietf/perc-wg/pull/161 <https://github.com/ietf/perc-wg/pull/161>
> 
> Apropos of your follow-up comment, I have sent a ping to the last address I have for Dan Wing, and will update the PR with whatever I hear back from him.
> 
> 
> On Thu, Aug 9, 2018 at 6:28 PM Ben Campbell <[e-mail] <mailto:[e-mail]>> wrote:
> Hi,
> 
> This is my AD Evaluation of draft-ietf-perc-srtp-ekt-diet-08.
> 
> The document is mostly in good shape. It’s easy to read and understand. However, I have a few substantive comments/questions and some editorial comments. I would like to at least discuss the substantive comments before going to IETF LC.
> 
> Thanks!
> 
> Ben.
> 
> ----------
> 
> *** Substantive Comments: ***
> 
> 
> §3: Is there a reason not to use the RFC 8174 boilerplate? There are at least a few instances of lowercase keywords in places that don’t seem intended as normative.
> 
> Not intentional.  I fixed one lower-case "must" and changed over to 8174.
> 
> 
> §4.2.1, first paragraph: Is there never a situation where you send neither version?  Also, did I miss a description of the purpose and semantics for ShortEKTField, other than “send it when you don’t send the full version”?
> 
> No, there is never such a situation.  EKT always adds at least one byte of overhead to every packet.  Because RTP lacks an internal length indicator, the receiver of an EKT-enabled flow needs to know how much EKT data to strip off the end of any given packet.  Even if the answer is "zero", you still need a way to signal that, which is what the ShortEKTField does.
> 
> 
> §4.2.2, step 3: "If the EKT decryption operation returns an authentication failure, then the packet processing stops.”
> 
> .... and then what?
> 
> Clarified that EKT processing is definitely over, and the packet SHOULD be discarded.
> 
> 
> - Step 6: "This applies in transforms such as [ I-D.ietf-perc-double] for replacing just half the key, but SHOULD return a processing error otherwise.”
> 
> I don’t understand that sentence. Is the point to say that using a “too short” key to replace part of the old key is only valid for perc-double or similar transforms, and receiving one in other contexts is an error?
> 
> Revised to clarify and tighten.
> 
> - Next Sentence: "Outbound packets SHOULD continue to use the old SRTP Master Key for 250 ms after” Does that imply a normative requirement for recipients to keep old keys around for at least that long? There’s currently an implementation suggestion about that, but it doesn’t use MUST or SHOULD.
> (Editorial: The sentence seems to belong in the “outbound” section, not the “inbound” section.)
> 
> Yeah, moved this to the outbound section.  Added MAY-level inbound considerations to the implementation notes -- either the receiver can accept packet loss or do trial decryption.
> 
> 
> §4.3:
> 
> - “ This ciphertext value MAY be cached by an SRTP
>    receiver to minimize computational effort by noting when the SRTP
>    master key is unchanged and avoiding repeating the steps defined in ..”
> 
> Are we really talking about the recipient? How would the recipient know that the key has not changed without processing the ciphertext value? (Also, the sentence is unfinished--maybe the answer would be clear if the rest of the sentence came out of hiding :-) )
> 
> The unclear ending is a missing cross-reference to Section 4.2.  I think the receiver optimization here is that if you cache the tag, you can memcmp instead of decrypting.  Same on the sender side, memcpy vs. encrypt.  Updated to reflect that.
> 
> - “ The receiver may want to have a sliding window to retain old SRTP
>    Master Keys (and related context) for some brief period of time, so
>    that out of order packets can be processed as well as packets sent
>    during the time keys are changing.”
> 
> See comment to §4.2.2, step 6. “may want” seems weak given that the sender SHOULD keep using the old key for a period of time.
> 
> Revised this as noted above, though note that MAY WISH TO is defined in RFC 6919.
> 
> 
> §4.7: “A new sender SHOULD send a packet containing the FullEKTField as soon as possible...”
> 
> Why not MUST? Would it ever make sense not to do this? It seems like the mechanism doesn’t work very well otherwise.
> 
> How would you comply with a "MUST ... as soon as possible"?  I don't think we want to dictate a schedule here, but I've added a note that packet loss can result if you don't.
> 
> 
> §6, first paragraph: “or other” - what other?
> 
> Clarified.
> 
> 
> - paragraph 8: “ An endpoint MUST NOT encrypt more than T different FullEKTField values using the same EKTKey.”
> 
> Is there a reciprocal requirement for decryption?
> 
> No.  The risk attaches once the ciphertext is transmitted.
> 
> 
> - last paragraph: “...SHOULD be limited using the ekt_ttl”
> 
> Why not MUST?
> 
> Upgraded.
> 
> 
> §7.3: Should this cite RFC 5246 instead of RFC 4366?
> 
> You mean RFC 8446?  Actually this line and the similar one below it are both unnecessary, so I've deleted them.
> 
> 
> *** Editorial Comments: ***
> 
> §2: “...each endpoint picks there own SRTP master key...:
> s/their/its
> 
> Done.
> 
> 
> §4.1, first paragraph: Please reference figures by number. I can imagine some future RFC rendering failing to  maintain the relative positions.  (This occurs a few times throughout the document.)
> 
> §4.2.2, step 3:
> Should "The EKTCiphertext authentication is checked and is decrypted” be something like “
> The EKTCiphertext field authentication is checked and its contents decrypted” I assume you don’t mean to say its authentication is decrypted.
> 
> "The EKTCiphertext is authenticated and decrypted..." -- since these operations are combined in many SRTP transforms.
> 
> 
> Also, there’s a number of occurrences of “The [fieldname]...” that should probably say “The [fieldname] field...”
> 
> This doesn't seem like a bad ambiguity to me.
> 
> 
> - Step 6: s/crypto/cryptographic
> 
> Disagree.
> 
> 
> - Step 7: is the replay protection part relevant to the step?
> 
> Deleted.  Added a note about replay to the security considerations.
> 
> 
> §4.3, third paragraph: This is the first mention of ekt_ttl. It would help to have a brief explanation or at least a forward reference.
> 
> Added a forward reference
> 
> 
> §4.6: s/ “other specification” / “another specification” ; or “other specifications”
> 
> Revised.
> 
> 
> §4.7, third paragraph: Both occurrences of “which” need preceding commas.
> 
> Done.
> 
> 
> §5.2, paragraph after figure 4: “ ... the extensions defined in this session allows the DTLS server...”
> Defined in this “session” or this “section”?
> 
> Actually, "in this document"
> 
> 
> §, paragraph 7: “... causing the receiving system will decrypt...”
> s/will/to
> 
> Done
> 
> 
> 
> 
> _______________________________________________
> Perc mailing list
> [e-mail] <mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/perc <https://www.ietf.org/mailman/listinfo/perc>