Review of draft-igoe-secsh-aes-gcm-00.txt

[Tero Kivinen <kivinen%iki.fi@localhost>]

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.

The document describes how to use combined mode aes-gcm ciphers in the
secure shell. I think this document have several problems:


1) The draft does not define how the per encryption IV is defined. I
   assume it is some kind of counter with most likely initial value
   and fixed part taken from the appropriate IV generated by the secsh
   key derivation part, but also things like byte order of the counter
   etc needs to be defined. RFC 5116 provides just generic
   instructions for the nonce generation (generation of distinct
   nonces for each operation for given key) in section 3.1 and then
   provides recommended format for the nonce in section 3.2, and then
   there is another format for the nonce in section 3.2.1, but nothing
   in this document says which format is used.

2) This is first combined mode cipher for the secsh, so it should also
   provide information required to support combined mode ciphers in
   secsh. For example it should say that combined mode encryption is
   run only once and what key it is using (most likely the encryption
   key from section 7.2 from RFC 4253 etc). It also needs to say that
   mac generation DOES NOT follow section 6.4 of RFC 4253 (i.e. no
   MAC(key, sequence_number || unencrypted_packet)).

3) I am not sure if there is need adding sequence_number to the
   authenticated data of the combined mode. That sequence_number is
   added to the mac of the normal secsh modes, but as I think the
   aes-gcm mode will provide same protection by using the IV in a
   format of counter, but I am not sure if that is enough. At least it
   should be mentioned why it is not needed.

4) Section 7 says that it is "SHOULD" to use same aead-aes-*-gcm-ssh
   for both confidentiality and data integrity mechanism. That implies
   that it would be valid to use for example 3des as encryption
   algorithm and aead-aes-128-gcm-ssh as integrity algorithm. This
   causes few problems, as both of those algorithms require IV, thus
   the draft needs to defined how the Initial IV of the section 7.2
   RFC 4253 needs to be splitted between 3des IV and
   aead-aes-128-gcm-ssh IV. Another example would be that if someone
   decides to use aead-aes-256-gcm-ssh as encryption algorithm and
   aead-aes-128-gcm-ssh for the integrity then the aes-gcm needs to be
   run twice, and now the key for the aead-aes-256-gcm-ssh is taken
   from encryption key and the key for the aead-aes-128-gcm-ssh is
   taken from the integrity key (and the separate IV for both needs to
   be taken from Initial IV field). I would suggest way mandate that
   same aead-aes-*-gcm-ssh algorithm MUST be selected for encryption
   and integrity algorithms (perhaps with only one exception when
   using none encryption and aead-aes-*-gcm-ssh as integrity
   algorithm).

5) Next question then if encryption and integrity algorithms happen to
   be same combined mode ciphers (in case we keep "SHOULD" in point 4,
   and allow mixing different combined mode ciphers for both
   encryption and integrity alrogithms), does that still mean that we
   need to run the aes-gcm twice with different IVs and keys
   (encryption key and integrity key)? My guess is that you do NOT
   want to run it twice, but you only want to run it once using the
   encryption key and one Initial IV and use output of that for both
   encryption and mac.

6) This should have reference to the RFC 4253, as that is the actual
   document using the algorithms, and this really actually modifies
   that documents at least from mac generation part. Also the question
   is that as this extends standard track document with informational
   document, that does not just define new ciphers, but also changes
   the base document, should this document be on the standard track
   also? 

In general I think this document requires still quite a lot of more
text to describe how to use combined mode ciphers in the secsh. Bit
like we needed to have much more description in the RFC 5282 when the
AEAD modes where defined to be used in the IKEv2 SAs (I do not think
secsh needs that much text as RFC5282 as secsh case is simplier, but
it still needs something).
-- 
kivinen%iki.fi@localhost