1. 26 Feb, 2019 1 commit
  2. 25 Feb, 2019 1 commit
    • Matt Caswell's avatar
      Ensure bn_cmp_words can handle the case where n == 0 · b250f2a4
      Matt Caswell authored
      
      
      Thanks to David Benjamin who reported this, performed the analysis and
      suggested the patch. I have incorporated some of his analysis in the
      comments below.
      
      This issue can cause an out-of-bounds read. It is believed that this was
      not reachable until the recent "fixed top" changes. Analysis has so far
      only identified one code path that can encounter this - although it is
      possible that others may be found. The one code path only impacts 1.0.2 in
      certain builds. The fuzzer found a path in RSA where iqmp is too large. If
      the input is all zeros, the RSA CRT logic will multiply a padded zero by
      iqmp. Two mitigating factors:
      
      - Private keys which trip this are invalid (iqmp is not reduced mod p).
      Only systems which take untrusted private keys care.
      - In OpenSSL 1.1.x, there is a check which rejects the oversize iqmp,
      so the bug is only reproducible in 1.0.2 so far.
      
      Fortunately, the bug appears to be relatively harmless. The consequences of
      bn_cmp_word's misbehavior are:
      
      - OpenSSL may crash if the buffers are page-aligned and the previous page is
      non-existent.
      - OpenSSL will incorrectly treat two BN_ULONG buffers as not equal when they
      are equal.
      - Side channel concerns.
      
      The first is indeed a concern and is a DoS bug. The second is fine in this
      context. bn_cmp_word and bn_cmp_part_words are used to compute abs(a0 - a1)
      in Karatsuba. If a0 = a1, it does not matter whether we use a0 - a1 or
      a1 - a0. The third would be worth thinking about, but it is overshadowed
      by the entire Karatsuba implementation not being constant time.
      
      Due to the difficulty of tripping this and the low impact no CVE is felt
      necessary for this issue.
      
      Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
      Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/8326)
      
      (cherry picked from commit 576129cd)
      b250f2a4
  3. 20 Feb, 2019 2 commits
    • Nicola Tuveri's avatar
      Clear BN_FLG_CONSTTIME on BN_CTX_get() · 9acdddf1
      Nicola Tuveri authored
      (cherry picked from commit c8147d37
      
      )
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/8295)
      9acdddf1
    • Nicola Tuveri's avatar
      Test for constant-time flag leakage in BN_CTX · d769ce09
      Nicola Tuveri authored
      This commit adds a simple unit test to make sure that the constant-time
      flag does not "leak" among BN_CTX frames:
      
      - test_ctx_consttime_flag() initializes (and later frees before
        returning) a BN_CTX object, then it calls in sequence
        test_ctx_set_ct_flag() and test_ctx_check_ct_flag() using the same
        BN_CTX object.
      - test_ctx_set_ct_flag() starts a frame in the given BN_CTX and sets the
        BN_FLG_CONSTTIME flag on some of the BIGNUMs obtained from the frame
        before ending it.
      - test_ctx_check_ct_flag() then starts a new frame and gets a number of
        BIGNUMs from it. In absence of leaks, none of the BIGNUMs in the new
        frame should have BN_FLG_CONSTTIME set.
      
      In actual BN_CTX usage inside libcrypto the leak could happen at any
      depth level in the BN_CTX stack, with varying results depending on the
      patterns of sibling trees of nested function calls sharing the same
      BN_CTX object, and the effect of unintended BN_FLG_CONSTTIME on the
      called BN_* functions.
      
      This simple unit test abstracts away this complexity and verifies that
      the leak does not happen between two sibling functions sharing the same
      BN_CTX object at the same level of nesting.
      
      (manually cherry picked from commit fe16ae5f
      
      )
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/8295)
      d769ce09
  4. 18 Feb, 2019 1 commit
  5. 15 Feb, 2019 1 commit
  6. 15 Jan, 2019 3 commits
  7. 03 Jan, 2019 1 commit
  8. 15 Dec, 2018 1 commit
  9. 12 Dec, 2018 2 commits
  10. 07 Dec, 2018 1 commit
  11. 06 Dec, 2018 5 commits
  12. 03 Dec, 2018 1 commit
  13. 24 Nov, 2018 2 commits
  14. 23 Nov, 2018 1 commit
  15. 22 Nov, 2018 1 commit
  16. 20 Nov, 2018 6 commits
  17. 14 Nov, 2018 1 commit
  18. 12 Nov, 2018 1 commit
  19. 09 Nov, 2018 1 commit
  20. 02 Nov, 2018 1 commit
  21. 01 Nov, 2018 1 commit
  22. 29 Oct, 2018 1 commit
  23. 28 Oct, 2018 1 commit
  24. 18 Oct, 2018 3 commits
    • Dr. Matthias St. Pierre's avatar
      md_rand.c: don't stop polling until properly initialized · 896e8c57
      Dr. Matthias St. Pierre authored
      
      
      Previously, the RNG sets `initialized=1` after the first call to
      RAND_poll(), although its criterion for being initialized actually
      is whether condition `entropy >= ENTROPY_NEEDED` is true.
      
      This commit now assigns `initialized=(entropy >= ENTROPY_NEEDED)`,
      which has the effect that on the next call, RAND_poll() will be
      called again, if it previously failed to obtain enough entropy.
      
      Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
      (Merged from https://github.com/openssl/openssl/pull/7439)
      896e8c57
    • Viktor Dukhovni's avatar
      Apply self-imposed path length also to root CAs · 35cf781c
      Viktor Dukhovni authored
      
      
      Also, some readers of the code find starting the count at 1 for EE
      cert confusing (since RFC5280 counts only non-self-issued intermediate
      CAs, but we also counted the leaf).  Therefore, never count the EE
      cert, and adjust the path length comparison accordinly.  This may
      be more clear to the reader.
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (cherry picked from commit dc5831da)
      35cf781c
    • Viktor Dukhovni's avatar
      Only CA certificates can be self-issued · c8ce9e50
      Viktor Dukhovni authored
      At the bottom of https://tools.ietf.org/html/rfc5280#page-12 and
      top of https://tools.ietf.org/html/rfc5280#page-13 (last paragraph
      of above https://tools.ietf.org/html/rfc5280#section-3.3), we see:
      
         This specification covers two classes of certificates: CA
         certificates and end entity certificates.  CA certificates may be
         further divided into three classes: cross-certificates, self-issued
         certificates, and self-signed certificates.  Cross-certificates are
         CA certificates in which the issuer and subject are different
         entities.  Cross-certificates describe a trust relationship between
         the two CAs.  Self-issued certificates are CA certificates in which
         the issuer and subject are the same entity.  Self-issued certificates
         are generated to support changes in policy or operations.  Self-
         signed certificates are self-issued certificates where the digital
         signature may be verified by the public key bound into the
         certificate.  Self-signed certificates are used to convey a public
         key for use to begin certification paths.  End entity certificates
         are issued to subjects that are not authorized to issue certificates.
      
      that the term "self-issued" is only applicable to CAs, not end-entity
      certificates.  In https://tools.ietf.org/html/rfc5280#section-4.2.1.9
      
      
      the description of path length constraints says:
      
         The pathLenConstraint field is meaningful only if the cA boolean is
         asserted and the key usage extension, if present, asserts the
         keyCertSign bit (Section 4.2.1.3).  In this case, it gives the
         maximum number of non-self-issued intermediate certificates that may
         follow this certificate in a valid certification path.  (Note: The
         last certificate in the certification path is not an intermediate
         certificate, and is not included in this limit.  Usually, the last
         certificate is an end entity certificate, but it can be a CA
         certificate.)
      
      This makes it clear that exclusion of self-issued certificates from
      the path length count applies only to some *intermediate* CA
      certificates.  A leaf certificate whether it has identical issuer
      and subject or whether it is a CA or not is never part of the
      intermediate certificate count.  The handling of all leaf certificates
      must be the same, in the case of our code to post-increment the
      path count by 1, so that we ultimately reach a non-self-issued
      intermediate it will be the first one (not zeroth) in the chain
      of intermediates.
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (cherry picked from commit ed422a2d)
      c8ce9e50