Skip to content

Commit

Permalink
crypto: hard-code tlsSocket.getCipher().version
Browse files Browse the repository at this point in the history
This aligns 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.

PR-URL: #16130
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
  • Loading branch information
davidben authored and rvagg committed Nov 11, 2017
1 parent 594ef76 commit 5fe81c8
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
6 changes: 3 additions & 3 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -558,12 +558,12 @@ Always returns `true`. This may be used to distinguish TLS sockets from regular
added: v0.11.4
-->

Returns an object representing the cipher name and the SSL/TLS protocol version
that first defined the cipher.
Returns an object representing the cipher name. The `version` key is a legacy
field which always contains the value `'TLSv1/SSLv3'`.

For example: `{ name: 'AES256-SHA', version: 'TLSv1/SSLv3' }`

See `SSL_CIPHER_get_name()` and `SSL_CIPHER_get_version()` in
See `SSL_CIPHER_get_name()` in
https://www.openssl.org/docs/man1.0.2/ssl/SSL_CIPHER_get_name.html for more
information.

Expand Down
3 changes: 1 addition & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2258,9 +2258,8 @@ void SSLWrap<Base>::GetCurrentCipher(const FunctionCallbackInfo<Value>& args) {
Local<Object> info = Object::New(env->isolate());
const char* cipher_name = SSL_CIPHER_get_name(c);
info->Set(env->name_string(), OneByteString(args.GetIsolate(), cipher_name));
const char* cipher_version = SSL_CIPHER_get_version(c);
info->Set(env->version_string(),
OneByteString(args.GetIsolate(), cipher_version));
OneByteString(args.GetIsolate(), "TLSv1/SSLv3"));
args.GetReturnValue().Set(info);
}

Expand Down

0 comments on commit 5fe81c8

Please sign in to comment.