Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support both OpenSSL 1.1.0 and 1.0.2 #16130

Closed
wants to merge 26 commits into from

Commits on Nov 2, 2017

  1. crypto: use X509_STORE_CTX_new

    In OpenSSL 1.1.0, X509_STORE_CTX is opaque and thus cannot be
    stack-allocated. This works in OpenSSL 1.1.0 and 1.0.2. Adapted from PR
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    684e25c View commit details
    Browse the repository at this point in the history
  2. crypto: make node_crypto_bio 1.1.0-compatible.

    This is cherry-picked from PR nodejs#8491 and then tidied up. The original had
    an unnecessarily large diff and messed up some public/private bits.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    e9e70ba View commit details
    Browse the repository at this point in the history
  3. crypto: estimate kExternalSize based on a build of OpenSSL 1.1.0f.

    The exact sizes are not particularly important (the original value was
    missing all the objects hanging off anyway), only that V8 garbage
    collector be aware that there is some memory usage beyond the sockets
    themselves.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    2e11a4b View commit details
    Browse the repository at this point in the history
  4. crypto: remove unnecessary SSLerr calls

    These are OpenSSL-internal APIs that are no longer accessible in 1.1.0
    and weren't necessary. OpenSSL will push its own errors and, if it
    doesn't, the calling code would handle it anyway.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    f7cc8d4 View commit details
    Browse the repository at this point in the history
  5. crypto: account for new 1.1.0 SSL APIs

    This is cherry-picked from PR nodejs#8491 and tidied up. This change does
    *not* account for the larger ticket key in OpenSSL 1.1.0. That will be
    done in a follow-up commit as the 48-byte ticket key is part of Node's
    public API.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    382ffc7 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    4b51882 View commit details
    Browse the repository at this point in the history
  7. crypto: use RSA and DH accessors.

    Parts of this were cherry-picked from PR nodejs#8491. Note that this only
    works with OpenSSL 1.0.2 or 1.1.0g or later. 1.1.0g is, as of writing,
    not yet released, but the fix is on the branch. See
    openssl/openssl#4384.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    4db2314 View commit details
    Browse the repository at this point in the history
  8. crypto: no need for locking callbacks in OpenSSL 1.1.0.

    The callbacks are all no-ops in OpenSSL 1.1.0. This isn't necessary (the
    functions still exist for compatibility), but silences some warnings and
    avoids allocating a few unused mutexes.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    17d6752 View commit details
    Browse the repository at this point in the history
  9. crypto: make CipherBase 1.1.0-compatible

    In OpenSSL 1.1.0, EVP_CIPHER_CTX must be heap-allocated. Once we're
    heap-allocating them, there's no need in a separate initialised_ bit.
    The presence of ctx_ is sufficient.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    cbf147f View commit details
    Browse the repository at this point in the history
  10. crypto: make Hash 1.1.0-compatible

    Likewise, 1.1.0 requires EVP_MD_CTX be heap-allocated.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    9947d57 View commit details
    Browse the repository at this point in the history
  11. crypto: make SignBase compatible with OpenSSL 1.1.0

    1.1.0 requires EVP_MD_CTX be heap-allocated. In doing so, move the Init
    and Update hooks to shared code because they are the same between Verify
    and Sign.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    95f56be View commit details
    Browse the repository at this point in the history
  12. crypto: Make Hmac 1.1.0-compatible

    1.1.0 requries HMAC_CTX be heap-allocated.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    e2f6b96 View commit details
    Browse the repository at this point in the history
  13. crypto: add compatibility logic for "DSS1" and "dss1"

    In OpenSSL 1.1.0, EVP_dss1() is removed. These hash names were exposed
    in Node's public API, so add compatibility hooks for them.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    52a4334 View commit details
    Browse the repository at this point in the history
  14. crypto: hard-code tlsSocket.getCipher().version

    This align the documentation with reality. This API never did what Node
    claims it did.
    
    The SSL_CIPHER_get_version function just isn't useful. In OpenSSL 1.0.2,
    it always returned the string "TLSv1/SSLv3" for anything but SSLv2
    ciphers, which Node does not support. Note how test-tls-multi-pfx.js
    claims that ECDHE-ECDSA-AES256-GCM-SHA384 was added in TLSv1/SSLv3 which
    is not true. That cipher is new as of TLS 1.2. The OpenSSL 1.0.2
    implementation is:
    
    char *SSL_CIPHER_get_version(const SSL_CIPHER *c)
    {
        int i;
    
        if (c == NULL)
            return ("(NONE)");
        i = (int)(c->id >> 24L);
        if (i == 3)
            return ("TLSv1/SSLv3");
        else if (i == 2)
            return ("SSLv2");
        else
            return ("unknown");
    }
    
    In OpenSSL 1.1.0, SSL_CIPHER_get_version changed to actually behave as
    Node documented it, but this changes the semantics of the function and
    breaks tests. The cipher's minimum protocol version is not a useful
    notion to return to the caller here, so just hardcode the string at
    "TLSv1/SSLv3" and document it as legacy.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    8b0b970 View commit details
    Browse the repository at this point in the history
  15. test: update test expectations for OpenSSL 1.1.0.

    Some errors in the two versions are different. The test-tls-no-sslv3 one
    because OpenSSL 1.1.x finally does version negotiation properly. 1.0.x's
    logic was somewhat weird and resulted in very inconsistent errors for
    SSLv3 in particular.
    
    Also the function codes are capitalized differently, but function codes
    leak implementation details, so don't assert on them to begin with.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    974dd52 View commit details
    Browse the repository at this point in the history
  16. test: remove sha from test expectations.

    "sha" in OpenSSL refers to SHA-0 which was removed from OpenSSL 1.1.0
    and is insecure. Replace it with SHA-256 which is present in both 1.0.2
    and 1.1.0. Short of shipping a reimplementation in Node, this is an
    unavoidable behavior change with 1.1.0.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    272fcbc View commit details
    Browse the repository at this point in the history
  17. crypto: emulate OpenSSL 1.0.x ticket scheme in 1.1.x

    OpenSSL 1.0.x used a 48-byte ticket key, but OpenSSL 1.1.x made it
    larger by using a larger HMAC-SHA256 key and using AES-256-CBC to
    encrypt. However, Node's public API exposes the 48-byte key. Implement
    the ticket key callback to restore the OpenSSL 1.0.x behavior.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    4fc521a View commit details
    Browse the repository at this point in the history
  18. test: test with a larger RSA key

    OpenSSL 1.1.0 rejects RSA keys smaller than 1024 bits by default. Fix
    the tests to use larger ones. This test only cares that the PEM blob be
    missing a trailing newline. Certificate adapted from
    test/fixtures/cert.pem.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    6b49375 View commit details
    Browse the repository at this point in the history
  19. test: revise test-tls-econnreset

    This test is testing what happens to the server if the client shuts off
    the connection (so the server sees ECONNRESET), but the way it does it
    is convoluted. It uses a static RSA key exchange with a tiny (384-bit)
    RSA key. The server doesn't notice (since it is static RSA, the client
    acts on the key first), so the client tries to encrypt a premaster and
    fails:
    
      rsa routines:RSA_padding_add_PKCS1_type_2:data too large for key size
      SSL routines:ssl3_send_client_key_exchange:bad rsa encrypt
    
    OpenSSL happens not to send an alert in this case, so we get ECONNRESET
    with no alert. This is quite fragile and, notably, breaks in OpenSSL
    1.1.0 now that small RSA keys are rejected by libssl. Instead, test by
    just connecting a TCP socket and immediately closing it.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    b98fb15 View commit details
    Browse the repository at this point in the history
  20. crypto: don't call deprecated ECDH APIs in 1.1.0

    These are both no-ops in OpenSSL 1.1.0.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    f913eae View commit details
    Browse the repository at this point in the history
  21. test: configure certs in tests

    OpenSSL 1.1.0 disables anonymous ciphers unless building with
    enable-weak-crypto. Avoid unnecessary dependencies on these ciphers in
    tests.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    6e1033b View commit details
    Browse the repository at this point in the history
  22. test: fix test-https-agent-session-eviction for 1.1.0

    This test is testing the workaround for an OpenSSL 1.0.x bug, which was
    fixed in 1.1.0. With the bug fixed, the test expectations need to change
    slightly.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    af94ddd View commit details
    Browse the repository at this point in the history
  23. crypto: make ALPN behave the same in 1.0.2 and 1.1.0

    This is kind of hairy. OpenSSL 1.0.2 ignored the return value and always
    treated everything as SSL_TLSEXT_ERR_NOACK (so the comment was wrong and
    Node was never sending a warning alert). OpenSSL 1.1.0 honors
    SSL_TLSEXT_ERR_NOACK vs SSL_TLSEXT_ERR_FATAL_ALERT and treats everything
    unknown as SSL_TLSEXT_ERR_FATAL_ALERT.
    
    Since this is a behavior change (tests break too), start by aligning
    everything on SSL_TLSEXT_ERR_NOACK. If sending no_application_protocol
    is desirable in the future, this can by changed to
    SSL_TLSEXT_ERR_FATAL_ALERT with whatever deprecation process is
    appropriate.
    
    However, note that, contrary to
    https://rt.openssl.org/Ticket/Display.html?id=3463#txn-54498,
    SSL_TLSEXT_ERR_FATAL_ALERT is *not* useful to a server with no fallback
    protocol. Even if such mismatches were rejected, such a server must
    *still* account for the fallback protocol case when the client does not
    advertise ALPN at all. Thus this may not be worth bothering.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    8c432ee View commit details
    Browse the repository at this point in the history
  24. crypto: clear some easy SSL_METHOD deprecation warnings

    Fixing the rest will be rather involved. I think the cleanest option is
    to deprecate the method string APIs which are weird to begin with.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    4bdd2b1 View commit details
    Browse the repository at this point in the history
  25. test: fix flakiness in test-http2-create-client-connect

    The first group of tests makes one more connection and leave the server
    alive for longer. Otherwise the test is just catching that the server
    has closed the socket, depending on timing.
    
    This does not quite make the test pass yet, however. There are some
    quirks with how the http2 code handles errors which actually affect
    1.0.2 as well.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    61f5494 View commit details
    Browse the repository at this point in the history
  26. crypto: deprecate {ecdhCurve: false}.

    This doesn't work in OpenSSL 1.1.0.  Per discussion on the PR, it is
    preferable to just deprecate this setting. Deprecate it and skip the
    test in OpenSSL 1.1.0.
    davidben committed Nov 2, 2017
    Configuration menu
    Copy the full SHA
    b1f5264 View commit details
    Browse the repository at this point in the history