subtract buffer size from computed rekey limit to avoid exceeding it #19
+14
−3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Hi!
This 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 failederror.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:
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 we 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:
openssh-portable/packet.c
Line 1003 in 3f4ea3c
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 inclientloopandserverloopafter 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, 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).
Our proposed fix is to deduce the maximum theoretical amount of buffered blocks from the computed
max_blocksvalue.