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
Closed
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
684e25c
crypto: use X509_STORE_CTX_new
davidben Sep 16, 2017
e9e70ba
crypto: make node_crypto_bio 1.1.0-compatible.
davidben Sep 14, 2017
2e11a4b
crypto: estimate kExternalSize based on a build of OpenSSL 1.1.0f.
davidben Sep 16, 2017
f7cc8d4
crypto: remove unnecessary SSLerr calls
davidben Sep 16, 2017
382ffc7
crypto: account for new 1.1.0 SSL APIs
davidben Sep 17, 2017
4b51882
crypto: test DiffieHellman keys work without a public half.
davidben Sep 17, 2017
4db2314
crypto: use RSA and DH accessors.
davidben Sep 17, 2017
17d6752
crypto: no need for locking callbacks in OpenSSL 1.1.0.
davidben Sep 18, 2017
cbf147f
crypto: make CipherBase 1.1.0-compatible
davidben Sep 20, 2017
9947d57
crypto: make Hash 1.1.0-compatible
davidben Sep 22, 2017
95f56be
crypto: make SignBase compatible with OpenSSL 1.1.0
davidben Sep 22, 2017
e2f6b96
crypto: Make Hmac 1.1.0-compatible
davidben Sep 22, 2017
52a4334
crypto: add compatibility logic for "DSS1" and "dss1"
davidben Sep 23, 2017
8b0b970
crypto: hard-code tlsSocket.getCipher().version
davidben Sep 23, 2017
974dd52
test: update test expectations for OpenSSL 1.1.0.
davidben Sep 17, 2017
272fcbc
test: remove sha from test expectations.
davidben Sep 23, 2017
4fc521a
crypto: emulate OpenSSL 1.0.x ticket scheme in 1.1.x
davidben Sep 23, 2017
6b49375
test: test with a larger RSA key
davidben Sep 23, 2017
b98fb15
test: revise test-tls-econnreset
davidben Sep 23, 2017
f913eae
crypto: don't call deprecated ECDH APIs in 1.1.0
davidben Sep 23, 2017
6e1033b
test: configure certs in tests
davidben Sep 23, 2017
af94ddd
test: fix test-https-agent-session-eviction for 1.1.0
davidben Sep 23, 2017
8c432ee
crypto: make ALPN behave the same in 1.0.2 and 1.1.0
davidben Sep 23, 2017
4bdd2b1
crypto: clear some easy SSL_METHOD deprecation warnings
davidben Sep 18, 2017
61f5494
test: fix flakiness in test-http2-create-client-connect
davidben Sep 23, 2017
b1f5264
crypto: deprecate {ecdhCurve: false}.
davidben Oct 21, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
@@ -737,6 +737,16 @@ Type: Runtime
internal mechanics of the `REPLServer` itself, and is therefore not
necessary in user space.
<a id="DEP0083"></a>
### DEP0083: Disabling ECDH by setting ecdhCurve to false
Type: Runtime
The `ecdhCurve` option to `tls.createSecureContext()` and `tls.TLSSocket` could
be set to `false` to disable ECDH entirely on the server only. This mode is
deprecated in preparation for migrating to OpenSSL 1.1.0 and consistency with
the client. Use the `ciphers` parameter instead.
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
6 changes: 3 additions & 3 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
@@ -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.

12 changes: 12 additions & 0 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
@@ -65,6 +65,16 @@ function validateKeyCert(value, type) {
exports.SecureContext = SecureContext;


function ecdhCurveWarning() {
if (ecdhCurveWarning.emitted) return;
process.emitWarning('{ ecdhCurve: false } is deprecated.',
'DeprecationWarning',
'DEP0083');
ecdhCurveWarning.emitted = true;
}
ecdhCurveWarning.emitted = false;


exports.createSecureContext = function createSecureContext(options, context) {
if (!options) options = {};

@@ -140,6 +150,8 @@ exports.createSecureContext = function createSecureContext(options, context) {
c.context.setECDHCurve(tls.DEFAULT_ECDH_CURVE);
else if (options.ecdhCurve)
c.context.setECDHCurve(options.ecdhCurve);
else
ecdhCurveWarning();

if (options.dhparam) {
const warning = c.context.setDHParam(options.dhparam);
4 changes: 4 additions & 0 deletions src/node_constants.cc
Original file line number Diff line number Diff line change
@@ -759,6 +759,10 @@ void DefineSignalConstants(Local<Object> target) {
}

void DefineOpenSSLConstants(Local<Object> target) {
#ifdef OPENSSL_VERSION_NUMBER
NODE_DEFINE_CONSTANT(target, OPENSSL_VERSION_NUMBER);
#endif

#ifdef SSL_OP_ALL
NODE_DEFINE_CONSTANT(target, SSL_OP_ALL);
#endif
609 changes: 409 additions & 200 deletions src/node_crypto.cc

Large diffs are not rendered by default.

91 changes: 53 additions & 38 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
@@ -51,8 +51,6 @@
#include <openssl/rand.h>
#include <openssl/pkcs12.h>

#define EVP_F_EVP_DECRYPTFINAL 101

#if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb)
# define NODE__HAVE_TLSEXT_STATUS_CB
#endif // !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb)
@@ -105,8 +103,20 @@ class SecureContext : public BaseObject {
static const int kTicketKeyNameIndex = 3;
static const int kTicketKeyIVIndex = 4;

#if OPENSSL_VERSION_NUMBER >= 0x10100000L
unsigned char ticket_key_name_[16];
unsigned char ticket_key_aes_[16];
unsigned char ticket_key_hmac_[16];
#endif

protected:
#if OPENSSL_VERSION_NUMBER < 0x10100000L
static const int64_t kExternalSize = sizeof(SSL_CTX);
#else
// OpenSSL 1.1.0 has opaque structures. This is an estimate based on the size
// as of OpenSSL 1.1.0f.
static const int64_t kExternalSize = 872;
#endif

static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Init(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -144,6 +154,15 @@ class SecureContext : public BaseObject {
HMAC_CTX* hctx,
int enc);

#if OPENSSL_VERSION_NUMBER >= 0x10100000L
static int TicketCompatibilityCallback(SSL* ssl,
unsigned char* name,
unsigned char* iv,
EVP_CIPHER_CTX* ectx,
HMAC_CTX* hctx,
int enc);
#endif

SecureContext(Environment* env, v8::Local<v8::Object> wrap)
: BaseObject(env, wrap),
ctx_(nullptr),
@@ -220,19 +239,32 @@ class SSLWrap {
protected:
typedef void (*CertCb)(void* arg);

#if OPENSSL_VERSION_NUMBER < 0x10100000L
// Size allocated by OpenSSL: one for SSL structure, one for SSL3_STATE and
// some for buffers.
// NOTE: Actually it is much more than this
static const int64_t kExternalSize =
sizeof(SSL) + sizeof(SSL3_STATE) + 42 * 1024;
#else
// OpenSSL 1.1.0 has opaque structures. This is an estimate based on the size
// as of OpenSSL 1.1.0f.
static const int64_t kExternalSize = 4448 + 1024 + 42 * 1024;
#endif

static void InitNPN(SecureContext* sc);
static void AddMethods(Environment* env, v8::Local<v8::FunctionTemplate> t);

#if OPENSSL_VERSION_NUMBER < 0x10100000L
static SSL_SESSION* GetSessionCallback(SSL* s,
unsigned char* key,
int len,
int* copy);
#else
static SSL_SESSION* GetSessionCallback(SSL* s,
const unsigned char* key,
int len,
int* copy);
#endif
static int NewSessionCallback(SSL* s, SSL_SESSION* sess);
static void OnClientHello(void* arg,
const ClientHelloParser::ClientHello& hello);
@@ -423,9 +455,7 @@ class Connection : public AsyncWrap, public SSLWrap<Connection> {
class CipherBase : public BaseObject {
public:
~CipherBase() override {
if (!initialised_)
return;
EVP_CIPHER_CTX_cleanup(&ctx_);
EVP_CIPHER_CTX_free(ctx_);
}

static void Initialize(Environment* env, v8::Local<v8::Object> target);
@@ -464,27 +494,22 @@ class CipherBase : public BaseObject {
v8::Local<v8::Object> wrap,
CipherKind kind)
: BaseObject(env, wrap),
initialised_(false),
ctx_(nullptr),
kind_(kind),
auth_tag_len_(0) {
MakeWeak<CipherBase>(this);
}

private:
EVP_CIPHER_CTX ctx_; /* coverity[member_decl] */
bool initialised_;
EVP_CIPHER_CTX* ctx_;
const CipherKind kind_;
unsigned int auth_tag_len_;
char auth_tag_[EVP_GCM_TLS_TAG_LEN];
};

class Hmac : public BaseObject {
public:
~Hmac() override {
if (!initialised_)
return;
HMAC_CTX_cleanup(&ctx_);
}
~Hmac() override;

static void Initialize(Environment* env, v8::Local<v8::Object> target);

@@ -499,22 +524,17 @@ class Hmac : public BaseObject {

Hmac(Environment* env, v8::Local<v8::Object> wrap)
: BaseObject(env, wrap),
initialised_(false) {
ctx_(nullptr) {
MakeWeak<Hmac>(this);
}

private:
HMAC_CTX ctx_; /* coverity[member_decl] */
bool initialised_;
HMAC_CTX* ctx_;
};

class Hash : public BaseObject {
public:
~Hash() override {
if (!initialised_)
return;
EVP_MD_CTX_cleanup(&mdctx_);
}
~Hash() override;

static void Initialize(Environment* env, v8::Local<v8::Object> target);

@@ -528,13 +548,13 @@ class Hash : public BaseObject {

Hash(Environment* env, v8::Local<v8::Object> wrap)
: BaseObject(env, wrap),
initialised_(false) {
mdctx_(nullptr),
finalized_(false) {
MakeWeak<Hash>(this);
}

private:
EVP_MD_CTX mdctx_; /* coverity[member_decl] */
bool initialised_;
EVP_MD_CTX* mdctx_;
bool finalized_;
};

@@ -552,28 +572,24 @@ class SignBase : public BaseObject {

SignBase(Environment* env, v8::Local<v8::Object> wrap)
: BaseObject(env, wrap),
initialised_(false) {
mdctx_(nullptr) {
}

~SignBase() override {
if (!initialised_)
return;
EVP_MD_CTX_cleanup(&mdctx_);
}
~SignBase() override;

Error Init(const char* sign_type);
Error Update(const char* data, int len);

protected:
void CheckThrow(Error error);

EVP_MD_CTX mdctx_; /* coverity[member_decl] */
bool initialised_;
EVP_MD_CTX* mdctx_;
};

class Sign : public SignBase {
public:
static void Initialize(Environment* env, v8::Local<v8::Object> target);

Error SignInit(const char* sign_type);
Error SignUpdate(const char* data, int len);
Error SignFinal(const char* key_pem,
int key_pem_len,
const char* passphrase,
@@ -597,8 +613,6 @@ class Verify : public SignBase {
public:
static void Initialize(Environment* env, v8::Local<v8::Object> target);

Error VerifyInit(const char* verify_type);
Error VerifyUpdate(const char* data, int len);
Error VerifyFinal(const char* key_pem,
int key_pem_len,
const char* sig,
@@ -688,9 +702,10 @@ class DiffieHellman : public BaseObject {

private:
static void GetField(const v8::FunctionCallbackInfo<v8::Value>& args,
BIGNUM* (DH::*field), const char* err_if_null);
const BIGNUM* (*get_field)(const DH*),
const char* err_if_null);
static void SetKey(const v8::FunctionCallbackInfo<v8::Value>& args,
BIGNUM* (DH::*field), const char* what);
void (*set_field)(DH*, BIGNUM*), const char* what);
bool VerifyContext();

bool initialised_;
91 changes: 64 additions & 27 deletions src/node_crypto_bio.cc
Original file line number Diff line number Diff line change
@@ -28,24 +28,20 @@
namespace node {
namespace crypto {

const BIO_METHOD NodeBIO::method = {
BIO_TYPE_MEM,
"node.js SSL buffer",
NodeBIO::Write,
NodeBIO::Read,
NodeBIO::Puts,
NodeBIO::Gets,
NodeBIO::Ctrl,
NodeBIO::New,
NodeBIO::Free,
nullptr
};
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#define BIO_set_data(bio, data) bio->ptr = data
#define BIO_get_data(bio) bio->ptr
#define BIO_set_shutdown(bio, shutdown_) bio->shutdown = shutdown_
#define BIO_get_shutdown(bio) bio->shutdown
#define BIO_set_init(bio, init_) bio->init = init_
#define BIO_get_init(bio) bio->init
#endif


BIO* NodeBIO::New() {
// The const_cast doesn't violate const correctness. OpenSSL's usage of
// BIO_METHOD is effectively const but BIO_new() takes a non-const argument.
return BIO_new(const_cast<BIO_METHOD*>(&method));
return BIO_new(const_cast<BIO_METHOD*>(GetMethod()));
}


@@ -70,12 +66,11 @@ void NodeBIO::AssignEnvironment(Environment* env) {


int NodeBIO::New(BIO* bio) {
bio->ptr = new NodeBIO();
BIO_set_data(bio, new NodeBIO());

// XXX Why am I doing it?!
bio->shutdown = 1;
bio->init = 1;
bio->num = -1;
BIO_set_shutdown(bio, 1);
BIO_set_init(bio, 1);

return 1;
}
@@ -85,10 +80,10 @@ int NodeBIO::Free(BIO* bio) {
if (bio == nullptr)
return 0;

if (bio->shutdown) {
if (bio->init && bio->ptr != nullptr) {
if (BIO_get_shutdown(bio)) {
if (BIO_get_init(bio) && BIO_get_data(bio) != nullptr) {
delete FromBIO(bio);
bio->ptr = nullptr;
BIO_set_data(bio, nullptr);
}
}

@@ -97,13 +92,13 @@ int NodeBIO::Free(BIO* bio) {


int NodeBIO::Read(BIO* bio, char* out, int len) {
int bytes;
BIO_clear_retry_flags(bio);

bytes = FromBIO(bio)->Read(out, len);
NodeBIO* nbio = FromBIO(bio);
int bytes = nbio->Read(out, len);

if (bytes == 0) {
bytes = bio->num;
bytes = nbio->eof_return();
if (bytes != 0) {
BIO_set_retry_read(bio);
}
@@ -161,7 +156,7 @@ int NodeBIO::Puts(BIO* bio, const char* str) {


int NodeBIO::Gets(BIO* bio, char* out, int size) {
NodeBIO* nbio = FromBIO(bio);
NodeBIO* nbio = FromBIO(bio);

if (nbio->Length() == 0)
return 0;
@@ -201,7 +196,7 @@ long NodeBIO::Ctrl(BIO* bio, int cmd, long num, // NOLINT(runtime/int)
ret = nbio->Length() == 0;
break;
case BIO_C_SET_BUF_MEM_EOF_RETURN:
bio->num = num;
nbio->set_eof_return(num);
break;
case BIO_CTRL_INFO:
ret = nbio->Length();
@@ -216,10 +211,10 @@ long NodeBIO::Ctrl(BIO* bio, int cmd, long num, // NOLINT(runtime/int)
ret = 0;
break;
case BIO_CTRL_GET_CLOSE:
ret = bio->shutdown;
ret = BIO_get_shutdown(bio);
break;
case BIO_CTRL_SET_CLOSE:
bio->shutdown = num;
BIO_set_shutdown(bio, num);
break;
case BIO_CTRL_WPENDING:
ret = 0;
@@ -241,6 +236,41 @@ long NodeBIO::Ctrl(BIO* bio, int cmd, long num, // NOLINT(runtime/int)
}


const BIO_METHOD* NodeBIO::GetMethod() {
#if OPENSSL_VERSION_NUMBER < 0x10100000L
static const BIO_METHOD method = {
BIO_TYPE_MEM,
"node.js SSL buffer",
Write,
Read,
Puts,
Gets,
Ctrl,
New,
Free,
nullptr
};

return &method;
#else
static BIO_METHOD* method = nullptr;

if (method == nullptr) {
method = BIO_meth_new(BIO_TYPE_MEM, "node.js SSL buffer");
BIO_meth_set_write(method, Write);
BIO_meth_set_read(method, Read);
BIO_meth_set_puts(method, Puts);
BIO_meth_set_gets(method, Gets);
BIO_meth_set_ctrl(method, Ctrl);
BIO_meth_set_create(method, New);
BIO_meth_set_destroy(method, Free);
}

return method;
#endif
}


void NodeBIO::TryMoveReadHead() {
// `read_pos_` and `write_pos_` means the position of the reader and writer
// inside the buffer, respectively. When they're equal - its safe to reset
@@ -488,5 +518,12 @@ NodeBIO::~NodeBIO() {
write_head_ = nullptr;
}


NodeBIO* NodeBIO::FromBIO(BIO* bio) {
CHECK_NE(BIO_get_data(bio), nullptr);
return static_cast<NodeBIO*>(BIO_get_data(bio));
}


} // namespace crypto
} // namespace node
19 changes: 13 additions & 6 deletions src/node_crypto_bio.h
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@ class NodeBIO {
NodeBIO() : env_(nullptr),
initial_(kInitialBufferLength),
length_(0),
eof_return_(-1),
read_head_(nullptr),
write_head_(nullptr) {
}
@@ -95,14 +96,19 @@ class NodeBIO {
return length_;
}

inline void set_eof_return(int num) {
eof_return_ = num;
}

inline int eof_return() {
return eof_return_;
}

inline void set_initial(size_t initial) {
initial_ = initial;
}

static inline NodeBIO* FromBIO(BIO* bio) {
CHECK_NE(bio->ptr, nullptr);
return static_cast<NodeBIO*>(bio->ptr);
}
static NodeBIO* FromBIO(BIO* bio);

private:
static int New(BIO* bio);
@@ -114,12 +120,12 @@ class NodeBIO {
static long Ctrl(BIO* bio, int cmd, long num, // NOLINT(runtime/int)
void* ptr);

static const BIO_METHOD* GetMethod();

// Enough to handle the most of the client hellos
static const size_t kInitialBufferLength = 1024;
static const size_t kThroughputBufferLength = 16384;

static const BIO_METHOD method;

class Buffer {
public:
Buffer(Environment* env, size_t len) : env_(env),
@@ -151,6 +157,7 @@ class NodeBIO {
Environment* env_;
size_t initial_;
size_t length_;
int eof_return_;
Buffer* read_head_;
Buffer* write_head_;
};
13 changes: 13 additions & 0 deletions test/parallel/test-crypto-dh.js
Original file line number Diff line number Diff line change
@@ -56,6 +56,19 @@ const secret3 = dh3.computeSecret(key2, 'hex', 'base64');

assert.strictEqual(secret1, secret3);

// computeSecret works without a public key set at all.
const dh4 = crypto.createDiffieHellman(p1, 'buffer');
dh4.setPrivateKey(privkey1);

assert.deepStrictEqual(dh1.getPrime(), dh4.getPrime());
assert.deepStrictEqual(dh1.getGenerator(), dh4.getGenerator());
assert.deepStrictEqual(dh1.getPrivateKey(), dh4.getPrivateKey());
assert.strictEqual(dh4.verifyError, 0);

const secret4 = dh4.computeSecret(key2, 'hex', 'base64');

assert.strictEqual(secret1, secret4);

const wrongBlockLength =
/^Error: error:0606506D:digital envelope routines:EVP_DecryptFinal_ex:wrong final block length$/;

8 changes: 4 additions & 4 deletions test/parallel/test-crypto.js
Original file line number Diff line number Diff line change
@@ -132,12 +132,12 @@ const noCapitals = /^[^A-Z]+$/;
assert(tlsCiphers.every((value) => noCapitals.test(value)));
validateList(tlsCiphers);

// Assert that we have sha and sha1 but not SHA and SHA1.
// Assert that we have sha1 and sha256 but not SHA1 and SHA256.
assert.notStrictEqual(0, crypto.getHashes().length);
assert(crypto.getHashes().includes('sha1'));
assert(crypto.getHashes().includes('sha'));
assert(crypto.getHashes().includes('sha256'));
assert(!crypto.getHashes().includes('SHA1'));
assert(!crypto.getHashes().includes('SHA'));
assert(!crypto.getHashes().includes('SHA256'));
assert(crypto.getHashes().includes('RSA-SHA1'));
assert(!crypto.getHashes().includes('rsa-sha1'));
validateList(crypto.getHashes());
@@ -238,7 +238,7 @@ assert.throws(function() {
// Throws crypto error, so there is an opensslErrorStack property.
// The openSSL stack should have content.
if ((err instanceof Error) &&
/asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag/.test(err) &&
/asn1 encoding routines:[^:]*:wrong tag/.test(err) &&
err.opensslErrorStack !== undefined &&
Array.isArray(err.opensslErrorStack) &&
err.opensslErrorStack.length > 0) {
17 changes: 8 additions & 9 deletions test/parallel/test-http2-create-client-connect.js
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@
// Tests http2.connect()

const common = require('../common');
const Countdown = require('../common/countdown');
if (!common.hasCrypto)
common.skip('missing crypto');
const fixtures = require('../common/fixtures');
@@ -25,13 +26,12 @@ const URL = url.URL;
[{ port: port, hostname: '127.0.0.1' }, { protocol: 'http:' }]
];

let count = items.length;
const serverClose = new Countdown(items.length + 1,
() => setImmediate(() => server.close()));

const maybeClose = common.mustCall((client) => {
client.destroy();
if (--count === 0) {
setImmediate(() => server.close());
}
serverClose.dec();
}, items.length);

items.forEach((i) => {
@@ -42,7 +42,7 @@ const URL = url.URL;

// Will fail because protocol does not match the server.
h2.connect({ port: port, protocol: 'https:' })
.on('socketError', common.mustCall());
.on('socketError', common.mustCall(() => serverClose.dec()));
}));
}

@@ -70,13 +70,12 @@ const URL = url.URL;
[{ port: port, hostname: '127.0.0.1', protocol: 'https:' }, opts]
];

let count = items.length;
const serverClose = new Countdown(items.length,
() => setImmediate(() => server.close()));

const maybeClose = common.mustCall((client) => {
client.destroy();
if (--count === 0) {
setImmediate(() => server.close());
}
serverClose.dec();
}, items.length);

items.forEach((i) => {
28 changes: 20 additions & 8 deletions test/parallel/test-https-agent-session-eviction.js
Original file line number Diff line number Diff line change
@@ -8,7 +8,8 @@ if (!common.hasCrypto)

const assert = require('assert');
const https = require('https');
const SSL_OP_NO_TICKET = require('crypto').constants.SSL_OP_NO_TICKET;
const { OPENSSL_VERSION_NUMBER, SSL_OP_NO_TICKET } =
require('crypto').constants;

const options = {
key: readKey('agent1-key.pem'),
@@ -58,14 +59,25 @@ function second(server, session) {
res.resume();
});

// Let it fail
req.on('error', common.mustCall(function(err) {
assert(/wrong version number/.test(err.message));
if (OPENSSL_VERSION_NUMBER >= 0x10100000) {
// Although we have a TLS 1.2 session to offer to the TLS 1.0 server,
// connection to the TLS 1.0 server should work.
req.on('response', common.mustCall(function(res) {
// The test is now complete for OpenSSL 1.1.0.
server.close();
}));
} else {
// OpenSSL 1.0.x mistakenly locked versions based on the session it was
// offering. This causes this sequent request to fail. Let it fail, but
// test that this is mitigated on the next try by invalidating the session.
req.on('error', common.mustCall(function(err) {
assert(/wrong version number/.test(err.message));

req.on('close', function() {
third(server);
});
}));
req.on('close', function() {
third(server);
});
}));
}
req.end();
}

8 changes: 5 additions & 3 deletions test/parallel/test-https-connect-address-family.js
Original file line number Diff line number Diff line change
@@ -7,20 +7,22 @@ if (!common.hasIPv6)
common.skip('no IPv6 support');

const assert = require('assert');
const fixtures = require('../common/fixtures');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, available as common.fixtures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm. That doesn't seem to work. I also don't see common.fixtures in any other test. There's common.fixturesDir, though that just points to the directory. Is that plus fs.readFileSync plus path-munging preferred over fixtures.readKey?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine as is. It matches what people were instructed to use in the latest Code & Learn.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, you're both right. I thought (don't know why) that common.js re-exported everything from fixtures.js.

const https = require('https');
const dns = require('dns');

function runTest() {
const ciphers = 'AECDH-NULL-SHA';
https.createServer({ ciphers }, common.mustCall(function(req, res) {
https.createServer({
cert: fixtures.readKey('agent1-cert.pem'),
key: fixtures.readKey('agent1-key.pem'),
}, common.mustCall(function(req, res) {
this.close();
res.end();
})).listen(0, '::1', common.mustCall(function() {
const options = {
host: 'localhost',
port: this.address().port,
family: 6,
ciphers: ciphers,
rejectUnauthorized: false,
};
// Will fail with ECONNREFUSED if the address family is not honored.
48 changes: 31 additions & 17 deletions test/parallel/test-tls-cert-regression.js
Original file line number Diff line number Diff line change
@@ -27,29 +27,43 @@ if (!common.hasCrypto)

const tls = require('tls');


const cert =
`-----BEGIN CERTIFICATE-----
MIIBfjCCASgCCQDmmNjAojbDQjANBgkqhkiG9w0BAQUFADBFMQswCQYDVQQGEwJB
VTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0
cyBQdHkgTHRkMCAXDTE0MDExNjE3NTMxM1oYDzIyODcxMDMxMTc1MzEzWjBFMQsw
CQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJu
ZXQgV2lkZ2l0cyBQdHkgTHRkMFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAPKwlfMX
6HGZIt1xm7fna72eWcOYfUfSxSugghvqYgJt2Oi3lH+wsU1O9FzRIVmpeIjDXhbp
Mjsa1HtzSiccPXsCAwEAATANBgkqhkiG9w0BAQUFAANBAHOoKy0NkyfiYH7Ne5ka
uvCyndyeB4d24FlfqEUlkfaWCZlNKRaV9YhLDiEg3BcIreFo4brtKQfZzTRs0GVm
KHg=
MIIDNDCCAp2gAwIBAgIJAJvXLQpGPpm7MA0GCSqGSIb3DQEBBQUAMHAxCzAJBgNV
BAYTAkdCMRAwDgYDVQQIEwdHd3luZWRkMREwDwYDVQQHEwhXYXVuZmF3cjEUMBIG
A1UEChMLQWNrbmFjayBMdGQxEjAQBgNVBAsTCVRlc3QgQ2VydDESMBAGA1UEAxMJ
bG9jYWxob3N0MB4XDTA5MTEwMjE5MzMwNVoXDTEwMTEwMjE5MzMwNVowcDELMAkG
A1UEBhMCR0IxEDAOBgNVBAgTB0d3eW5lZGQxETAPBgNVBAcTCFdhdW5mYXdyMRQw
EgYDVQQKEwtBY2tuYWNrIEx0ZDESMBAGA1UECxMJVGVzdCBDZXJ0MRIwEAYDVQQD
Ewlsb2NhbGhvc3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANdym7nGe2yw
6LlJfJrQtC5TmKOGrSXiyolYCbGOy4xZI4KD31d3097jhlQFJyF+10gwkE62DuJe
fLvBZDUsvLe1R8bzlVhZnBVn+3QJyUIWQAL+DsRj8P3KoD7k363QN5dIaA1GOAg2
vZcPy1HCUsvOgvDXGRUCZqNLAyt+h/cpAgMBAAGjgdUwgdIwHQYDVR0OBBYEFK4s
VBV4shKUj3UX/fvSJnFaaPBjMIGiBgNVHSMEgZowgZeAFK4sVBV4shKUj3UX/fvS
JnFaaPBjoXSkcjBwMQswCQYDVQQGEwJHQjEQMA4GA1UECBMHR3d5bmVkZDERMA8G
A1UEBxMIV2F1bmZhd3IxFDASBgNVBAoTC0Fja25hY2sgTHRkMRIwEAYDVQQLEwlU
ZXN0IENlcnQxEjAQBgNVBAMTCWxvY2FsaG9zdIIJAJvXLQpGPpm7MAwGA1UdEwQF
MAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAFxR7BA1mUlsYqPiogtxSIfLzHWh+s0bJ
SBuhNrHes4U8QxS8+x/KWjd/81gzsf9J1C2VzTlFaydAgigz3SkQYgs+TMnFkT2o
9jqoJrcdf4WpZ2DQXUALaZgwNzPumMUSx8Ac5gO+BY/RHyP6fCodYvdNwyKslnI3
US7eCSHZsVo=
-----END CERTIFICATE-----`;

const key =
`-----BEGIN RSA PRIVATE KEY-----
MIIBPQIBAAJBAPKwlfMX6HGZIt1xm7fna72eWcOYfUfSxSugghvqYgJt2Oi3lH+w
sU1O9FzRIVmpeIjDXhbpMjsa1HtzSiccPXsCAwEAAQJBAM4uU9aJE0OfdE1p/X+K
LrCT3XMdFCJ24GgmHyOURtwDy18upQJecDVdcZp16fjtOPmaW95GoYRyifB3R4I5
RxECIQD7jRM9slCSVV8xp9kOJQNpHjhRQYVGBn+pyllS2sb+RQIhAPb7Y+BIccri
NWnuhwCW8hA7Fkj/kaBdAwyW7L3Tvui/AiEAiqLCovMecre4Yi6GcsQ1b/6mvSmm
IOS+AT6zIfXPTB0CIQCJKGR3ymN/Qw5crL1GQ41cHCQtF9ickOq/lBUW+j976wIh
AOaJnkQrmurlRdePX6LvN/LgGAQoxwovfjcOYNnZsIVY
MIICXgIBAAKBgQDXcpu5xntssOi5SXya0LQuU5ijhq0l4sqJWAmxjsuMWSOCg99X
d9Pe44ZUBSchftdIMJBOtg7iXny7wWQ1LLy3tUfG85VYWZwVZ/t0CclCFkAC/g7E
Y/D9yqA+5N+t0DeXSGgNRjgINr2XD8tRwlLLzoLw1xkVAmajSwMrfof3KQIDAQAB
AoGBAIBHR/tT93ce2mJAJAXV0AJpWc+7x2pwX2FpXtQujnlxNZhnRlrBCRCD7h4m
t0bVS/86kyGaesBDvAbavfx/N5keYzzmmSp5Ht8IPqKPydGWdigk4x90yWvktai7
dWuRKF94FXr0GUuBONb/dfHdp4KBtzN7oIF9WydYGGXA9ZmBAkEA8/k01bfwQZIu
AgcdNEM94Zcug1gSspXtUu8exNQX4+PNVbadghZb1+OnUO4d3gvWfqvAnaXD3KV6
N4OtUhQQ0QJBAOIRbKMfaymQ9yE3CQQxYfKmEhHXWARXVwuYqIFqjmhSjSXx0l/P
7mSHz1I9uDvxkJev8sQgu1TKIyTOdqPH1tkCQQDPa6H1yYoj1Un0Q2Qa2Mg1kTjk
Re6vkjPQ/KcmJEOjZjtekgFbZfLzmwLXFXqjG2FjFFaQMSxR3QYJSJQEYjbhAkEA
sy7OZcjcXnjZeEkv61Pc57/7qIp/6Aj2JGnefZ1gvI1Z9Q5kCa88rA/9Iplq8pA4
ZBKAoDW1ZbJGAsFmxc/6mQJAdPilhci0qFN86IGmf+ZBnwsDflIwHKDaVofti4wQ
sPWhSOb9VQjMXekI4Y2l8fqAVTS2Fn6+8jkVKxXBywSVCw==
-----END RSA PRIVATE KEY-----`;

function test(cert, key, cb) {
8 changes: 5 additions & 3 deletions test/parallel/test-tls-connect-address-family.js
Original file line number Diff line number Diff line change
@@ -7,19 +7,21 @@ if (!common.hasIPv6)
common.skip('no IPv6 support');

const assert = require('assert');
const fixtures = require('../common/fixtures');
const tls = require('tls');
const dns = require('dns');

function runTest() {
const ciphers = 'AECDH-NULL-SHA';
tls.createServer({ ciphers }, common.mustCall(function() {
tls.createServer({
cert: fixtures.readKey('agent1-cert.pem'),
key: fixtures.readKey('agent1-key.pem'),
}, common.mustCall(function() {
this.close();
})).listen(0, '::1', common.mustCall(function() {
const options = {
host: 'localhost',
port: this.address().port,
family: 6,
ciphers: ciphers,
rejectUnauthorized: false,
};
// Will fail with ECONNREFUSED if the address family is not honored.
8 changes: 8 additions & 0 deletions test/parallel/test-tls-ecdh-disable.js
Original file line number Diff line number Diff line change
@@ -27,6 +27,11 @@ if (!common.hasCrypto)
if (!common.opensslCli)
common.skip('missing openssl-cli');

const OPENSSL_VERSION_NUMBER =
require('crypto').constants.OPENSSL_VERSION_NUMBER;
if (OPENSSL_VERSION_NUMBER >= 0x10100000)
common.skip('false ecdhCurve not supported in OpenSSL 1.1.0');

const assert = require('assert');
const tls = require('tls');
const exec = require('child_process').exec;
@@ -39,6 +44,9 @@ const options = {
ecdhCurve: false
};

common.expectWarning('DeprecationWarning',
'{ ecdhCurve: false } is deprecated.');

const server = tls.createServer(options, common.mustNotCall());

server.listen(0, '127.0.0.1', common.mustCall(function() {
64 changes: 10 additions & 54 deletions test/parallel/test-tls-econnreset.js
Original file line number Diff line number Diff line change
@@ -25,72 +25,28 @@ if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const fixtures = require('../common/fixtures');
const net = require('net');
const tls = require('tls');

const cacert =
`-----BEGIN CERTIFICATE-----
MIIBxTCCAX8CAnXnMA0GCSqGSIb3DQEBBQUAMH0xCzAJBgNVBAYTAlVTMQswCQYD
VQQIEwJDQTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEZMBcGA1UEChMQU3Ryb25n
TG9vcCwgSW5jLjESMBAGA1UECxMJU3Ryb25nT3BzMRowGAYDVQQDExFjYS5zdHJv
bmdsb29wLmNvbTAeFw0xNDAxMTcyMjE1MDdaFw00MTA2MDMyMjE1MDdaMH0xCzAJ
BgNVBAYTAlVTMQswCQYDVQQIEwJDQTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEZ
MBcGA1UEChMQU3Ryb25nTG9vcCwgSW5jLjESMBAGA1UECxMJU3Ryb25nT3BzMRow
GAYDVQQDExFjYS5zdHJvbmdsb29wLmNvbTBMMA0GCSqGSIb3DQEBAQUAAzsAMDgC
MQDKbQ6rIR5t1q1v4Ha36jrq0IkyUohy9EYNvLnXUly1PGqxby0ILlAVJ8JawpY9
AVkCAwEAATANBgkqhkiG9w0BAQUFAAMxALA1uS4CqQXRSAyYTfio5oyLGz71a+NM
+0AFLBwh5AQjhGd0FcenU4OfHxyDEOJT/Q==
-----END CERTIFICATE-----`;

const cert =
`-----BEGIN CERTIFICATE-----
MIIBfDCCATYCAgQaMA0GCSqGSIb3DQEBBQUAMH0xCzAJBgNVBAYTAlVTMQswCQYD
VQQIEwJDQTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEZMBcGA1UEChMQU3Ryb25n
TG9vcCwgSW5jLjESMBAGA1UECxMJU3Ryb25nT3BzMRowGAYDVQQDExFjYS5zdHJv
bmdsb29wLmNvbTAeFw0xNDAxMTcyMjE1MDdaFw00MTA2MDMyMjE1MDdaMBkxFzAV
BgNVBAMTDnN0cm9uZ2xvb3AuY29tMEwwDQYJKoZIhvcNAQEBBQADOwAwOAIxAMfk
I0LWU15pPUwIQNMnRVhhOibi0TQmAau8FBtgwEfGK01WpfGUaJr1a41K8Uq7xwID
AQABoxkwFzAVBgNVHREEDjAMhwQAAAAAhwR/AAABMA0GCSqGSIb3DQEBBQUAAzEA
cGpYrhkrb7mIh9DNhV0qp7pGjqBzlHqB7KQXw2luLDp//6dyHBMexDCQznkhZKRU
-----END CERTIFICATE-----`;

const key =
`-----BEGIN RSA PRIVATE KEY-----
MIH0AgEAAjEAx+QjQtZTXmk9TAhA0ydFWGE6JuLRNCYBq7wUG2DAR8YrTVal8ZRo
mvVrjUrxSrvHAgMBAAECMBCGccvSwC2r8Z9Zh1JtirQVxaL1WWpAQfmVwLe0bAgg
/JWMU/6hS36TsYyZMxwswQIZAPTAfht/zDLb7Hwgu2twsS1Ra9w/yyvtlwIZANET
26votwJAHK1yUrZGA5nnp5qcmQ/JUQIZAII5YV/UUZvF9D/fUplJ7puENPWNY9bN
pQIZAMMwxuS3XiO7two2sQF6W+JTYyX1DPCwAQIZAOYg1TvEGT38k8e8jygv8E8w
YqrWTeQFNQ==
-----END RSA PRIVATE KEY-----`;

const ca = [ cert, cacert ];

let clientError = null;
let connectError = null;

const server = tls.createServer({ ca: ca, cert: cert, key: key }, () => {
assert.fail('should be unreachable');
}).on('tlsClientError', function(err, conn) {
const server = tls.createServer({
cert: fixtures.readKey('agent1-cert.pem'),
key: fixtures.readKey('agent1-key.pem'),
}, common.mustNotCall()).on('tlsClientError', function(err, conn) {
assert(!clientError && conn);
clientError = err;
server.close();
}).listen(0, function() {
const options = {
ciphers: 'AES128-GCM-SHA256',
port: this.address().port,
ca: ca
};
tls.connect(options).on('error', function(err) {
assert(!connectError);

connectError = err;
net.connect(this.address().port, function() {
// Destroy the socket once it is connected, so the server sees ECONNRESET.
this.destroy();
server.close();
}).write('123');
}).on('error', common.mustNotCall());
});

process.on('exit', function() {
assert(clientError);
assert(connectError);
assert(/socket hang up/.test(clientError.message));
assert(/ECONNRESET/.test(clientError.code));
});
4 changes: 3 additions & 1 deletion test/parallel/test-tls-junk-server.js
Original file line number Diff line number Diff line change
@@ -21,7 +21,9 @@ server.listen(0, function() {
req.end();

req.once('error', common.mustCall(function(err) {
assert(/unknown protocol/.test(err.message));
// OpenSSL 1.0.x and 1.1.x use different error messages for junk inputs.
assert(/unknown protocol/.test(err.message) ||
/wrong version number/.test(err.message));
server.close();
}));
});
4 changes: 3 additions & 1 deletion test/parallel/test-tls-no-sslv3.js
Original file line number Diff line number Diff line change
@@ -46,6 +46,8 @@ process.on('exit', function() {
common.printSkipMessage('`openssl s_client -ssl3` not supported.');
} else {
assert.strictEqual(errors.length, 1);
assert(/:wrong version number/.test(errors[0].message));
// OpenSSL 1.0.x and 1.1.x report invalid client versions differently.
assert(/:wrong version number/.test(errors[0].message) ||
/:version too low/.test(errors[0].message));
}
});
Original file line number Diff line number Diff line change
@@ -20,8 +20,10 @@ const server = tls.createServer({})
}).on('tlsClientError', common.mustCall(function(e) {
assert.ok(e instanceof Error,
'Instance of Error should be passed to error handler');
// OpenSSL 1.0.x and 1.1.x use different error codes for junk inputs.
assert.ok(
/SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/.test(e.message),
/SSL routines:[^:]*:(unknown protocol|wrong version number)/.test(
e.message),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit: can you indent by four spaces here and below? (Line continuation.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslint gets unhappy and seems to really want this indent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 spaces is the standard line continuation for our JS style guide. This seems fine to me but if you really, really wanted to make it a bit less awkward because of the length then maybe just put the RegExp into a variable... ?

'Expecting SSL unknown protocol');

server.close();
Original file line number Diff line number Diff line change
@@ -20,8 +20,10 @@ const server = net.createServer(function(c) {
s.on('error', common.mustCall(function(e) {
assert.ok(e instanceof Error,
'Instance of Error should be passed to error handler');
// OpenSSL 1.0.x and 1.1.x use different error codes for junk inputs.
assert.ok(
/SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/.test(e.message),
/SSL routines:[^:]*:(unknown protocol|wrong version number)/.test(
e.message),
'Expecting SSL unknown protocol');
}));