Subject: Re: [AVTCORE] Working group last call on draft-ietf-avtcore-srtp-aes-gcm-03

From: Magnus Westerlund <[email-masked]>
Date: Mon, 21 Jan 2013 13:45:01 +0100

WG,

This WG last call ended before Christmas. I appreciate that a couple of
persons did take the time to review the document. The authors will need
to review and address the comments, making updates where appropriate.
When this is done we can hopefully request publication, but the nature
of the changes may require additional steps prior to that.

Cheers

Magnus

On 2012-11-27 15:20, Magnus Westerlund wrote:
> WG,
> 
> This announces the WG last call on AES-GCM and AES-CCM Authenticated
> Encryption in Secure RTP (SRTP)
> https://datatracker.ietf.org/doc/draft-ietf-avtcore-srtp-aes-gcm/
> 
> Please provide any comments no later than the 12th of December. Also
> comments of the nature of "I have read it and have no comments and think
> it should be published" are highly valuable.
> 
> Cheers
> 
> Magnus Westerlund
> 
> ----------------------------------------------------------------------
> Multimedia Technologies, Ericsson Research EAB/TVM
> ----------------------------------------------------------------------
> Ericsson AB                | Phone  +46 10 7148287
> Färögatan 6                | Mobile +46 73 0949079
> SE-164 80 Stockholm, Sweden| mailto: [email-masked]
> ----------------------------------------------------------------------
> 
> _______________________________________________
> Audio/Video Transport Core Maintenance
> [email-masked]
> https://www.ietf.org/mailman/listinfo/avt
> 
> 

-- 

Magnus Westerlund

----------------------------------------------------------------------
Multimedia Technologies, Ericsson Research EAB/TVM
----------------------------------------------------------------------
Ericsson AB                | Phone  +46 10 7148287
Färögatan 6                | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden| mailto: [email-masked]
----------------------------------------------------------------------

---

From: "Igoe, Kevin M." <[email-masked]>
Date: Fri, 25 Jan 2013 19:03:54 +0000

I'll try to address these point in line.

(David: Point 13 is an MTU related issue & I could use your
guidance here.)

> -----Original Message-----
> From: Magnus Westerlund [mailto:[email-masked]]
> Sent: Monday, December 10, 2012 8:26 AM
> To: [email-masked]; EKR
> Cc: IETF AVTCore WG
> Subject: Re: [AVTCORE] Working group last call on draft-ietf-avtcore-
> srtp-aes-gcm-03
> 
> WG, Authors
> 
> EKR, please look at issue 8 and 16 below.
> 
> 
> I have made a personal review of this document. My comments are the
> following. I have not checked for overlap with already made comments.
> 
> 1) In the front page header: "Intended Status: Informational" Please
> change this to standards track.
Oops.  It SHALL be changed.
> 
> 2) Section 1: The first mentioning of SRTP, RTP etc should have
> references, that also applies to AES and the AES-GCM, AES-CCM.
Done.

> 
> 3) Section 4)
> 
>       Instantiation   In AEAD, an instantiation is an (Encryption_key,
>                       salt) pair together with all of the data
>                       structures (for example, counters) needed for it
>                       to function properly.  In SRTP/SRTCP, each
>                       endpoint will need two instantiations of the AEAD
>                       algorithm for each master key in its possession,
>                       one for SRTP and one for SRTCP.
> 
> When reading this I got hang up on this. I realized why when getting
> further one. First of all I had forgotten about the fact that the SSRC
> is provided at invocation time through the IV, rather than at
> Instantiation. So I don't think this text is wrong, I think the issue
> might be in Section 3, that needs an bullet 4 that maps down to
> invocation level by making clear that a particular packet is processed
> by forming the IV for that packet using SSRC, Roll-over counter and
> SSRC
> combined with selecting the instantiation that is relevant for this
> packet.
Here is a "bullet 4":

       d)  Each time an instantiation of AES-GCM or AES-CCM is invoked
           to encrypt and authenticate an SRTP or SRTCP data packet a
           new IV is used.  SRTP combines the 4-octet synchronization
           source (SSRC) identifier, the 4-octet rollover counter (ROC),
           and the 2-octer sequence number(SEQ) with the 12-octet
           encryptions salt to form a 12-octet IV (see section 9.1). 
           SRTCP combines the SSRC and 31-bit SRTCP index with the
           encryption salt to form a 12-octet IV (see section 10.1). 

> 
> 4) Section 6.
>    In both GCM and CCM, each outbound packet uses a 12-octet IV and an
>    encryption key to form two outputs, a 16-octet first_key_block which
>    is used informing the authentication tag and keystream of octets
>    which is XORed to the plaintext to form cipher.
> 
> I guess "informing" should be "in forming"? Is a "the" needed before
> "keystream"?
Mea culpe, changed for sake of correctness.
> 
> 5) Section 6:
>    When GCM is used, the concatenation of a 12-octet IV, with a 4-octet
>    block counter forms the input to AES.  This is used to build a
>    key_stream as follows:
> 
   When GCM is used, the concatenation of the 12-octet IV (see sections
   9.1 and 10.1) with the 4-octet block counter forms the input to AES. 
   This is used to build a key_stream as follows:

> Please add a forward pointer to where the IV is defined.
> 
> 6) Section 6, page 7: The example code, is clearly not full code, but
> does it allow usage of some commonly available code? If so I think the
> authors needs to consider if they can offer this code as "Code
> component" following the IPR license from the trust for Code
> Components.

Aside from the truncate operation and defining an AES_ENC routine,
this is executable python code. Python's truncate notation is a tad obscure, 
so I chose to use
    keystream = truncate(( key_stream, len(Plaintext) ) 
rather than Prython's
    keystream = keystream[:len(Plaintext)]

It's really intended to be pseudo-code. 

> Please see:
> http://trustee.ietf.org/license-info/
> 
> 7. Section 6,
>    These keystream generation processes allows for a keystream of
> length
>    of up to (2^24)-16 octets for AES-CCM and up to (2^36)-32 octets for
>    AES-GCM.
> 
> I wonder if the decimal values should be included in the above sentence
> to ease for humans.
> 
I'd prefer to stick with this definition.  Using the decimal expansion
just opens up the possibility of someone mistyping and putting the wrong 
decimal value into their code

> 8. Section 8.1. SRTP/SRTCP Authentication Field
> 
> I think this section is attempting the wrong approach to this issue.
> First of all as the algorithms include authentication I think they
> fulfill the mandates from SRTP on requiring authentication on RTCP.
> Secondly, I think the right approach to deal with the issue that you
> have no data is to have the algorithms define there usage of the
> authentication tag to be zero length. All the defined algorithm
> identifiers includes both key lengths and auth tag lengths. Thus
> writing
> this as "SHOULD NOT" is very strange, as all the other CIPHERS include
> the encryption + authentication with one ID. This one is no different.
> 
> This would then have implications on Section 9.2, 10.3, etc which
> mentions this field.
> 
The issue isn't that we aren't using authentication, but rather that
the authentication is integrated into the cipher and is not located 
in the field provided for it by SRTP/SRTCP.

> 9. Section 9.2 and 9.3: please consider to use some separator between
> the defined term and the definition, like:
> 
>     Counter Management: A rekey MUST be performed to establish a new
>                         master key before the (ROC,SEQ) pair cycles
> back
>                         to its original value.
Good idea. Done.
> 
> 10. Section 9.3:
>      SSRC Management    The set of all SSRC values must be partitioned
>                         into disjoint pools, one pool for each endpoint
>                         using the master key to originate outbound
>                         data.  Each such endpoint MUST only issue SSRC
>                         values from the pool it has been assigned.
>                         Further, each endpoint MUST maintain a history
>                         of outbound SSRC identifiers that it has issued
>                         within the lifetime of the current master key,
>                         and when a new synchronization source requests
>                         an SSRC identifier it MUST NOT be given an
>                         identifier that has been previously issued.  A
>                         rekey MUST be performed before its pool of SSRC
>                         values is exhausted.
> 
> I wonder if one should specifically discuss the cases where each
> end-point has its own master key and thus can use any SSRC value from a
> crypto perspective. This clearly apply to Security Description and
> DTLS-SRTP.

Hmm, I thought the text made it clear that SSRC reuse is only a problem
between endpoints using the same master key to originate data. Reading it 
over, I can see where this point might not be as clear as I had thought.  
I'm mulling over how to phrase this point more clearly.

> 
> 11. Section 10.1:
> 
>                   0  1  2  3  4  5  6  7  8  9 10 11
>                 +--+--+--+--+--+--+--+--+--+--+--+--+
>                 |00|00|    SSRC   |00|00|0+SRTCP Idx|---+
>                 +--+--+--+--+--+--+--+--+--+--+--+--+   |
>                                                         |
>                 +--+--+--+--+--+--+--+--+--+--+--+--+   |
>                 |         Encryption Salt           |->(+)
>                 +--+--+--+--+--+--+--+--+--+--+--+--+   |
>                                                         |
>                 +--+--+--+--+--+--+--+--+--+--+--+--+   |
>                 |       Initialization Vector       |<--+
>                 +--+--+--+--+--+--+--+--+--+--+--+--+
> 
> 
> Please clarify in this figure and text above that the Salt is the one
> derived for the SRTCP.
> 
Done. Diagrams now have SRTP Encryption Salt and SRTCP Encryption Salt.

> 12. Section 10.1:
> 
>    Using the terminology of section 8.2.1.  of [GCM], the first eight
>    octets of the IV are the fixed field and the last four octets are
> the
>    invocation field.
> 
> As I haven't look at the ref, this might be correct. But compared to
> the
> definition of Invocation in Section 3:
> 
>       Invocation      SRTP/SRTCP data streams are broken into packets.
>                       Each packet is processed by a single invocation
> of
>                       the appropriate instantiation of the AEAD
>                       algorithm.
> 
> This can't be correct. As an end-point can use multiple SSRC
> simultaneously, it looks like the SSRC should be considered an
> Invocation field, or?
> 
This is an effort to harmonize with NIST's documentation on AES-GCM,
where the input to AES consists of three fields:  a fixed field,
an invocation field, and a block counter field.  It would have been
clearer if NIST had used the name "invocation counter field" rather
than "invocation field". In SRTP, 0x0000 || SSRC XORed with the first 
6-octets of the SRTP Salt form the fixed field, ROC || SEQ XORed wit
the last 6-octets of the SRTP salt form the invocation field, and the
block-counter provides the last 4 bytes of the AES input. In SRTCP,
0x0000 || SSRC || 0x0000 XOR first 8 bytes of the SRTCP Salt are the 
fixed field, and the invocation field is the SRTCP index (padded out
with a single "0" bit in the msb) XOR last 4 octets of the SRTCP salt
form the invocation field.  

I can add language to sections 9.1 and 10.1 to explain this more fully.
Alternatively, since only a handful of us crypto-geeks would care about 
this point, I can just drop any attempt to match this up to NIST's
notation.  The latter is easier & in the long run probably less confusing.
.  

> 13.  Section 11.1:
> 
> The lower bound on C_MAX is obtained by subtracting
>    away a 20-octet IP header, 8-octet UDP header, and 12-octet RTP
>    header from the maximum transmission unit (MTU) of 2272.
> 
> Who's MTU is 2272? Considering SRTP I don't think there is any
> definition of "THE MTU" as stated in the above sentence.
I'll have to defer to my co-author on this point.

> 
> 14. Section 11.3
> 
> In addition to the flag octet used in counter node encryptionn,
>    AES-CCM authentications also useus a flag octet that conveys
>    information about the length of the authentication tag, length of
> the
>    block counter, and presence of additional authenticated data (see
>    section 2.2 of [RFC 3610]).
> 
> "counter node encryption" I guess this should be "mode"
Nea culpa.  Text corrected.
> 
> 15. Section 14.2:
> 
>    AEAD_AES_128_CCM_8
>         cipher:               AES_128_CCM
>         cipher_key_length:    128 bits
>         cipher_salt_length:   96 bits
>         maximum lifetime:     at most 2^31 SRTCP packets and
>                               at most 2^48 SRTP packets
> 
> Shouldn't this read for cipher: AES_128_CCM_8?
You suggest losing the AEAD prefix?  My motivation was that
by keeping it, it would be immediately obvious that the 
authentication is integral to the cipher, rather than being in the
authentication field allocated by SRTP/SRTCP.  I'm willing to remove
the AEAD prefix if that is the consensus.

> 
> Same errors exist for all the shorter tag lengths.
> 
> 16.
>   AEAD_AES_128_CCM
>         cipher:               AES_128_CCM
>         cipher_key_length:    128 bits
>         cipher_salt_length:   96 bits
>         maximum lifetime:     at most 2^31 SRTCP packets and
>                               at most 2^48 SRTP packets
> 
> Shouldn't these definition include the following values also:
> 
> auth_function: NULL
> auth_key_length: 0
> auth_tag_length: 0
>
> Thus completing the definitions that you are not using the regular
> authentication tag length?
> 
> That was all.
> 

Is the auth_function really NULL, or is it AES-GCM/CCM?  I'm willing
to go whichever way folks feel is clearer.  I'm leaning toward following
your suggestion.

> I think there is some few issues that absolutely must be sorted out
> before publishing this document. But I think it is in fairly good
> shape.
> I would expect a second WG last call on the changes before we proceed.
> But lets start with discussing the changes.
> 
> Cheers
> 
> Magnus Westerlund
>