Re: [secdir] Secdir review of draft-ietf-perc-double-10

[Benjamin Kaduk <[e-mail]>]

Sorry for top posting; on phone.

To ask the obviius question: what kind of hardware was your testing on?
Did it include ARM CPUs/mobile devices?  Anything without aesni?

Thanks,

Benjamin

On Wed, Oct 31, 2018 at 03:29:58PM -0400, Richard Barnes wrote:
> Hi Radia,
> 
> Thanks for the review.  Responses to the substantive comments inline.  Will
> take a look at the editorial ones in a bit.
> 
> 
> On Wed, Oct 31, 2018 at 2:25 PM Radia Perlman <[e-mail]>
> wrote:
> 
> >
> > I have reviewed this document as part of the security directorate's
> > ongoing effort to review all IETF documents being processed by the IESG.
> > These comments were written primarily for the benefit of the security area
> > directors.  Document editors and WG chairs should treat these comments just
> > like any other last call comments.
> >
> > This document specifies a syntax for doubly encrypting Secure Real Time
> > Protocol (SRTP) packets so that the inner (media) content is encrypted with
> > a key known only to the endpoints while some header content is encrypted
> > with a separate key known by both the endpoints and network relay points
> > called Media Distributors (with the outer encryption key (usually) changing
> > on each hop).
> >
> > Below are details for consideration by the authors and other potential
> > security reviewers.
> >
> > Substantive comments:
> >
> > Page 2 First paragraph:
> > AES-GCM is the only specified cryptographic mode, but may not be the
> > appropriate mode to use in distributing this content because there may be
> > cases where we want to guarantee the integrity of the data end-to-end while
> > allowing intermediaries to see the content. To support this case, it might
> > make sense to allow the encryption and integrity protection keys to be
> > separate so that an intermediary might be given the encryption key but not
> > the integrity protection key.
> >
> 
> I think you mean we would give the intermediary the integrity key, not the
> encryption key :)  Since we want to enable the intermediary to modify
> things, but not read the content.  Even that wouldn't work, however,
> because we do want the payload content to be integrity-protected
> end-to-end.  So you need to double up at least on the integrity check.
> 
> 
> 
> > As noted in section 8, the inner payload ends up being encrypted twice
> > (which is a substantial waste of resources). It might make more sense to
> > include the inner encrypted payload as additional authenticated data in the
> > outer invocation of AES-GCM to avoid that overhead.
> >
> 
> You're correct that the second encryption is duplicative, but in the
> testing I've done, it is not a substantial waste of resources.  As noted
> above, you need two integrity checks anyway, so if we're not going to rely
> on multiple primitives, that means multiple invocations of AES-GCM.  And in
> practice, an "integrity only" invocation of GCM, with everything in the
> AAD, is not noticeably faster or slower than a normal invocation that also
> encrypts.
> 
> Another benefit of the current scheme is that having the outer transform be
> identical to the base AES-GCM transform makes implementation easier for
> intermediaries.  They don't have to change their SRTP stacks, they just
> have to adapt packet processing a little bit to add the OHB logic.
> 
> So yes, in principle, it's a little duplicative, but in practice, the
> duplication doesn't matter and the symmetry you get from it is advantageous.
> 
> 
> 
> > Page 5 section 3.1 Key Derivation:
> > This section specifies how the end-to-end and hop-by-hop keys must be
> > derived from a single "master key". But this could only be true at the
> > source node and would imply the intermediaries are not allowed to see the
> > master key and the destination would not need to see the master key
> > (because by the time the destination receives the message the key used for
> > the outer encryption would be different.
> >
> > I would expect that the inner and outer keys would be negotiated
> > independently and there would be no master key from which they are all
> > derived.
> >
> 
> The key derivation here is inherent to SRTP; it would be a much more
> significant change to SRTP processing logic to specify keys directly,
> rather than adapting the KDF.
> 
> The KDF adaptation is needed so that you can give the intermediary the HBH
> half of the master key (using something like draft-ietf-perc-dtls-tunnel
> <https://tools.ietf.org/wg/perc/draft-ietf-perc-dtls-tunnel/>), and it'll
> end up KDF'ing that to the right keys / IVs for the HBH transform.
> 
> 
> 
> > Page 5 Section 4:
> > If the set of RTP header fields is fixed and there is a fixed ordering to
> > them, this is OK. But it wasn't obvious to me how to reflect in the OHB the
> > difference between a new header field being added. And if fields are
> > deleted (and therefore added to the OHB), it's not obvious in what order
> > they should be reinserted by the ultimate recipient.
> >
> 
> The set of RTP header fields that get E2E protection is fixed, with a fixed
> bit layout.  There is a variable extension portion at the end, but that
> receives no E2E protection, so it's not included in the OHB.
> 
> 
> 
> > Page 6 Section 5:
> > The second paragraph says the extensions are truncated when computing the
> > inner checksum while the third paragraph says there is information in the
> > OHB to reconstruct the original extensions to allow verification of the
> > inner checksum. Either of these two techniques could be used, but the
> > source and destinations must use the same one. Or is this specifying that
> > some combination be used?
> >
> 
> Where are you seeing a claim that the OHB can help with extensions?  It's
> not impossible that it's there; earlier versions had this feature, but it
> was removed to simplify things.
> 
> If the text you have in mind is "OHB that expresses any changes made
> between the inner and outer transforms", then I think it's just
> approximation for brevity.  The exact changes that the OHB can express are
> defined elsewhere.
> 
> 
> 
> > Page 9 paragraph 1 says:
> > "A Media Distributor that decrypts, modifies, and re-encrypts packets
> >    in this way MUST use an independent key for each recipient, SHOULD
> >    use an independent salt for each recipient, ..."
> > This represents substantial additional work for the Media Distributor. If
> > it is important to use an independent key for each recipient, some
> > justification should be noted. The likely one is that it prevents one
> > recipient from undetectably modifying the copy of data sent to another
> > recipient. This protection is not needed in all scenarios, and so perhaps
> > should be optional.
> >
> 
> The hard line here is that whenever the plaintext changes (e.g., the PT
> header gets sent to a different value), then a different (key, IV) pair
> needs to get used.  Otherwise you have nonce reuse.
> 
> The requirement as written, however, is probably a bit too strong.  It
> presumes that each recipient is getting a different packet, which is very
> common in conferencing scenarios, but not universal.  I'll think about how
> to clarify.
> 
> Thanks,
> --Richard
> 
> 
> >
> >
> > Editorial Comments / Typos:
> >
> > Page 2 Second paragraph:
> > There may be a word missing. What is "the double"? Also, throughout the
> > document the  protocol is referred to as "double", which if intended should
> > probably be listed in section 2.
> >
> > Page 2 Last line:
> > If multiple Media Distributors are allowed, hop by hop also refers to the
> > path between two Media Distributors.
> >
> > Page 8 Section 5.2 bullet 2:
> > <fields are allowed> -> <fields that are allowed>
> >
> > Page 11 section 7.1 paragraph 1
> > I suspect there is a word or two missing. "it MUST be encrypted the packet
> > in repair mode" sounds like something Yoda would say.
> >
> > Page 13 section 9 last paragraph says:
> > "If the MD doesn't modify any header fields,
> >    then an MD that supports AES-GCM could be unused unmodified."
> > This should say something like ...could use the same key and relay packets
> > unmodified.
> >
> >