From 8a8ac8ce4de0fa7c520a8a8146e23ede1bde504e Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 23 Sep 2017 00:35:33 -0400 Subject: [PATCH] crypto: hard-code tlsSocket.getCipher().version 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: https://github.com/nodejs/node/pull/16130 Reviewed-By: Ben Noordhuis Reviewed-By: Rod Vagg --- doc/api/tls.md | 6 +++--- src/node_crypto.cc | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index 26d7e157aa894d..a19a78dc9a0c2e 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -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. diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 8a86c6613a2d08..f1a24ba5458a1f 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2265,9 +2265,8 @@ void SSLWrap::GetCurrentCipher(const FunctionCallbackInfo& args) { Local 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); }