Skip to content
  1. Nov 07, 2016
    • David Benjamin's avatar
      Improve RSA test coverage. · f3205557
      David Benjamin authored
      MD5/SHA1 and MDC-2 have special-case logic beyond the generic DigestInfo
      wrapping. Test that each of these works, including hash and length
      mismatches (both input and signature). Also add VerifyRecover tests. It
      appears 5824cc29
      
       added support for
      VerifyRecover, but forgot to add the test data.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      
      GH: #1474
      f3205557
    • David Benjamin's avatar
      Make RSA_sign.pod less confusing. · aa90ca11
      David Benjamin authored
      
      
      PKCS #1 v2.0 is the name of a document which specifies an algorithm
      RSASSA-PKCS1-v1_5, often referred to as "PKCS #1 v1.5" after an earlier
      document which specified it. This gets further confusing because the
      document PKCS #1 v2.1 specifies two signature algorithms,
      RSASSA-PKCS1-v1_5 and RSASSA-PSS. RSA_sign implements RSASSA-PKCS1-v1_5.
      
      Refer to the document using the RFC number which is easier to find
      anyway, and refer to the algorithm by its name.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      
      GH: #1474
      aa90ca11
    • David Benjamin's avatar
      Implement RSASSA-PKCS1-v1_5 as specified. · 608a0264
      David Benjamin authored
      RFC 3447, section 8.2.2, steps 3 and 4 states that verifiers must encode
      the DigestInfo struct and then compare the result against the public key
      operation result. This implies that one and only one encoding is legal.
      
      OpenSSL instead parses with crypto/asn1, then checks that the encoding
      round-trips, and allows some variations for the parameter. Sufficient
      laxness in this area can allow signature forgeries, as described in
      https://www.imperialviolet.org/2014/09/26/pkcs1.html
      
      Although there aren't known attacks against OpenSSL's current scheme,
      this change makes OpenSSL implement the algorithm as specified. This
      avoids the uncertainty and, more importantly, helps grow a healthy
      ecosystem. Laxness beyond the spec, particularly in implementations
      which enjoy wide use, risks harm to the ecosystem for all. A signature
      producer which only tests against OpenSSL may not notice bugs and
      accidentally become widely deployed. Thus implementations have a
      responsibility to honor the specification as tightly as is practical.
      
      In some cases, the damage is permanent and the spec deviation and
      security risk becomes a tax all implementors must forever pay, but not
      here. Both BoringSSL and Go successfully implemented and deployed
      RSASSA-PKCS1-v1_5 as specified since their respective beginnings, so
      this change should be compatible enough to pin down in future OpenSSL
      releases.
      
      See also https://tools.ietf.org/html/draft-thomson-postel-was-wrong-00
      
      
      
      As a bonus, by not having to deal with sign/verify differences, this
      version is also somewhat clearer. It also more consistently enforces
      digest lengths in the verify_recover codepath. The NID_md5_sha1 codepath
      wasn't quite doing this right.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      
      GH: #1474
      608a0264
    • Matt Caswell's avatar
      Partial revert of "Fix client verify mode to check SSL_VERIFY_PEER" · c8e2f98c
      Matt Caswell authored
      This partially reverts commit c636c1c4
      
      . It also tweaks the documentation
      and comments in this area. On the client side the documented interface for
      SSL_CTX_set_verify()/SSL_set_verify() is that setting the flag
      SSL_VERIFY_PEER causes verfication of the server certificate to take place.
      Previously what was implemented was that if *any* flag was set then
      verification would take place. The above commit improved the semantics to
      be as per the documented interface.
      
      However, we have had a report of at least one application where an
      application was incorrectly using the interface and used *only*
      SSL_VERIFY_FAIL_IF_NO_PEER_CERT on the client side. In OpenSSL prior to
      the above commit this still caused verification of the server certificate
      to take place. After this commit the application silently failed to verify
      the server certificate.
      
      Ideally SSL_CTX_set_verify()/SSL_set_verify() could be modified to indicate
      if invalid flags were being used. However these are void functions!
      
      The simplest short term solution is to revert to the previous behaviour
      which at least means we "fail closed" rather than "fail open".
      
      Thanks to Cory Benfield for reporting this issue.
      
      Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
      c8e2f98c
    • Emilia Kasper's avatar
      Simplify tests part 2 · d836d71b
      Emilia Kasper authored
      
      
      1) Remove some unnecessary fixtures
      2) Add EXECUTE_TEST_NO_TEARDOWN shorthand when a fixture exists but has
      no teardown.
      3) Fix return values in ct_test.c (introduced by an earlier refactoring,
      oops)
      
      Note that for parameterized tests, the index (test vector) usually holds all the
      customization, and there should be no need for a separate test
      fixture. The CTS test is an exception: it demonstrates how to combine
      customization with parameterization.
      
      Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
      d836d71b
    • Matt Caswell's avatar
      Add a test for the wrong version number in a record · 8e47ee18
      Matt Caswell authored
      
      
      Prior to TLS1.3 we check that the received record version number is correct.
      In TLS1.3 we need to ignore the record version number. This adds a test to
      make sure we do it correctly.
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      8e47ee18
    • Matt Caswell's avatar
      Ignore the record version in TLS1.3 · 3c9539d2
      Matt Caswell authored
      
      
      The record layer version field must be ignored in TLSv1.3, so we remove the
      check when using that version.
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      3c9539d2
    • Matt Caswell's avatar
      test_sslcbcpadding only makes sense <TLS1.3 · 185c29b1
      Matt Caswell authored
      
      
      We may get failures if we run it in TLS1.3, and it makes no sense anyway
      so force TLS1.2
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      185c29b1
    • Matt Caswell's avatar
      Correct the Id for the TLS1.3 ciphersuite · 5d71f7ea
      Matt Caswell authored
      
      
      We have one TLS1.3 ciphersuite, but there is a typo in the id that should
      be corrected.
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      5d71f7ea
    • Matt Caswell's avatar
      Always ensure that init_msg is initialised for a CCS · c4377574
      Matt Caswell authored
      
      
      We read it later in grow_init_buf(). If CCS is the first thing received in
      a flight, then it will use the init_msg from the last flight we received. If
      the init_buf has been grown in the meantime then it will point to some
      arbitrary other memory location. This is likely to result in grow_init_buf()
      attempting to grow to some excessively large amount which is likely to
      fail. In practice this should never happen because the only time we receive
      a CCS as the first thing in a flight is in an abbreviated handshake. None
      of the preceding messages from the server flight would be large enough to
      trigger this.
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      c4377574
  2. Nov 06, 2016
  3. Nov 05, 2016
  4. Nov 04, 2016