Skip to content
  1. Feb 09, 2018
  3. Feb 02, 2018
  5. Feb 01, 2018
    • David Benjamin's avatar
      Don't leak the exponent bit width in BN_mod_exp_mont_consttime. · 4981e6fc
      David Benjamin authored 
      

The exponent here is one of d, dmp1, or dmq1 for RSA. This value and its
bit length are both secret. The only public upper bound is the bit width
of the corresponding modulus (RSA n, p, and q, respectively).

Although BN_num_bits is constant-time (sort of; see bn_correct_top notes
in preceding patch), this does not fix the root problem, which is that
the windows are based on the minimal bit width, not the upper bound. We
could use BN_num_bits(m), but BN_mod_exp_mont_consttime is public API
and may be called with larger exponents. Instead, use all top*BN_BITS2
bits in the BIGNUM. This is still sensitive to the long-standing
bn_correct_top leak, but we need to fix that regardless.

This may cause us to do a handful of extra multiplications for RSA keys
which are just above a whole number of words, but that is not a standard
RSA key size.

Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)

(cherry picked from commit 39eeb64f)
      4981e6fc
    • David Benjamin's avatar
      Make BN_num_bits_word constant-time. · 66509ddb
      David Benjamin authored 
      

(This patch was written by Andy Polyakov. I only wrote the commit
message. Mistakes in the analysis are my fault.)

BN_num_bits, by way of BN_num_bits_word, currently leaks the
most-significant word of its argument via branching and memory access
pattern.

BN_num_bits is called on RSA prime factors in various places. These have
public bit lengths, but all bits beyond the high bit are secret. This
fully resolves those cases.

There are a few places where BN_num_bits is called on an input where the
bit length is also secret. This does *not* fully resolve those cases as
we still only look at the top word. Today, that is guaranteed to be
non-zero, but only because of the long-standing bn_correct_top timing
leak. Once that is fixed, a constant-time BN_num_bits on such inputs
must count bits on each word.

Instead, those cases should not call BN_num_bits at all. In particular,
BN_mod_exp_mont_consttime uses the exponent bit width to pick windows,
but it should be using the maximum bit width. The next patch will fix
this.

Thanks to Dinghao Wu, Danfeng Zhang, Shuai Wang, Pei Wang, and Xiao Liu
for reporting this issue.

Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)

(cherry picked from commit 972c87df)
      66509ddb
  7. Jan 30, 2018
    • Matt Caswell's avatar
      Make sure we check an incoming reneg ClientHello in DTLS · d498e526
      Matt Caswell authored 
      

In TLS we have a check to make sure an incoming reneg ClientHello is
acceptable. The equivalent check is missing in the DTLS code. This means
that if a client does not signal the ability to handle secure reneg in the
initial handshake, then a subsequent reneg handshake should be rejected by
the server. In the DTLS case the reneg was being allowed if the the 2nd
ClientHello had a renegotiation_info extension. This is incorrect.

While incorrect, this does not represent a security issue because if
the renegotiation_info extension is present in the second ClientHello it
also has to be *correct*. Therefore this will only work if both the client
and server believe they are renegotiating, and both know the previous
Finished result. This is not the case in an insecure rengotiation attack.

I have also tidied up the check in the TLS code and given a better check
for determining whether we are renegotiating or not.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5192)
      d498e526
  9. Jan 24, 2018
  11. Jan 23, 2018
  13. Jan 22, 2018
  15. Jan 21, 2018
  17. Jan 16, 2018
    • Matt Caswell's avatar
      Revert BN_copy() flag copy semantics change · 8837a048
      Matt Caswell authored 
      Commit 9f944291

 changed the semantics of BN_copy() to additionally
copy the BN_FLG_CONSTTIME flag if it is set. This turns out to be
ill advised as it has unintended consequences. For example calling
BN_mod_inverse_no_branch() can sometimes return a result with the flag
set and sometimes not as a result. This can lead to later failures if we
go down code branches that do not support constant time, but check for
the presence of the flag.

The original commit was made due to an issue in BN_MOD_CTX_set(). The
original PR fixed the problem in that function, but it was changed in
review to fix it in BN_copy() instead. The solution seems to be to revert
the BN_copy() change and go back to the originally proposed way.

Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/5080)

(cherry picked from commit 7d461736)
      8837a048
  19. Jan 09, 2018
    • Matt Caswell's avatar
      Tolerate DTLS alerts with an incorrect version number · da9ed725
      Matt Caswell authored 
      

In the case of a protocol version alert being sent by a peer the record
version number may not be what we are expecting. In DTLS records with an
unexpected version number are silently discarded. This probably isn't
appropriate for alerts, so we tolerate a mismatch in the minor version
number.

This resolves an issue reported on openssl-users where an OpenSSL server
chose DTLS1.0 but the client was DTLS1.2 only and sent a protocol_version
alert with a 1.2 record number. This was silently ignored by the server.

Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5019)
      da9ed725
  21. Jan 07, 2018
  23. Jan 06, 2018
  25. Jan 05, 2018
  27. Dec 27, 2017
  29. Dec 23, 2017
  31. Dec 13, 2017
  33. Dec 11, 2017
  35. Dec 10, 2017
  37. Dec 09, 2017
  39. Dec 08, 2017
  41. Dec 07, 2017
  43. Dec 06, 2017
    • Matt Caswell's avatar
      Add a test for CVE-2017-3737 · c7383fb5
      Matt Caswell authored 
      

Test reading/writing to an SSL object after a fatal error has been
detected.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      c7383fb5
    • Matt Caswell's avatar
      Don't allow read/write after fatal error · 898fb884
      Matt Caswell authored 
      

OpenSSL 1.0.2 (starting from version 1.0.2b) introduced an "error state"
mechanism. The intent was that if a fatal error occurred during a handshake
then OpenSSL would move into the error state and would immediately fail if
you attempted to continue the handshake. This works as designed for the
explicit handshake functions (SSL_do_handshake(), SSL_accept() and
SSL_connect()), however due to a bug it does not work correctly if
SSL_read() or SSL_write() is called directly. In that scenario, if the
handshake fails then a fatal error will be returned in the initial function
call. If SSL_read()/SSL_write() is subsequently called by the application
for the same SSL object then it will succeed and the data is passed without
being decrypted/encrypted directly from the SSL/TLS record layer.

In order to exploit this issue an attacker would have to trick an
application into behaving incorrectly by issuing an SSL_read()/SSL_write()
after having already received a fatal error.

Thanks to David Benjamin (Google) for reporting this issue and suggesting
this fix.

CVE-2017-3737

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      898fb884
    • Andy Polyakov's avatar
      bn/asm/rsaz-avx2.pl: fix digit correction bug in rsaz_1024_mul_avx2. · ca51bafc
      Andy Polyakov authored 
      

Credit to OSS-Fuzz for finding this.

CVE-2017-3738

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      ca51bafc