1. 06 Sep, 2019 7 commits
    • Nicola Tuveri's avatar
      Make BN_num_bits() consttime upon BN_FLG_CONSTTIME · b9a380f7
      Nicola Tuveri authored
      This issue was partially addressed by commit
      972c87df
      
      , which hardened its callee
      BN_num_bits_word() to avoid leaking the most-significant word of its
      argument via branching and memory access pattern.
      The commit message also reported:
      > 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.
      
      BN_num_bits() is called directly or indirectly (e.g., through
      BN_num_bytes() or BN_bn2binpad() ) in various parts of the `crypto/ec`
      code, notably in all the currently supported implementations of scalar
      multiplication (in the generic path through ec_scalar_mul_ladder() as
      well as in dedicated methods like ecp_nistp{224,256,521}.c and
      ecp_nistz256.c).
      
      Under the right conditions, a motivated SCA attacker could retrieve the
      secret bitlength of a secret nonce through this vulnerability,
      potentially leading, ultimately, to recover a long-term secret key.
      
      With this commit, exclusively for BIGNUMs that are flagged with
      BN_FLG_CONSTTIME, instead of accessing only bn->top, all the limbs of
      the BIGNUM are accessed up to bn->dmax and bitwise masking is used to
      avoid branching.
      
      Memory access pattern still leaks bn->dmax, the size of the lazily
      allocated buffer for representing the BIGNUM, which is inevitable with
      the current BIGNUM architecture: reading past bn->dmax would be an
      out-of-bound read.
      As such, it's the caller responsibility to ensure that bn->dmax does not
      leak secret information, by explicitly expanding the internal BIGNUM
      buffer to a public value sufficient to avoid any lazy reallocation
      while manipulating it: this should be already done at the top level
      alongside setting the BN_FLG_CONSTTIME.
      
      Thanks to David Schrammel and Samuel Weiser for reporting this issue
      through responsible disclosure.
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
      (Merged from https://github.com/openssl/openssl/pull/9511)
      
      (cherry picked from commit 8b44198b916015f77bef1befa26edb48ad8a0238)
      b9a380f7
    • Nicola Tuveri's avatar
      Fix a SCA leak using BN_bn2bin() · 083f297a
      Nicola Tuveri authored
      
      
      BN_bn2bin() is not constant-time and leaks the number of bits in the
      processed BIGNUM.
      
      The specialized methods in ecp_nistp224.c, ecp_nistp256.c and
      ecp_nistp521.c internally used BN_bn2bin() to convert scalars into the
      internal fixed length representation.
      
      This can leak during ECDSA/ECDH key generation or handling the nonce
      while generating an ECDSA signature, when using these implementations.
      The amount and risk of leaked information useful for a SCA attack
      varies for each of the three curves, as it depends mainly on the
      ratio between the bitlength of the curve subgroup order (governing the
      size of the secret nonce/key) and the limb size for the internal BIGNUM
      representation (which depends on the compilation target architecture).
      
      To fix this, we replace BN_bn2bin() with BN_bn2binpad(), bounding the
      output length to the width of the internal representation buffer: this
      length is public.
      
      Internally the final implementation of both BN_bn2binpad() and
      BN_bn2bin() already has masking in place to avoid leaking bn->top
      through memory access patterns.
      Memory access pattern still leaks bn->dmax, the size of the lazily
      allocated buffer for representing the BIGNUM, which is inevitable with
      the current BIGNUM architecture: reading past bn->dmax would be an
      out-of-bound read.
      As such, it's the caller responsibility to ensure that bn->dmax does not
      leak secret information, by explicitly expanding the internal BIGNUM
      buffer to a public value sufficient to avoid any lazy reallocation
      while manipulating it: this is already done at the top level alongside
      setting the BN_FLG_CONSTTIME.
      
      Finally, the internal implementation of BN_bn2binpad() indirectly calls
      BN_num_bits() via BN_num_bytes(): the current implementation of
      BN_num_bits() can leak information to a SCA attacker, and is addressed
      in the next commit.
      
      Thanks to David Schrammel and Samuel Weiser for reporting this issue
      through responsible disclosure.
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
      (Merged from https://github.com/openssl/openssl/pull/9511)
      
      (cherry picked from commit 805315d3a20f7274195eed75b06c391dacf3b197)
      083f297a
    • Bernd Edlinger's avatar
      Fix a SCA leak in BN_generate_dsa_nonce · 9e1403d9
      Bernd Edlinger authored
      
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      Reviewed-by: default avatarNicola Tuveri <nic.tuv@gmail.com>
      (Merged from https://github.com/openssl/openssl/pull/9782)
      
      (cherry picked from commit 31ca19403d56ad71d823cf62990518dfc6905bb4)
      9e1403d9
    • Cesar Pereida Garcia's avatar
    • Cesar Pereida Garcia's avatar
      [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation. · 2f18596c
      Cesar Pereida Garcia authored
      
      
      This commit addresses multiple side-channel vulnerabilities present
      during RSA key validation.
      Private key parameters are re-computed using variable-time functions.
      
      This issue was discovered and reported by the NISEC group at TAU Finland.
      
      Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/9779)
      2f18596c
    • Matt Caswell's avatar
      Teach TLSProxy how to parse CertificateRequest messages · 6f34a16e
      Matt Caswell authored
      
      
      We also use this in test_tls13messages to check that the extensions we
      expect to see in a CertificateRequest are there.
      
      Reviewed-by: default avatarTomas Mraz <tmraz@fedoraproject.org>
      (Merged from https://github.com/openssl/openssl/pull/9780)
      
      (cherry picked from commit dc5bcb88d819de55eb37460c122e02fec91c6d86)
      6f34a16e
    • Matt Caswell's avatar
      Don't send a status_request extension in a CertificateRequest message · f8affa29
      Matt Caswell authored
      
      
      If a TLSv1.3 server configured to respond to the status_request extension
      also attempted to send a CertificateRequest then it was incorrectly
      inserting a non zero length status_request extension into that message.
      
      The TLSv1.3 RFC does allow that extension in that message but it must
      always be zero length.
      
      In fact we should not be sending the extension at all in that message
      because we don't support it.
      
      Fixes #9767
      
      Reviewed-by: default avatarTomas Mraz <tmraz@fedoraproject.org>
      (Merged from https://github.com/openssl/openssl/pull/9780)
      
      (cherry picked from commit debb64a0ca43969eb3f043aa8895a4faa7f12b6e)
      f8affa29
  2. 05 Sep, 2019 3 commits
  3. 04 Sep, 2019 4 commits
  4. 30 Aug, 2019 1 commit
  5. 29 Aug, 2019 2 commits
  6. 27 Aug, 2019 1 commit
  7. 24 Aug, 2019 1 commit
  8. 23 Aug, 2019 1 commit
    • Richard Levitte's avatar
      test/evp_test.c: distinguish parsing errors from processing errors · 738da3d0
      Richard Levitte authored
      
      
      Parsing functions are at liberty to return:
      
      1:  when parsing on processing of the parsed value succeeded
      0:  when the parsed keyword is unknown
      -1: when the parsed value processing failed
      
      Some parsing functions didn't do this quite right, they returned 0
      when they should have returned -1, causing a message like this:
      
          Line 123: unknown keyword PeerKey
      
      When this message (which is displayed when the parsing function
      returns -1) would have been more appropriate:
      
          Line 123: error processing keyword PeerKey = ffdhe2048-2-pub
      
      Reviewed-by: default avatarTomas Mraz <tmraz@fedoraproject.org>
      (Merged from https://github.com/openssl/openssl/pull/9682)
      
      (cherry picked from commit f42c225d7f9a0bce0bf46103343402d3f0ad742f)
      738da3d0
  9. 22 Aug, 2019 1 commit
    • Richard Levitte's avatar
      openssl dgst, openssl enc: check for end of input · 8be96f23
      Richard Levitte authored
      
      
      The input reading loop in 'openssl dgst' and 'openssl enc' doesn't
      check for end of input, and because of the way BIO works, it thereby
      won't detect that the end is reached before the read is an error.
      With the FILE BIO, an error occurs when trying to read past EOF, which
      is fairly much ok, except when the command is used interactively, at
      least on Unix.  The result in that case is that the user has to press
      Ctrl-D twice for the command to terminate.
      
      The issue is further complicated because both these commands use
      filter BIOs on top of the FILE BIO, so a naïve attempt to check
      BIO_eof() doesn't quite solve it, since that only checks the state of
      the source/sink BIO, and the filter BIO may have some buffered data
      that still needs to be read.  Fortunately, there's BIO_pending() that
      checks exactly that, if any filter BIO has pending data that needs to
      be processed.
      
      We end up having to check both BIO_pending() and BIO_eof().
      
      Thanks to Zsigmond Lőrinczy for the initial effort and inspiration.
      
      Fixes #9355
      
      Reviewed-by: default avatarTomas Mraz <tmraz@fedoraproject.org>
      (Merged from https://github.com/openssl/openssl/pull/9668)
      
      (cherry picked from commit 8ed7bbb411d2a9e0edef928958ad955e0be3d6dd)
      8be96f23
  10. 20 Aug, 2019 2 commits
    • Johannes's avatar
      Correct documented return value for BIO_get_mem_data() · 378d53ec
      Johannes authored
      
      
      CLA: trivial
      
      Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
      Reviewed-by: default avatarTomas Mraz <tmraz@fedoraproject.org>
      (Merged from https://github.com/openssl/openssl/pull/9643)
      
      (cherry picked from commit 797a5b7af9d9bbfbcbff4607c10ad5c5595ac785)
      378d53ec
    • Pauli's avatar
      Start up DEVRANDOM entropy improvement for older Linux devices. · 3ff98f55
      Pauli authored
      
      
      Improve handling of low entropy at start up from /dev/urandom by waiting for
      a read(2) call on /dev/random to succeed.  Once one such call has succeeded,
      a shared memory segment is created and persisted as an indicator to other
      processes that /dev/urandom is properly seeded.
      
      This does not fully prevent against attacks weakening the entropy source.
      An attacker who has control of the machine early in its boot sequence
      could create the shared memory segment preventing detection of low entropy
      conditions.  However, this is no worse than the current situation.
      
      An attacker would also be capable of removing the shared memory segment
      and causing seeding to reoccur resulting in a denial of service attack.
      This is partially mitigated by keeping the shared memory alive for the
      duration of the process's existence.  Thus, an attacker would not only need
      to have called call shmctl(2) with the IPC_RMID command but the system
      must subsequently enter a state where no instances of libcrypto exist in
      any process.  Even one long running process will prevent this attack.
      
      The System V shared memory calls used here go back at least as far as
      Linux kernel 2.0.  Linux kernels 4.8 and later, don't have a reliable way
      to detect that /dev/urandom has been properly seeded, so a failure is raised
      for this case (i.e. the getentropy(2) call has already failed).
      
      Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
      (Merged from https://github.com/openssl/openssl/pull/9595)
      
      [manual merge]
      3ff98f55
  11. 19 Aug, 2019 4 commits
  12. 18 Aug, 2019 3 commits
  13. 17 Aug, 2019 1 commit
  14. 15 Aug, 2019 3 commits
  15. 14 Aug, 2019 4 commits
  16. 13 Aug, 2019 2 commits