[AVTCORE] draft-ietf-avtcore-srtp-aes-gcm-10 review

John Mattsson <john.mattsson@ericsson.com> Mon, 14 October 2013 13:12 UTC

Return-Path: <john.mattsson@ericsson.com>
X-Original-To: avt@ietfa.amsl.com
Delivered-To: avt@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 08FC011E8186 for <avt@ietfa.amsl.com>; Mon, 14 Oct 2013 06:12:25 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -6.274
X-Spam-Level:
X-Spam-Status: No, score=-6.274 tagged_above=-999 required=5 tests=[AWL=-1.675, BAYES_00=-2.599, GB_I_LETTER=-2]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id QF2y97qg+eZd for <avt@ietfa.amsl.com>; Mon, 14 Oct 2013 06:12:19 -0700 (PDT)
Received: from sesbmg20.ericsson.net (sesbmg20.ericsson.net [193.180.251.56]) by ietfa.amsl.com (Postfix) with ESMTP id BFBF111E818A for <avt@ietf.org>; Mon, 14 Oct 2013 06:12:18 -0700 (PDT)
X-AuditID: c1b4fb38-b7fcf8e0000062b8-eb-525bedb04239
Received: from ESESSHC024.ericsson.se (Unknown_Domain [153.88.253.125]) by sesbmg20.ericsson.net (Symantec Mail Security) with SMTP id 84.AA.25272.0BDEB525; Mon, 14 Oct 2013 15:12:17 +0200 (CEST)
Received: from ESESSMB307.ericsson.se ([169.254.7.84]) by ESESSHC024.ericsson.se ([153.88.183.90]) with mapi id 14.02.0328.009; Mon, 14 Oct 2013 15:09:47 +0200
From: John Mattsson <john.mattsson@ericsson.com>
To: "avt@ietf.org" <avt@ietf.org>, "draft-ietf-avtcore-srtp-aes-gcm@tools.ietf.org" <draft-ietf-avtcore-srtp-aes-gcm@tools.ietf.org>
Thread-Topic: draft-ietf-avtcore-srtp-aes-gcm-10 review
Thread-Index: Ac7I3KoMq4YJWcF+RZ2qUP93gMGt0A==
Date: Mon, 14 Oct 2013 13:09:46 +0000
Message-ID: <CAAB765F71FCD344B6BABC031C19EC490C9AB4@ESESSMB307.ericsson.se>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [153.88.183.20]
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrGLMWRmVeSWpSXmKPExsUyM+Jvre7Gt9FBBod3S1m87FnJbrH2SKID k8eSJT+ZPL5c/swWwBTFZZOSmpNZllqkb5fAlfH0h1dBa1TFxLULWBsYP7t3MXJySAiYSJw5 uJsVwhaTuHBvPVsXIxeHkMBRRolra/+ygySEBBYzSpyfnAdiswkYSMzd08AGYosIdDBKdO2X ALGFBYwkXmzfARTnAIqbS2zb7g9Roifxp3kfM4jNIqAq8eTHXjCbV8BbYufmk2A2I9De76fW MIHYzALiEreezGeCuEdAYsme88wQtqjEy8f/oO5UlLg6fTlUvZ7EjalT2CBsbYllC19DzReU ODnzCcsERuFZSMbOQtIyC0nLLCQtCxhZVjFyFKcWJ+WmGxlsYgSG9cEtvy12MF7+a3OIUZqD RUmc9+Nb5yAhgfTEktTs1NSC1KL4otKc1OJDjEwcnFINjOevvPzPxC5yZ06M9O3ILYFKzB2B t9LmfZ7KVNoceSmzuUnT20Gf/++1H6+2GzZ3PnDpPnQy16S8SMg4SKVUTbPpqaVBofgTSetV xTs1Xuxz5P7498AFwSufT8nJarNeWZ66vaH8Rn3OmRvJ+RXsB3pWJJ9c8uvaWS6x0yyW/cZ7 J08zSSgTUmIpzkg01GIuKk4EAO/U2PY5AgAA
Subject: [AVTCORE] draft-ietf-avtcore-srtp-aes-gcm-10 review
X-BeenThere: avt@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: Audio/Video Transport Core Maintenance <avt.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/avt>, <mailto:avt-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/avt>
List-Post: <mailto:avt@ietf.org>
List-Help: <mailto:avt-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/avt>, <mailto:avt-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 14 Oct 2013 13:12:25 -0000

Hi,

I reviewed draft-ietf-avtcore-srtp-aes-gcm-10. Sorry for commenting after the publication request.

I support the publication, but I have some comments that should probably be addressed before proceeding to the IESG review:

/John


Comments and suggestions on draft-ietf-avtcore-srtp-aes-gcm-10
-------------------------------------------------------------------------

- [s6] The values for the maximum keystream/plaintext length do not seem to agree with the P_MAX values in [RFC5116]. Section 6 states:
"up to (2^28)-16 octets for AES-CCM and up to (2^36)-32 octets for AES-GCM."

[RFC5116] states:
"P_MAX is 2^36 - 31 octets" for GCM and "P_MAX is 2^24 - 1 octets" for CCM. 

While lowering the value for GCM with 1 octet would be ok, increasing the values with a factor of 16 for CCM would require some motivation. Might this this actually be a typo in [RFC5116]? 

Why were the values from [RFC5116] not followed directly? I assume it was done to match an integer number of blocks. I suggest adding a short motivation.

- [s11] The values for C_MAX in Section 11 do not agree with the values for keystream/plaintext length in Section 6. Section 11 states:

"C_MAX      maximum ciphertext       GCM: MUST be <= 2^36-16 octets.
            length per invocation    CCM: MUST be <= 2^28-16 octets."

As the minimum AEAD tag length is 8 octets this would mean that the maximum keystream length is 2^28-24. Assuming the values in Section 6 is correct, the correct value should probably be:
"CCM: MUST be <= 2^28 octets."

- [s14.1] The text describing the handling of the block counters do not seem correct. Section 14.1 states:
" At the start of each packet, the block counter MUST be reset (to 0 for CCM, to 1 for GCM). The block counter is incremented after each block key has been produced, but it MUST NOT be allowed to exceed 2^32 for GCM and 2^24 for CCM." 

1. While the number of blocks could exceed 2^32 for GCM and 2^24 for CCM, the block counter can never exceed 2^32-1 for GCM and 2^24-1 for CCM.

2. Even with a block counter value of 2^24-1, CCM would have exceeded maximum keystream length of (2^28)-16 octets specified in Section 6.

Assuming that the keystream limits in Section 6 are correct, I think the correct values should be:
"The block counter is incremented after each block key has been produced, but it MUST NOT be allowed to exceed (2^32) - 2 for GCM and (2^24) - 2 for CCM.

- The values for the maximum Ciphertext length for GCM_8 do not seem to agree with the constraints in Appendix C of 800-38D: "For any implementation that supports 32-bit or 64-bit tags, one of the rows in Table 1 or Table 2, respectively, shall be enforced." This seems to be missing in RFC5285 as well. 

- The values for the maximum number on invocations (2^48 and 2^31) for GCM_8 do not seem to agree with the constraints in Appendix C of 800-38D. This seems to be missing in RFC5285 as well.

I have not had time to read the actual papers behind Appendix B and C in 800-38D, but if Appendix C for some reason do not apply to draft-ietf-avtcore-srtp-aes-gcm-10 (and RFC5285), this should be mentioned and motivated. Otherwise, would there be any additional constraints on Ciphertext length and maximum number on invocations for GCM_12?

- The document decreases the session salt length from the RFC3711 default of 112 to 96. Whether it is to follow the recommendations in RFC5116 or allowing longer keystreams, it should be motivated. And even if the negative security implications against pre-computation and TMTO attacks are theoretical (2^96 keys in use instead of 2^112) the negative security implications compared to RFC3711 should be discussed (shortly).

- The document decreases the session salt length from the RFC3711 default of 112 to 96, but it does not mention weather the default length of the master salt is also changed to 96 or if it is still 112.

- There is a lot of "optional", "must" and "not recommended" etc. written with lower case. Both in text and figures. Should all be CAPITAL I think. 

- [s3] As this section gives an overview of the security architecture in the draft rather than general SRTP I would suggest:
"SRTP/SRTCP security" -> "SRTP/SRTCP AEAD security"

- [p4] "A Key Derivation Function is applied to the shared master key value to form separate encryption keys, authentication keys and salting keys for SRTP and for SRTCP (a total of six keys).  This process is described in sections 4.3.1 and 4.3.3 of [RFC3711]."

Key derivation for SRTCP is described in section 4.3.2. Maybe best to write "This process is described in section 4.3 of [RFC3711]."

- [s4] The defined terms are barely used at all:
"Crypto Context" is not used at all in the text.
"Instantiation" is used exactly 1 time after the definition.
"Invocation" is used only 3 times. 

Should be fixed in one way or another.

- [s5.2] 5.2.1 specifies the encrypt input for Associated data and Plaintext as "Bit string", while 5.2.2 defines them defines them as "Octet string". I assume both should be "Octet string". 

- [s5.2.1] "cannot", should this maybe be "MUST NOT" or "SHOULD NOT" instead?

- [s6] As the output of GCM_keystream and CCM_keystream is key_stream and not Ciphertext, the input should probably be len(Plaintext) instead of Plaintext.

- [s8.2] Should probably use ""NOT RECOMMENDED".

- [s8.2] Maybe give VBR codecs as an example.

- [s9.2] Marker (M) and Payload Type (PT) is missing from the list of Associated Data fields.

- [Figure 2] While PT means "Packet Type" in RTCP it means "Payload Type" in RTP.

- [Figure 2,4,5] I suggest writing "SRTP authentication tag" and "SRTCP authentication tag" instead of just "authentication tag".

- [s9.2] In the paragraph below Figure 2, shoudn't "cipher" be "ciphertext".

- [s9] Figure 2,4,5 does not show AEAD input as SRT(C)P MKI and SRT(C)P authentication tag are appended after encryption (and removed before decryption). As the packet processing from RFC3711 is not changed (nor should it be) the packet will never look like Figure 2,4,5 during the processing. 

I would suggest changing the pictures to show:
"Format of an AEAD SRTP packet"
"Format of an AEAD SRTCP packet when encryption flag = 1"
"Format of an AEAD SRTCP packet when encryption flag = 0"

which would then include the new "AEAD authentication tag".

- [s9.3] "Additional Authenticated Data (AAD)" Is it not best to just use Associated Data (AD) consistently in the whole document?

- [s9.2, s10.2, s10.3] The SRT(C)P MKI and the SRT(C)P authentication tag both have variable lengths (i.e. not 32 bit). 

- [Figure 4,5] sender info, report block 1, report block 2, ..., SDES items, ..., SRTCP MKI all have variable lengths (i.e. use ":" not "|"). 

- [Figure 4,5] The figures replaces the text "PT=SDES=202" with the general "Packet Type" but still keeps "SDES items".

- [s13] In the tables, shouldn't "Default key lifetime" be "Maximum key lifetime"

- [s13] In the tables, "Cipher (for SRTP and SRTCP)" includes tag length but not key length. Should include both or none.

- [s14.1] As all the bullets contain "MUST", they are maybe more requirements than recommendations

- [s14.1] "Each time a rekey occurs, the initial values of the SRTCP index and the values of all the SEQ counters MUST be saved."
Save initial SEQ value? Do you mean something like:
"the current values of the SRTCP index and the SRTP index MUST be not be changed"

- [s14.1] "Packet Counter" this is called "SRTP index" or "packet index" in RFC3711. I suggest using one of these terms (and as SEQ starts on a random value, it is not really a counter either).

- [s14.1] I would suggest "Processing MUST cease if" -> "Processing MUST cease before" to avoid ambiguity of what to do with the last packet.

- [s14.2] " The goal of an authentication tag is to minimize the probability" As this is not minimize in the mathematical way, I suggest to use some synonym like "reduce", or reformulate.

- [s14.2] Strictly, prob_success <= expected number of successes (equal  for N_tries = 0,1).

- [s14.2] I suggest to mention that:
   if   expected number of successes << 1 
   then prob_success approx. expected number of successes
Otherwise it is hard to understand that the table contains approximate and quite exact values and not only maximum bounds for the probabilities.

- [s14.2] "2^-N_tries" is correct for tags in general, but isn't the correct value for GCM "ceiling(len(Ciphertext + AD) / 16) * 2^-N_tries"? (c.f. Appendix B of 800-38D).

- [s15.1] "In order to allow SDP to signal". As RFC4567 could also be used, I suggest changing "SDP" to "Security Descriptions" or "SDP Security Descriptions" 

- [s15.1] I think the correct registry is "Session Description Protocol (SDP) Security Descriptions".

- [s15.3] As the "AEAD authentication tag length" is general and may be used for future AEAD algorithms, the Possible values should be "depends on cipher used"

- [s15.3] If there is a default key length, there should also be a default "AEAD authentication tag length" (8 octets).

- [s16] I suggest changing "AEAD" to "AEAD_AES" or "AES-GCM and AES-CCM" as Section 12 is not that general.


Editorial Comments on draft-ietf-avtcore-srtp-aes-gcm-10
-------------------------------------------------------------------------

- Some of the section headings miss to capitalize all the first letters.

- [p1] "each AEAD will" -> "each AEAD algorithm will"

- [p4] " number(SEQ)" -> " number (SEQ)"

- [p5] "Cipher Context" -> "Crypto Context"

- [p6] "Octets long" -> "octets long"

- [p7] "9.1 and 10.1)with" -> "9.1 and 10.1) with"

- [s9.1] "4-octer" -> "4-octet"

- [9.1, 10.1] "12 octet" -> "12-octet"

- Figure 1 and 3 uses different approaches to enumerate the bytes. 

- [s9.2] "see figure 2" -> "see Figure 2"

- [Figure 4] "SRTCP MKI (optional) index" -> "SRTCP MKI (OPTIONAL)"

- [s13] Caption is above Table 1 and 8 but below all other tables.

- [s13.2] "CBC MAC" -> "CBC-MAC"

- [s15.3] Delete "lists maintained under "MIKEY Security Protocol Parameters"."

- [s16] "length )" -> "length)"
	
- [s16] "the key length of the Key Derivation Function"
     -> "the input key length of the Key Derivation Function"

- [Table 16] remove space before 16:
   "AEAD_AES_128_GCM_12  |  AES-GCM   |  16 octets  | 12 octets   |"
-> "AEAD_AES_128_GCM_12  |  AES-GCM   | 16 octets   | 12 octets   |"

- [s17] If you would like to thank my Ericsson colleagues, their names are "Westerlund" and "Ohlsson"

(BTW the pdf version of the document has strange page breaks...)


------------------------------------------------------------------------------------
JOHN MATTSSON
MSc Engineering Physics, MSc Business Administration and Economics 
Senior Researcher, Security 

Ericsson AB
Security Research
Färögatan 6
SE-164 80 Stockholm, Sweden
Phone +46 10 71 43 501
SMS/MMS +46 76 11 53 501
john.mattsson@ericsson.com
www.ericsson.com




This Communication is Confidential. We only send and receive email on the basis of the terms set out at www.ericsson.com/email_disclaimer