Skip to content
  1. Feb 06, 2018
  2. Feb 05, 2018
  3. Feb 01, 2018
    • David Benjamin's avatar
      Fix timing leak in BN_from_montgomery_word. · f345b1f3
      David Benjamin authored
      BN_from_montgomery_word doesn't have a constant memory access pattern.
      Replace the pointer trick with a constant-time select. There is, of
      course, still the bn_correct_top leak pervasive in BIGNUM itself.
      
      See also https://boringssl-review.googlesource.com/22904
      
       from BoringSSL.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
      (Merged from https://github.com/openssl/openssl/pull/5228)
      f345b1f3
    • David Benjamin's avatar
      Don't leak the exponent bit width in BN_mod_exp_mont_consttime. · 39eeb64f
      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)
      39eeb64f
    • David Benjamin's avatar
      Make BN_num_bits_word constant-time. · 972c87df
      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)
      972c87df
    • Todd Short's avatar
      Add TLSv1.3 post-handshake authentication (PHA) · 9d75dce3
      Todd Short authored
      
      
      Add SSL_verify_client_post_handshake() for servers to initiate PHA
      
      Add SSL_force_post_handshake_auth() for clients that don't have certificates
      initially configured, but use a certificate callback.
      
      Update SSL_CTX_set_verify()/SSL_set_verify() mode:
      
      * Add SSL_VERIFY_POST_HANDSHAKE to postpone client authentication until after
      the initial handshake.
      
      * Update SSL_VERIFY_CLIENT_ONCE now only sends out one CertRequest regardless
      of when the certificate authentication takes place; either initial handshake,
      re-negotiation, or post-handshake authentication.
      
      Add 'RequestPostHandshake' and 'RequirePostHandshake' SSL_CONF options that
      add the SSL_VERIFY_POST_HANDSHAKE to the 'Request' and 'Require' options
      
      Add support to s_client:
      * Enabled automatically when cert is configured
      * Can be forced enabled via -force_pha
      
      Add support to s_server:
      * Use 'c' to invoke PHA in s_server
      * Remove some dead code
      
      Update documentation
      
      Update unit tests:
      * Illegal use of PHA extension
      * TLSv1.3 certificate tests
      
      DTLS and TLS behave ever-so-slightly differently. So, when DTLS1.3 is
      implemented, it's PHA support state machine may need to be different.
      Add a TODO and a #error
      
      Update handshake context to deal with PHA.
      
      The handshake context for TLSv1.3 post-handshake auth is up through the
      ClientFinish message, plus the CertificateRequest message. Subsequent
      Certificate, CertificateVerify, and Finish messages are based on this
      handshake context (not the Certificate message per se, but it's included
      after the hash). KeyUpdate, NewSessionTicket, and prior Certificate
      Request messages are not included in post-handshake authentication.
      
      After the ClientFinished message is processed, save off the digest state
      for future post-handshake authentication. When post-handshake auth occurs,
      copy over the saved handshake context into the "main" handshake digest.
      This effectively discards the any KeyUpdate or NewSessionTicket messages
      and any prior post-handshake authentication.
      
      This, of course, assumes that the ID-22 did not mean to include any
      previous post-handshake authentication into the new handshake transcript.
      This is implied by section 4.4.1 that lists messages only up to the
      first ClientFinished.
      
      Reviewed-by: default avatarBen Kaduk <kaduk@mit.edu>
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/4964)
      9d75dce3
  4. Jan 31, 2018
    • Andy Polyakov's avatar
      6b6981ef
    • Benjamin Kaduk's avatar
      Restore clearing of init_lock after free · adeb4bc7
      Benjamin Kaduk authored
      
      
      The behavior of resetting the init_lock value to NULL after
      freeing it during OPENSSL_cleanup() was added as part of the
      global lock commits that were just reverted, but there is desire
      to retain this behavior for clarity.
      
      It is unclear that the library would actually remain usable in
      any form after OPENSSL_cleanup(), since the required re-initialization
      occurs under a CRYPTO_ONCE check that cannot be reset at cleanup time.
      That said, a NULL dereference is probably more friendly behavior
      in these treacherous waters than using freed memory would be.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
      Reviewed-by: default avatarMatthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
      (Merged from https://github.com/openssl/openssl/pull/5089)
      adeb4bc7
    • Benjamin Kaduk's avatar
      Revert the crypto "global lock" implementation · 63ab5ea1
      Benjamin Kaduk authored
      Conceptually, this is a squashed version of:
      
          Revert "Address feedback"
      
          This reverts commit 75551e07.
      
      and
      
          Revert "Add CRYPTO_thread_glock_new"
      
          This reverts commit ed6b2c79
      
      .
      
      But there were some intervening commits that made neither revert apply
      cleanly, so instead do it all as one shot.
      
      The crypto global locks were an attempt to cope with the awkward
      POSIX semantics for pthread_atfork(); its documentation (the "RATIONALE"
      section) indicates that the expected usage is to have the prefork handler
      lock all "global" locks, and the parent and child handlers release those
      locks, to ensure that forking happens with a consistent (lock) state.
      However, the set of functions available in the child process is limited
      to async-signal-safe functions, and pthread_mutex_unlock() is not on
      the list of async-signal-safe functions!  The only synchronization
      primitives that are async-signal-safe are the semaphore primitives,
      which are not really appropriate for general-purpose usage.
      
      However, the state consistency problem that the global locks were
      attempting to solve is not actually a serious problem, particularly for
      OpenSSL.  That is, we can consider four cases of forking application
      that might use OpenSSL:
      
      (1) Single-threaded, does not call into OpenSSL in the child (e.g.,
      the child calls exec() immediately)
      
      For this class of process, no locking is needed at all, since there is
      only ever a single thread of execution and the only reentrancy is due to
      signal handlers (which are themselves limited to async-signal-safe
      operation and should not be doing much work at all).
      
      (2) Single-threaded, calls into OpenSSL after fork()
      
      The application must ensure that it does not fork() with an unexpected
      lock held (that is, one that would get unlocked in the parent but
      accidentally remain locked in the child and cause deadlock).  Since
      OpenSSL does not expose any of its internal locks to the application
      and the application is single-threaded, the OpenSSL internal locks
      will be unlocked for the fork(), and the state will be consistent.
      (OpenSSL will need to reseed its PRNG in the child, but that is
      an orthogonal issue.)  If the application makes use of locks from
      libcrypto, proper handling for those locks is the responsibility of
      the application, as for any other locking primitive that is available
      for application programming.
      
      (3) Multi-threaded, does not call into OpenSSL after fork()
      
      As for (1), the OpenSSL state is only relevant in the parent, so
      no particular fork()-related handling is needed.  The internal locks
      are relevant, but there is no interaction with the child to consider.
      
      (4) Multi-threaded, calls into OpenSSL after fork()
      
      This is the case where the pthread_atfork() hooks to ensure that all
      global locks are in a known state across fork() would come into play,
      per the above discussion.  However, these "calls into OpenSSL after
      fork()" are still subject to the restriction to async-signal-safe
      functions.  Since OpenSSL uses all sorts of locking and libc functions
      that are not on the list of safe functions (e.g., malloc()), this
      case is not currently usable and is unlikely to ever be usable,
      independently of the locking situation.  So, there is no need to
      go through contortions to attempt to support this case in the one small
      area of locking interaction with fork().
      
      In light of the above analysis (thanks @davidben and @achernya), go
      back to the simpler implementation that does not need to distinguish
      "library-global" locks or to have complicated atfork handling for locks.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
      Reviewed-by: default avatarMatthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
      (Merged from https://github.com/openssl/openssl/pull/5089)
      63ab5ea1
    • Richard Levitte's avatar
      Remove "dummy" BIO create and destroy functions · 94f1c937
      Richard Levitte authored
      
      
      They aren't needed if all they do is set bio->init = 1 and zero other
      fields that are already zeroed
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/5223)
      94f1c937
    • Richard Levitte's avatar
      BIO: at the end of BIO_new, declare the BIO inited if no create method present · 7f55808f
      Richard Levitte authored
      
      
      Without this, every BIO implementation is forced to have a create
      method, just to set bio->init = 1.
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/5223)
      7f55808f
    • Dr. Matthias St. Pierre's avatar
      crypto/rand/rand_lib.c: fix undefined reference to `clock_gettime' · 2e230e86
      Dr. Matthias St. Pierre authored
      
      
      Some older glibc versions require the `-lrt` linker option for
      resolving the reference to `clock_gettime'. Since it is not desired
      to add new library dependencies in version 1.1.1, the call to
      clock_gettime() is replaced by a call to gettimeofday() for the
      moment. It will be added back in version 1.2.
      
      Signed-off-by: default avatarDr. Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
      
      Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
      Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
      (Merged from https://github.com/openssl/openssl/pull/5199)
      2e230e86
  5. Jan 29, 2018
  6. Jan 28, 2018
  7. Jan 25, 2018
    • Benjamin Kaduk's avatar
      Add support for the TLS 1.3 signature_algorithms_cert extension · c589c34e
      Benjamin Kaduk authored
      
      
      The new extension is like signature_algorithms, but only for the
      signature *on* the certificate we will present to the peer (the
      old signature_algorithms extension is still used for signatures that
      we *generate*, i.e., those over TLS data structures).
      
      We do not need to generate this extension, since we are the same
      implementation as our X.509 stack and can handle the same types
      of signatures, but we need to be prepared to receive it, and use the received
      information when selecting what certificate to present.
      
      There is a lot of interplay between signature_algorithms_cert and
      signature_algorithms, since both affect what certificate we can
      use, and thus the resulting signature algorithm used for TLS messages.
      So, apply signature_algorithms_cert (if present) as a filter on what
      certificates we can consider when choosing a certificate+sigalg
      pair.
      
      As part of this addition, we also remove the fallback code that let
      keys of type EVP_PKEY_RSA be used to generate RSA-PSS signatures -- the
      new rsa_pss_pss_* and rsa_pss_rsae_* signature schemes have pulled
      the key type into what is covered by the signature algorithm, so
      we should not apply this sort of compatibility workaround.
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/5068)
      c589c34e
    • Bernd Edlinger's avatar
  8. Jan 24, 2018
  9. Jan 23, 2018
  10. Jan 22, 2018
  11. Jan 21, 2018
  12. Jan 19, 2018
  13. Jan 18, 2018
  14. Jan 16, 2018
  15. Jan 09, 2018
  16. Jan 08, 2018