From 5e141fb75a176c12cf4a807fdc9969cf2ff8ae23 Mon Sep 17 00:00:00 2001 From: Vitaly Dyatlov Date: Sun, 8 Jul 2018 20:12:58 +0000 Subject: [PATCH 1/3] src: revert removal of SecureContext `_external` getter This `_external` getter is essential for some libs to work: uWebSockets as an example. --- src/node_crypto.cc | 21 ++++++++++++++++++++ src/node_crypto.h | 1 + test/parallel/test-accessor-properties.js | 17 +++++++++++++++- test/parallel/test-tls-external-accessor.js | 22 +++++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-external-accessor.js diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 31abeec6f3d7d8..ee33af66e67956 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -365,6 +365,19 @@ void SecureContext::Initialize(Environment* env, Local target) { t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kTicketKeyIVIndex"), Integer::NewFromUnsigned(env->isolate(), kTicketKeyIVIndex)); + Local ctx_getter_templ = + FunctionTemplate::New(env->isolate(), + CtxGetter, + env->as_external(), + Signature::New(env->isolate(), t)); + + + t->PrototypeTemplate()->SetAccessorProperty( + FIXED_ONE_BYTE_STRING(env->isolate(), "_external"), + ctx_getter_templ, + Local(), + static_cast(ReadOnly | DontDelete)); + target->Set(secureContextString, t->GetFunction()); env->set_secure_context_constructor_template(t); } @@ -1346,6 +1359,14 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl, } +void SecureContext::CtxGetter(const FunctionCallbackInfo& info) { + SecureContext* sc; + ASSIGN_OR_RETURN_UNWRAP(&sc, info.This()); + Local ext = External::New(info.GetIsolate(), sc->ctx_); + info.GetReturnValue().Set(ext); +} + + template void SecureContext::GetCertificate(const FunctionCallbackInfo& args) { SecureContext* wrap; diff --git a/src/node_crypto.h b/src/node_crypto.h index 4587a96e725f86..256da2eeb1db08 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -163,6 +163,7 @@ class SecureContext : public BaseObject { const v8::FunctionCallbackInfo& args); static void EnableTicketKeyCallback( const v8::FunctionCallbackInfo& args); + static void CtxGetter(const v8::FunctionCallbackInfo& info); template static void GetCertificate(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js index 064ef844c38e6c..713be7eac203a6 100644 --- a/test/parallel/test-accessor-properties.js +++ b/test/parallel/test-accessor-properties.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); // This tests that the accessor properties do not raise assertions // when called with incompatible receivers. @@ -50,4 +50,19 @@ const UDP = process.binding('udp_wrap').UDP; typeof Object.getOwnPropertyDescriptor(UDP.prototype, 'fd'), 'object' ); + + if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check + // There are accessor properties in crypto too + const crypto = process.binding('crypto'); + + assert.throws(() => { + crypto.SecureContext.prototype._external; + }, TypeError); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor( + crypto.SecureContext.prototype, '_external'), + 'object' + ); + } } diff --git a/test/parallel/test-tls-external-accessor.js b/test/parallel/test-tls-external-accessor.js new file mode 100644 index 00000000000000..33d371923a600c --- /dev/null +++ b/test/parallel/test-tls-external-accessor.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); + +// Ensure accessing ._external doesn't hit an assert in the accessor method. +{ + const pctx = tls.createSecureContext().context; + const cctx = Object.create(pctx); + assert.throws(() => cctx._external, TypeError); + pctx._external; +} +{ + const pctx = tls.createSecurePair().credentials.context; + const cctx = Object.create(pctx); + assert.throws(() => cctx._external, TypeError); + pctx._external; +} From 3eded9fafdd058f4cfdad9f9a147a3b0996b626b Mon Sep 17 00:00:00 2001 From: Vitaly Dyatlov Date: Sun, 8 Jul 2018 20:12:58 +0000 Subject: [PATCH 2/3] src: revert removal of SecureContext `_external` getter This `_external` getter is essential for some libs to work: uWebSockets as an example. --- src/node_crypto.cc | 21 ++++++++++++++++++++ src/node_crypto.h | 1 + test/parallel/test-accessor-properties.js | 17 +++++++++++++++- test/parallel/test-tls-external-accessor.js | 22 +++++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-external-accessor.js diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 31abeec6f3d7d8..f3afb24b4531ac 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -365,6 +365,19 @@ void SecureContext::Initialize(Environment* env, Local target) { t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kTicketKeyIVIndex"), Integer::NewFromUnsigned(env->isolate(), kTicketKeyIVIndex)); + Local ctx_getter_templ = + FunctionTemplate::New(env->isolate(), + CtxGetter, + env->as_external(), + Signature::New(env->isolate(), t)); + + + t->PrototypeTemplate()->SetAccessorProperty( + FIXED_ONE_BYTE_STRING(env->isolate(), "_external"), + ctx_getter_templ, + Local(), + static_cast(ReadOnly | DontDelete)); + target->Set(secureContextString, t->GetFunction()); env->set_secure_context_constructor_template(t); } @@ -1346,6 +1359,14 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl, } +void SecureContext::CtxGetter(const FunctionCallbackInfo& info) { + SecureContext* sc; + ASSIGN_OR_RETURN_UNWRAP(&sc, info.This()); + Local ext = External::New(info.GetIsolate(), sc->ctx_.get()); + info.GetReturnValue().Set(ext); +} + + template void SecureContext::GetCertificate(const FunctionCallbackInfo& args) { SecureContext* wrap; diff --git a/src/node_crypto.h b/src/node_crypto.h index 4587a96e725f86..256da2eeb1db08 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -163,6 +163,7 @@ class SecureContext : public BaseObject { const v8::FunctionCallbackInfo& args); static void EnableTicketKeyCallback( const v8::FunctionCallbackInfo& args); + static void CtxGetter(const v8::FunctionCallbackInfo& info); template static void GetCertificate(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js index 064ef844c38e6c..713be7eac203a6 100644 --- a/test/parallel/test-accessor-properties.js +++ b/test/parallel/test-accessor-properties.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); // This tests that the accessor properties do not raise assertions // when called with incompatible receivers. @@ -50,4 +50,19 @@ const UDP = process.binding('udp_wrap').UDP; typeof Object.getOwnPropertyDescriptor(UDP.prototype, 'fd'), 'object' ); + + if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check + // There are accessor properties in crypto too + const crypto = process.binding('crypto'); + + assert.throws(() => { + crypto.SecureContext.prototype._external; + }, TypeError); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor( + crypto.SecureContext.prototype, '_external'), + 'object' + ); + } } diff --git a/test/parallel/test-tls-external-accessor.js b/test/parallel/test-tls-external-accessor.js new file mode 100644 index 00000000000000..33d371923a600c --- /dev/null +++ b/test/parallel/test-tls-external-accessor.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); + +// Ensure accessing ._external doesn't hit an assert in the accessor method. +{ + const pctx = tls.createSecureContext().context; + const cctx = Object.create(pctx); + assert.throws(() => cctx._external, TypeError); + pctx._external; +} +{ + const pctx = tls.createSecurePair().credentials.context; + const cctx = Object.create(pctx); + assert.throws(() => cctx._external, TypeError); + pctx._external; +} From 9b6808547b5a2ac12befcdc7dbcd8da8a3e44b99 Mon Sep 17 00:00:00 2001 From: SirAnthony Date: Tue, 9 Oct 2018 17:52:05 +0300 Subject: [PATCH 3/3] Update node_crypto.cc to match current master --- src/node_crypto.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index f3afb24b4531ac..e1a35257fcdd14 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -378,7 +378,8 @@ void SecureContext::Initialize(Environment* env, Local target) { Local(), static_cast(ReadOnly | DontDelete)); - target->Set(secureContextString, t->GetFunction()); + target->Set(secureContextString, + t->GetFunction(env->context()).ToLocalChecked()); env->set_secure_context_constructor_template(t); }