[Bug 2521] New: subtract buffer size from computed rekey limit to avoid exceeding it

[bugzilla-daemon at bugzilla.mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=2521

            Bug ID: 2521
           Summary: subtract buffer size from computed rekey limit to
                    avoid exceeding it
           Product: Portable OpenSSH
           Version: 6.8p1
          Hardware: amd64
                OS: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: olo at fb.com

Created attachment 2778
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2778&action=edit
The patch to rekey limit computation, based on GitHub commit 2c48eb1

I'm refiling this report in Bugzilla as a follow-up to my GitHub pull
request https://github.com/openssh/openssh-portable/pull/19 (which went
largely unnoticed).

The pull request changes the way in which the rekey limit is computed
based on cipher block size to address a problem with OpenSSH going over
the intended limit.

But first, a short background story:

In 2013, Red Hat has introduced a patch for OpenSSL that adds some
additional checks to its GCM implementation:

https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20131111/1144834.html

These checks are based on recommendations from NIST SP 800-38D:

http://csrc.nist.gov/publications/nistpubs/800-38D/SP-800-38D.pdf

Among those, section 5.2.1.1 imposes a limit on plaintext length that
amounts to 64 GiB.

At Facebook, this was causing our scp transfers larger than 64 GiB to
die with a cipher_crypt: EVP_Cipher failed error.

The check implementing this limit has been recently rolled back by Red
Hat:

https://rhn.redhat.com/errata/RHBA-2015-0772.html

The reason for dropping it is stated in the package's ChangeLog:

Thu Mar 26 2015 Tomáš Mráz tmraz at redhat.com 1.0.1e-30.8
drop the AES-GCM restriction of 2^32 operations because the IV is
always 96 bits (32 bit fixed field + 64 bit invocation field)
According to our own analysis, the change does not remove an operations
count restriction (specified in Sec 8.3 of NIST SP 800-38D and
dependent on usage of a non-recommended IV configuration), but total
plaintext length restriction (specified in Sec 5.2.1.1, which is
unconditional).

Regardless of validity of the removed check, it has exposed what I
believe to be a bug in OpenSSH in the way that rekey limits (based on
data, instead of time) are handled.

Currently, if the rekey limit is not explicitly configured, it's
computed algorithmically based on the cipher's block size:
https://github.com/openssh/openssh-portable/blob/3f4ea3c9ab1d32d43c9222c4351f58ca11144156/packet.c#L1003

For a 128-bit block cipher like AES-GCM, this amounts to a limit of
exactly 64GiB - the same as the recommended by NIST.

However, since the check for exceeding the rekey limit (max_blocks_*
fields in the session state) is only performed in clientloop and
serverloop after processing a buffered batch of packets, the amount of
data encrypted/decrypted will almost always go above the limit for a
few blocks (depending on how much of them were in the buffer) before
rekeying is triggered.

In our case at Facebook, this was causing AES-GCM to go above the 64
GiB limit shortly before triggering rekeying and abort with an error,
unless a sufficiently lower RekeyLimit is explicitly set (which itself
can only be set to values less than 4GiB because of u32int being used,
but that's a different story).

My proposed fix is to deduce the maximum theoretical amount of buffered
blocks from the computed max_blocks value.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.