Subject: Re: [Perc] AD Evaluation of draft-ietf-perc-double-09

From: Richard Barnes <[email-masked]>
Date: Fri, 5 Oct 2018 10:48:37 -0400

Hey Ben,

After talking with Cullen, I am now the stuckee for this.  I've filed a PR
at:

https://github.com/ietf/perc-wg/pull/160

Comments inline below.

On Tue, Jun 5, 2018 at 12:40 AM Ben Campbell <[email-masked]> wrote:

> Hi,
>
> This is my AD Evaluation of draft-ietf-perc-double-09. Thanks for this
> work; I think it is important and on the right track, but suffers from some
> readability issues, especially from §7 onwards. I’d like to discuss my
> comments prior to IETF last call.
>
> Thanks!
>
> Ben.
>
> —————————————————
>
> *** Substantive Comments ***
>
> General: Does it make sense to progress this prior to
> draft-ietf-perc-private-media-framework rather than wait and progress them
> together? it seems like one really needs to read the framework to
> understand at least parts of this draft. (Which would, btw, suggest making
> the framework a normative reference.)
>

I think at this point, -framework has caught up a bit (I understand it's
awaiting your review), so this seems OBE.  I agree that it will make easier
reading for ADs to read the three docs (this, framework, and EKT) on the
same telechat.

> §2, Media Distributor: Should this be defined as switch-based? Otherwise
> it seems circular.
>

I've copied down the definition §1, which explicitly says that the MD does
not need to interpret or change media content.  Also added a ref to
-framework at this point.

> §5 and subsections: These sections are confusing in the treatment of
> extensions. §5.1 step 3 says to truncate the header to remove any
> extensions. Yet other sections of text repeatedly mention the handling of
> extensions. I think the document needs a paragraph or two to describe the
> handling of RTP extensions at a high level.
>

I've added an overview of the transform, including thoughts on extensions,
to the top of §5.

> Along those lines, it’s not entirely clear what is meant by putting “ (12
> + 4 * CC bytes)” in parentheses after the guidance to truncate in §5.1. Is
> that the amount to be truncated? Amount left after truncation? (Same
> question for §5.3)
>

Clarified that this is the amount left after truncation, i.e., take the
first N bytes, in both places.

> §5.2: It would be helpful to include a paragraph or two to describe the
> impact if multiple MDs modify the same field(s). I can infer that, but it
> would be better to be explicit. (IIRC, there is a passing mention in the
> security considerations, but it would be better to have more here.)
>

Added a paragraph.

> §8: Can you offer any guidance about selecting 128 vs 256?
>

I don't really think there's much to say here; it's up to the user of the
transform.  Use 128 unless you're paranoid, then use 256?  :)  Note that
RFC 7714, which defines the AES-GCM transforms, does not provide similar
guidance.

> §9:
>
> - The security considerations mainly summarize the mechanism. I’d like to
> see a description of the potential attacks or risks  and how the double
> mechanism mitigates them.
> - “… this shouldn’t typically impact the strength of e2e integrity given
> the fact that there doesn’t exist header extensions defined today that
> needs e2e protection.  However, if future specifications define header
> extensions that needs e2e integrity protection, the input to inner
> transform may be modified to consider including the header.”
>
> - 2nd paragraph: Why is the HBH key for an outgoing packet only
> “typically” different than the key for the incoming packet? Are there
> security implications if they are the same?
>
> - 2nd to last paragraph, last sentence: Please elaborate on those risks?
>

I agree with your assessment that the existing security considerations were
not that useful.  I've replaced them with an analysis that's more focused
on what protections are provided with respect to which attackers.

The new version also calls out clearly that when a packet is re-encrypted,
the outer keys MUST be different.

> *** Editorial comments and nits ***
>
> There are a number of abbreviations that need to be expanded on first use.
> Please remember that abbreviations should be expanded both in the abstract
> and the body.
>
> - SRTP
> - SSRC
> - SEQ
> - ROC
> - PRF
> - PT
> - SEQ
> - RTX
> - RED
> - FEC
>

I've expanded most of these.  I have left SSRC, SEQ, ROC, and PT, since in
this context, they are basically opaque labels for RTP header fields.

> §2:
> - Please use the boilerplate from RFC8174. There are a number of lower
> case instances of normative keywords.
>

Done.

> - Please use consistent sentence (or lack of sentence) structure for the
> definitions in the bullet list.
>

Done.

> §3,
>
> - " Generate key and salt values of the length required for the combined
> inner (end-to-end) and outer (hop-by-hop) algorithms.”: The document is
> inconsistent in whether it describes a single key with inner and outer
> halves, or separate inner and outer keys. I realize that this is an
> artifact of syntax, but it is likely to confuse a reader new to the topic.
>

The very next bullet says to glue them together.  Not sure how else to say
this.

> - Paragraph starting with “Obviously, if the Media Distributor…”:
> “Obviously” is a null word in context.
>

Deleted.

> §3.1: This section would be much more approachable if you defined the
> terms (such as n, k, and x), rather than forcing readers to look them up in
> 3711. Also, the doubled crypto suite IDs are likely to be confusing to the
> reader who sees them here without the explanation later in the draft.
>

Done.

> §5.2, step 2: “Change any parts of the RTP packet…” -The MS is limited to
> changing the 3 fields defined for the OHB, right, at least as defined in
> this draft? Not just anything it wants?
>

Correct.  Done.

> §5.3, last two paragraphs: It’s sort of an odd construction to talk about
> “any of the following” with a single entry list.
>

Revised to clarify.

> §7 and subsections: These could use another proofreading pass. In
> particular, it is imprecise about inner vs outer encryption, which actors
> do what, and has a number of pronouns with ambiguous antecedents. The heavy
> use of passive voice obscures the actors in multiple places. I will call
> out some specifics, but please do not treat my comments as exhaustive.
>

Did a revision pass, let me know what you think.

> §7: “ The repair mechanism, when used with double, typically operates on
> the double encrypted data and encrypts
>    them using only the HBH key.” Does “double encrypted” mean “outer
> encrypted”? Also, while I recognize “data” as plural, it’s plural in a
> non-countable sense; that is, s / them / it.
>
> §7.1:
> - “ … cached payloads MUST be the encrypted packets …” - Inner or outer?
> What device enforces this? (Consider active voice.)
> - Which element represents “the other side”? Are we talking about an
> endpoint or MD? (Please stick to the defined terminology)
> - “ When encrypting a retransmission packet, it MUST be encrypted the
> packet in repair mode.” - I can’t parse the phrase after the comma. Also,
> what device MUST encrypt it?
>
> §7.2:
> - “the primary encoding MAY contain…” - Is that MAY a grant of permission
> or a statement of fact?
>
> - " The sender takes encrypted payload from the cached packets”- Missing
> article for encrypted payload? Or should this say “… takes encrypted
> packets from the cached payload…”?
>
> - “ Any header extensions from the primary encoding are copied to the RTP
> packet that will carry the RED payload and the other RTP header information
> such as SSRC, SEQ, CSRC, etc are set to the same as the primary payload.
> The RED RTP packet is then encrypted in repair mode and sent.”: This is
> confusing; parts are copied from the primary encoding and others are the
> same as in the primary encoding? Don’t both of those mean the same thing?
>
> - “ The receiver decrypts the payload…”: Endpoint or MD? Is this inner or
> outer encryption?
>
> - Last paragraph: Seems like that belongs in the FEC section. Also, does
> this document really need to make that sort of recommendation? It seems
> like a general statement about RED and FEC that is not specific to double.
>
>
> §7.3:
>
> - "When using Flex FEC [I-D.ietf-payload-flexible-fec-scheme] with double,
> the negotiation of double for the crypto is the out of band signaling that
> indicates that the repair packets MUST use the order of operations of SRTP
> followed by FEC when encrypting.” - The sentence is hard to parse. Also, is
> MUST a normative requirement or a statement of fact? (It seems odd to
> signal that you MUST do something).
>
- “…  data, which is already encrypted, it MUST be encrypted in repair mode
> packet.” s/ “, which” / “that”. Also, what actor MUST encrypt the data in
> repair mode? (Consider active voice.)
>
> - “ The algorithm recommend…” s / recommend / recommended
>

Done.

> §7.4:
> - s/ "sent with [RFC4733]” / “sent using the mechanism in [RFC4733]”
> - “ and the relay can not read it so it cannot be used” - missing comma
> between “it” and “so”.
>

Done.

> §8, last paragraph: “ For packets in repair mode, the data they are caring
> is often already encrypted further increasing the size.” - I assume
> “caring” is not the intended word. Did you mean “carrying”? If so, consider
> s / “data they are carrying” / “data they carry”

Revised.

---

From: Ben Campbell <[email-masked]>
Date: Tue, 9 Oct 2018 11:37:41 -0500

Thanks, I will look at both this as the framework draft after I get through this week’s telechat reading list.

Ben.

> On Oct 5, 2018, at 9:48 AM, Richard Barnes <[email-masked]> wrote:
> 
> Hey Ben,
> 
> After talking with Cullen, I am now the stuckee for this.  I've filed a PR at:
> 
> https://github.com/ietf/perc-wg/pull/160 <https://github.com/ietf/perc-wg/pull/160>
> 
> Comments inline below.
> 
> On Tue, Jun 5, 2018 at 12:40 AM Ben Campbell <[email-masked] <mailto:[email-masked]>> wrote:
> Hi,
> 
> This is my AD Evaluation of draft-ietf-perc-double-09. Thanks for this work; I think it is important and on the right track, but suffers from some readability issues, especially from §7 onwards. I’d like to discuss my comments prior to IETF last call.
> 
> Thanks!
> 
> Ben.
> 
> —————————————————
> 
> General: Does it make sense to progress this prior to draft-ietf-perc-private-media-framework rather than wait and progress them together? it seems like one really needs to read the framework to understand at least parts of this draft. (Which would, btw, suggest making the framework a normative reference.)
> 
> I think at this point, -framework has caught up a bit (I understand it's awaiting your review), so this seems OBE.  I agree that it will make easier reading for ADs to read the three docs (this, framework, and EKT) on the same telechat.
> 
> 
> §2, Media Distributor: Should this be defined as switch-based? Otherwise it seems circular.
> 
> I've copied down the definition §1, which explicitly says that the MD does not need to interpret or change media content.  Also added a ref to -framework at this point.
> 
> 
> §5 and subsections: These sections are confusing in the treatment of extensions. §5.1 step 3 says to truncate the header to remove any extensions. Yet other sections of text repeatedly mention the handling of extensions. I think the document needs a paragraph or two to describe the handling of RTP extensions at a high level.
> 
> I've added an overview of the transform, including thoughts on extensions, to the top of §5.
> 
> 
> Along those lines, it’s not entirely clear what is meant by putting “ (12 + 4 * CC bytes)” in parentheses after the guidance to truncate in §5.1. Is that the amount to be truncated? Amount left after truncation? (Same question for §5.3)
> 
> Clarified that this is the amount left after truncation, i.e., take the first N bytes, in both places.
> 
> 
> §5.2: It would be helpful to include a paragraph or two to describe the impact if multiple MDs modify the same field(s). I can infer that, but it would be better to be explicit. (IIRC, there is a passing mention in the security considerations, but it would be better to have more here.)
> 
> Added a paragraph.
> 
> 
> I don't really think there's much to say here; it's up to the user of the transform.  Use 128 unless you're paranoid, then use 256?  :)  Note that RFC 7714, which defines the AES-GCM transforms, does not provide similar guidance.
> 
> 
> §9:
> 
> - The security considerations mainly summarize the mechanism. I’d like to see a description of the potential attacks or risks  and how the double mechanism mitigates them.
> - “… this shouldn’t typically impact the strength of e2e integrity given the fact that there doesn’t exist header extensions defined today that needs e2e protection.  However, if future specifications define header extensions that needs e2e integrity protection, the input to inner transform may be modified to consider including the header.”
> 
> - 2nd paragraph: Why is the HBH key for an outgoing packet only “typically” different than the key for the incoming packet? Are there security implications if they are the same?
> 
> I agree with your assessment that the existing security considerations were not that useful.  I've replaced them with an analysis that's more focused on what protections are provided with respect to which attackers.
> 
> The new version also calls out clearly that when a packet is re-encrypted, the outer keys MUST be different.
> 
> 
> There are a number of abbreviations that need to be expanded on first use. Please remember that abbreviations should be expanded both in the abstract and the body.
> 
> - SRTP
> - SSRC
> - SEQ
> - ROC
> - PRF
> - PT
> - SEQ
> - RTX
> - RED
> - FEC
> 
> I've expanded most of these.  I have left SSRC, SEQ, ROC, and PT, since in this context, they are basically opaque labels for RTP header fields.
> 
> 
> §2:
> - Please use the boilerplate from RFC8174. There are a number of lower case instances of normative keywords.
> 
> Done.
> 
> - Please use consistent sentence (or lack of sentence) structure for the definitions in the bullet list.
> 
> Done.
> 
> §3,
> 
> - " Generate key and salt values of the length required for the combined inner (end-to-end) and outer (hop-by-hop) algorithms.”: The document is inconsistent in whether it describes a single key with inner and outer halves, or separate inner and outer keys. I realize that this is an artifact of syntax, but it is likely to confuse a reader new to the topic.
> 
> The very next bullet says to glue them together.  Not sure how else to say this.
> 
> - Paragraph starting with “Obviously, if the Media Distributor…”: “Obviously” is a null word in context.
> 
> Deleted.
> 
> §3.1: This section would be much more approachable if you defined the terms (such as n, k, and x), rather than forcing readers to look them up in 3711. Also, the doubled crypto suite IDs are likely to be confusing to the reader who sees them here without the explanation later in the draft.
> 
> Done.
> 
> §5.2, step 2: “Change any parts of the RTP packet…” -The MS is limited to changing the 3 fields defined for the OHB, right, at least as defined in this draft? Not just anything it wants?
> 
> Correct.  Done.
> 
> §5.3, last two paragraphs: It’s sort of an odd construction to talk about “any of the following” with a single entry list.
> 
> Revised to clarify.
> 
> §7 and subsections: These could use another proofreading pass. In particular, it is imprecise about inner vs outer encryption, which actors do what, and has a number of pronouns with ambiguous antecedents. The heavy use of passive voice obscures the actors in multiple places. I will call out some specifics, but please do not treat my comments as exhaustive.
> 
> §7: “ The repair mechanism, when used with double, typically operates on the double encrypted data and encrypts
>    them using only the HBH key.” Does “double encrypted” mean “outer encrypted”? Also, while I recognize “data” as plural, it’s plural in a non-countable sense; that is, s / them / it.
> 
> §7.1:
> - “ … cached payloads MUST be the encrypted packets …” - Inner or outer? What device enforces this? (Consider active voice.)
> - Which element represents “the other side”? Are we talking about an endpoint or MD? (Please stick to the defined terminology)
> - “ When encrypting a retransmission packet, it MUST be encrypted the packet in repair mode.” - I can’t parse the phrase after the comma. Also, what device MUST encrypt it?
> 
> §7.2:
> - “the primary encoding MAY contain…” - Is that MAY a grant of permission or a statement of fact?
> 
> - " The sender takes encrypted payload from the cached packets”- Missing article for encrypted payload? Or should this say “… takes encrypted packets from the cached payload…”?
> 
> - “ Any header extensions from the primary encoding are copied to the RTP packet that will carry the RED payload and the other RTP header information such as SSRC, SEQ, CSRC, etc are set to the same as the primary payload.  The RED RTP packet is then encrypted in repair mode and sent.”: This is confusing; parts are copied from the primary encoding and others are the same as in the primary encoding? Don’t both of those mean the same thing?
> 
> - “ The receiver decrypts the payload…”: Endpoint or MD? Is this inner or outer encryption?
> 
> - Last paragraph: Seems like that belongs in the FEC section. Also, does this document really need to make that sort of recommendation? It seems like a general statement about RED and FEC that is not specific to double.
> 
> 
> §7.3:
> 
> - "When using Flex FEC [I-D.ietf-payload-flexible-fec-scheme] with double, the negotiation of double for the crypto is the out of band signaling that indicates that the repair packets MUST use the order of operations of SRTP followed by FEC when encrypting.” - The sentence is hard to parse. Also, is MUST a normative requirement or a statement of fact? (It seems odd to signal that you MUST do something).
> - “…  data, which is already encrypted, it MUST be encrypted in repair mode packet.” s/ “, which” / “that”. Also, what actor MUST encrypt the data in repair mode? (Consider active voice.)
> 
> - “ and the relay can not read it so it cannot be used” - missing comma between “it” and “so”.
> 
> Done.
> 
> §8, last paragraph: “ For packets in repair mode, the data they are caring is often already encrypted further increasing the size.” - I assume “caring” is not the intended word. Did you mean “carrying”? If so, consider s / “data they are carrying” / “data they carry”
> 
> Revised.

---

From: Ben Campbell <[email-masked]>
Date: Mon, 15 Oct 2018 17:40:16 -0500

Thanks, the PR looks good to me. Please submit a revision when you are ready.

I think that, with the PR applied, it will be ready for IETF LC.  That should probably be done in tandem with the media framework (in my queue to review in the next couple of days. Authors and chairs: do you think we should also coordinate with ekt-diet?

Thanks!

Ben.

> §5.3, last two paragraphs: It’s sort of an odd construction to talk about “any of the following” with a single entry list..
> 
> Revised to clarify.
> 
> _______________________________________________
> Perc mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/perc

---

From: Richard Barnes <[email-masked]>
Date: Mon, 15 Oct 2018 18:42:49 -0400

I would propose we put forward all three documents together.  (-double,
-ekt-diet, and -framework)  They'll be more comprehensible to the ADs that
way.

On Mon, Oct 15, 2018 at 6:40 PM Ben Campbell <[email-masked]> wrote:

> I think that, with the PR applied, it will be ready for IETF LC.  That
> should probably be done in tandem with the media framework (in my queue to
> review in the next couple of days. Authors and chairs: do you think we
> should also coordinate with ekt-diet?
>
> Thanks!
>
> Ben.
>
>
> I agree with your assessment that the existing security considerations
> were not that useful.  I've replaced them with an analysis that's more
> focused on what protections are provided with respect to which attackers.
>
>> There are a number of abbreviations that need to be expanded on first
>> use. Please remember that abbreviations should be expanded both in the
>> abstract and the body.
>>
>> - SRTP
>> - SSRC
>> - SEQ
>> - ROC
>> - PRF
>> - PT
>> - SEQ
>> - RTX
>> - RED
>> - FEC
>>
>
>> “any of the following” with a single entry list..
>>
>
> Revised to clarify.
>
>
>> - "When using Flex FEC [I-D.ietf-payload-flexible-fec-scheme] with
>> double, the negotiation of double for the crypto is the out of band
>> signaling that indicates that the repair packets MUST use the order of
>> operations of SRTP followed by FEC when encrypting.” - The sentence is hard
>> to parse. Also, is MUST a normative requirement or a statement of fact? (It
>> seems odd to signal that you MUST do something).
>>
> - “…  data, which is already encrypted, it MUST be encrypted in repair
>> mode packet.” s/ “, which” / “that”. Also, what actor MUST encrypt the data
>> in repair mode? (Consider active voice.)
>>
>> §8, last paragraph: “ For packets in repair mode, the data they are
>> caring is often already encrypted further increasing the size.” - I assume

---

From: Ben Campbell <[email-masked]>
Date: Mon, 15 Oct 2018 18:04:28 -0500

Works for me. The framework draft review is in my queue for tomorrow.

Thanks!

Ben.

> On Oct 15, 2018, at 5:42 PM, Richard Barnes <[email-masked]> wrote:
> 
> I would propose we put forward all three documents together.  (-double, -ekt-diet, and -framework)  They'll be more comprehensible to the ADs that way.
> 
> On Mon, Oct 15, 2018 at 6:40 PM Ben Campbell <[email-masked] <mailto:[email-masked]>> wrote:
>> On Oct 5, 2018, at 9:48 AM, Richard Barnes <[email-masked] <mailto:[email-masked]>> wrote:
>> 
>> Hey Ben,
>> 
>> [email-masked] <mailto:[email-masked]>
>> https://www.ietf.org/mailman/listinfo/perc <https://www.ietf.org/mailman/listinfo/perc>
>

---

From: Richard Barnes <[email-masked]>
Date: Mon, 15 Oct 2018 20:40:39 -0400

Oh actually, let's not worry about synchronizing now.  I'm OK to send these
to IETF LC separately; I just think they should all be on the same IESG
telechat.

I'll push a new version of this tomorrow, and EKT as soon as I can find a
responsive author.

--Richard

On Mon, Oct 15, 2018 at 7:04 PM Ben Campbell <[email-masked]> wrote:

>> I've copied down the definition §1, which explicitly says that the MD
>> does not need to interpret or change media content.  Also added a ref to
>>> Along those lines, it’s not entirely clear what is meant by putting “
>>> (12 + 4 * CC bytes)” in parentheses after the guidance to truncate in §5.1.
>>> Is that the amount to be truncated? Amount left after truncation? (Same
>>> question for §5.3)
>>>
>>
>>> - The security considerations mainly summarize the mechanism. I’d like
>>> to see a description of the potential attacks or risks  and how the double
>> The new version also calls out clearly that when a packet is
>> re-encrypted, the outer keys MUST be different.
>>
>>
>>
>> I've expanded most of these.  I have left SSRC, SEQ, ROC, and PT, since
>> in this context, they are basically opaque labels for RTP header fields.
>>
>>
>>
>>> §2:
>>> §5.2, step 2: “Change any parts of the RTP packet…” -The MS is limited
>>> to changing the 3 fields defined for the OHB, right, at least as defined in
>>> §5.3, last two paragraphs: It’s sort of an odd construction to talk
>>> about “any of the following” with a single entry list..
>>>
>>
>> Revised to clarify.
>>
>>
>>> - “ Any header extensions from the primary encoding are copied to the
>>> RTP packet that will carry the RED payload and the other RTP header
>>> information such as SSRC, SEQ, CSRC, etc are set to the same as the primary
>>> payload.  The RED RTP packet is then encrypted in repair mode and sent.”:
>>> This is confusing; parts are copied from the primary encoding and others
>>> are the same as in the primary encoding? Don’t both of those mean the same
>>> thing?
>>>