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

Ben Campbell <ben@nostrum.com> Tue, 05 June 2018 04:40 UTC

Return-Path: <ben@nostrum.com>
X-Original-To: perc@ietfa.amsl.com
Delivered-To: perc@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id DEE99130EBA; Mon, 4 Jun 2018 21:40:16 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.88
X-Spam-Level:
X-Spam-Status: No, score=-1.88 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, T_SPF_HELO_PERMERROR=0.01, T_SPF_PERMERROR=0.01] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YJZSw0FAHApu; Mon, 4 Jun 2018 21:40:14 -0700 (PDT)
Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 16768130EB9; Mon, 4 Jun 2018 21:40:10 -0700 (PDT)
Received: from [10.0.1.83] (cpe-66-25-7-22.tx.res.rr.com [66.25.7.22]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id w554e7b0028479 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Mon, 4 Jun 2018 23:40:08 -0500 (CDT) (envelope-from ben@nostrum.com)
X-Authentication-Warning: raven.nostrum.com: Host cpe-66-25-7-22.tx.res.rr.com [66.25.7.22] claimed to be [10.0.1.83]
From: Ben Campbell <ben@nostrum.com>
Content-Type: multipart/signed; boundary="Apple-Mail=_A2F5B906-17AF-4744-8F6B-7BEF336E8124"; protocol="application/pgp-signature"; micalg="pgp-sha512"
Mime-Version: 1.0 (Mac OS X Mail 11.4 \(3445.8.2\))
Message-Id: <5AFC524E-7C2A-4C8F-B66B-7BAA3F048403@nostrum.com>
Date: Mon, 04 Jun 2018 23:40:06 -0500
Cc: perc@ietf.org
To: draft-ietf-perc-double.all@ietf.org
X-Mailer: Apple Mail (2.3445.8.2)
Archived-At: <https://mailarchive.ietf.org/arch/msg/perc/TpJyOUWRKTUJaPQuFKMA_zAC2i4>
Subject: [Perc] AD Evaluation of draft-ietf-perc-double-09
X-BeenThere: perc@ietf.org
X-Mailman-Version: 2.1.26
Precedence: list
List-Id: Privacy Enhanced RTP Conferencing <perc.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/perc>, <mailto:perc-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/perc/>
List-Post: <mailto:perc@ietf.org>
List-Help: <mailto:perc-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/perc>, <mailto:perc-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 05 Jun 2018 04:40:17 -0000

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.)

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

§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.

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)

§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.)

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

§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?

*** 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

§2:
- Please use the boilerplate from RFC8174. There are a number of lower case instances of normative keywords.
- Please use consistent sentence (or lack of sentence) structure for the definitions in the bullet list.

§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.

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

§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.

§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?

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

§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.)

- “ The algorithm recommend…” s / recommend / recommended

§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”.

§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”.

§9:
- “... all the RTP fields except headers created by the sender…” Does that mean all the fields created by the sender except headers? Or all the fields other than the headers created by the sender?

- s/ + / and

- “… 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.” - hard to parse

- " 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.” s/ “consider including the header” / “include the header”

- s/ “It is obviously critical” / “It is critical”

- “… the outer (hop-by-hop) algorithm key and not the half of the key … “ inconsistent in talking about H2H key vs half-key.

 - 11: It’s awkward to use “Thank you” when addressing 3rd persons. I suggest something like “Thanks for …  go to …” or “The authors would like to thank … for…”

- 12.2: It seems like some of these need to be normative:  [I-D.ietf-payload-flexible-fec-scheme], maybe [I-D.ietf-perc-private-media-framework], and [RFC4733].

- Appendix A: Why is this relegated to an appendix? I think would be helpful in the main body of the document.