Skip to content
Permalink
Browse files

tls: return correct version from getCipher()

OpenSSL 1.0.0 returned incorrect version information. OpenSSL 1.1.0
fixed this, but returning the correct information broke our tests, so
was considered semver-major. Because of this, the version was hard-coded
to the OpenSSL 1.0.0 (incorrect) string in 5fe81c8.

This is ancient history, start returning the correct cipher version.

PR-URL: #26625
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information...
sam-github authored and danbev committed Mar 12, 2019
1 parent 1ceb6d8 commit 0f745bf9bde4e19dd352f10299d4365a4edaf386
@@ -717,18 +717,25 @@ socket has been destroyed, `null` will be returned.
### tlsSocket.getCipher()
<!-- YAML
added: v0.11.4
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26625
description: Return the minimum cipher version, instead of a fixed string
(`'TLSv1/SSLv3'`).
-->

* Returns: {Object}
* `name` {string} The name of the cipher suite.
* `version` {string} The minimum TLS protocol version supported by this cipher
suite.

Returns an object representing the cipher name. The `version` key is a legacy
field which always contains the value `'TLSv1/SSLv3'`.
Returns an object containing information on the negotiated cipher suite.

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

See `SSL_CIPHER_get_name()` in
<https://www.openssl.org/docs/man1.1.0/ssl/SSL_CIPHER_get_name.html> for more
information.
See
[OpenSSL](https://www.openssl.org/docs/man1.1.1/man3/SSL_CIPHER_get_name.html)
for more information.

### tlsSocket.getEphemeralKeyInfo()
<!-- YAML
@@ -737,18 +737,18 @@ function makeSocketMethodProxy(name) {
}

[
'getFinished', 'getPeerFinished', 'getSession', 'isSessionReused',
'getEphemeralKeyInfo', 'getProtocol', 'getTLSTicket'
'getCipher',
'getEphemeralKeyInfo',
'getFinished',
'getPeerFinished',
'getProtocol',
'getSession',
'getTLSTicket',
'isSessionReused',
].forEach((method) => {
TLSSocket.prototype[method] = makeSocketMethodProxy(method);
});

TLSSocket.prototype.getCipher = function(err) {
if (this._handle)
return this._handle.getCurrentCipher();
return null;
};

// TODO: support anonymous (nocert) and PSK


@@ -1464,7 +1464,7 @@ void SSLWrap<Base>::AddMethods(Environment* env, Local<FunctionTemplate> t) {
env->SetProtoMethod(t, "loadSession", LoadSession);
env->SetProtoMethodNoSideEffect(t, "isSessionReused", IsSessionReused);
env->SetProtoMethodNoSideEffect(t, "verifyError", VerifyError);
env->SetProtoMethodNoSideEffect(t, "getCurrentCipher", GetCurrentCipher);
env->SetProtoMethodNoSideEffect(t, "getCipher", GetCipher);
env->SetProtoMethod(t, "endParser", EndParser);
env->SetProtoMethod(t, "certCbDone", CertCbDone);
env->SetProtoMethod(t, "renegotiate", Renegotiate);
@@ -2357,7 +2357,7 @@ void SSLWrap<Base>::VerifyError(const FunctionCallbackInfo<Value>& args) {


template <class Base>
void SSLWrap<Base>::GetCurrentCipher(const FunctionCallbackInfo<Value>& args) {
void SSLWrap<Base>::GetCipher(const FunctionCallbackInfo<Value>& args) {
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
Environment* env = w->ssl_env();
@@ -2371,8 +2371,9 @@ void SSLWrap<Base>::GetCurrentCipher(const FunctionCallbackInfo<Value>& args) {
const char* cipher_name = SSL_CIPHER_get_name(c);
info->Set(context, env->name_string(),
OneByteString(args.GetIsolate(), cipher_name)).FromJust();
const char* cipher_version = SSL_CIPHER_get_version(c);
info->Set(context, env->version_string(),
OneByteString(args.GetIsolate(), "TLSv1/SSLv3")).FromJust();
OneByteString(args.GetIsolate(), cipher_version)).FromJust();
args.GetReturnValue().Set(info);
}

@@ -281,7 +281,7 @@ class SSLWrap {
static void LoadSession(const v8::FunctionCallbackInfo<v8::Value>& args);
static void IsSessionReused(const v8::FunctionCallbackInfo<v8::Value>& args);
static void VerifyError(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetCurrentCipher(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetCipher(const v8::FunctionCallbackInfo<v8::Value>& args);
static void EndParser(const v8::FunctionCallbackInfo<v8::Value>& args);
static void CertCbDone(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Renegotiate(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -30,28 +30,42 @@ const tls = require('tls');
// Import fixtures directly from its module
const fixtures = require('../common/fixtures');

const cipher_list = ['AES128-SHA256', 'AES256-SHA256'];
const cipher_version_pattern = /TLS|SSL/;
const options = {
key: fixtures.readKey('agent2-key.pem'),
cert: fixtures.readKey('agent2-cert.pem'),
ciphers: cipher_list.join(':'),
honorCipherOrder: true
};

const server = tls.createServer(options, common.mustCall());
let clients = 0;
const server = tls.createServer(options, common.mustCall(() => {
if (--clients === 0)
server.close();
}, 2));

server.listen(0, '127.0.0.1', common.mustCall(function() {
const client = tls.connect({
clients++;
tls.connect({
host: '127.0.0.1',
port: this.address().port,
ciphers: cipher_list.join(':'),
ciphers: 'AES128-SHA256',
rejectUnauthorized: false
}, common.mustCall(function() {
const cipher = client.getCipher();
assert.strictEqual(cipher.name, cipher_list[0]);
assert(cipher_version_pattern.test(cipher.version));
client.end();
server.close();
const cipher = this.getCipher();
assert.strictEqual(cipher.name, 'AES128-SHA256');
assert.strictEqual(cipher.version, 'TLSv1.2');
this.end();
}));

clients++;
tls.connect({
host: '127.0.0.1',
port: this.address().port,
cipher: 'ECDHE-RSA-AES128-GCM-SHA256',
rejectUnauthorized: false
}, common.mustCall(function() {
const cipher = this.getCipher();
assert.strictEqual(cipher.name, 'ECDHE-RSA-AES128-GCM-SHA256');
assert.strictEqual(cipher.version, 'TLSv1.2');
this.end();
}));
}));
@@ -157,7 +157,7 @@ function test(options) {
}, common.mustCall(function() {
assert.deepStrictEqual(ecdsa.getCipher(), {
name: 'ECDHE-ECDSA-AES256-GCM-SHA384',
version: 'TLSv1/SSLv3'
version: 'TLSv1.2'
});
assert.strictEqual(ecdsa.getPeerCertificate().subject.CN, eccCN);
// XXX(sam) certs don't currently include EC key info, so depend on
@@ -177,7 +177,7 @@ function test(options) {
}, common.mustCall(function() {
assert.deepStrictEqual(rsa.getCipher(), {
name: 'ECDHE-RSA-AES256-GCM-SHA384',
version: 'TLSv1/SSLv3'
version: 'TLSv1.2'
});
assert.strictEqual(rsa.getPeerCertificate().subject.CN, rsaCN);
assert(rsa.getPeerCertificate().exponent, 'cert for an RSA key');
@@ -42,9 +42,9 @@ const server = tls.createServer(options, function(conn) {
process.on('exit', function() {
assert.deepStrictEqual(ciphers, [{
name: 'ECDHE-ECDSA-AES256-GCM-SHA384',
version: 'TLSv1/SSLv3'
version: 'TLSv1.2'
}, {
name: 'ECDHE-RSA-AES256-GCM-SHA384',
version: 'TLSv1/SSLv3'
version: 'TLSv1.2'
}]);
});

0 comments on commit 0f745bf

Please sign in to comment.
You can’t perform that action at this time.