Skip to content

Commit

Permalink
crypto,https,tls: disable engines if perms enabled
Browse files Browse the repository at this point in the history
When the experimental permission model is enabled, the running
JavaScript code is subject to certain restrictions, all of which can
be bypassed or even disabled by native code due to the nature of the
permission model implementation. That is why Node.js native addons
are disabled by default when the permission model is enabled. However,
the built-in crypto, https, and tls modules still allow loading
custom OpenSSL engines. Because OpenSSL engines can execute arbitrary
(native) code while being loaded by Node.js, this has the same security
implications as allowing native addons. In other words, allowing user
code to load OpenSSL engines at runtime effectively enables bypassing
any supposed security restrictions.

This patch adds appropriate checks before attempting to dynamically
load an OpenSSL engine that throw an error if the permission model is
enabled.

PR-URL: nodejs-private/node-private#409
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2023-30586
  • Loading branch information
tniessen authored and RafaelGSS committed Jun 19, 2023
1 parent 9c17e33 commit d274c3b
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 0 deletions.
2 changes: 2 additions & 0 deletions doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,8 @@ Wildcards are supported too:
There are constraints you need to know before using this system:

* Native modules are restricted by default when using the Permission Model.
* OpenSSL engines currently cannot be requested at runtime when the Permission
Model is enabled, affecting the built-in crypto, https, and tls modules.
* Relative paths are not supported through the CLI (`--allow-fs-*`).
* The model does not inherit to a child node process.
* The model does not inherit to a worker thread.
Expand Down
14 changes: 14 additions & 0 deletions src/crypto/crypto_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,13 @@ void SecureContext::SetEngineKey(const FunctionCallbackInfo<Value>& args) {

CHECK_EQ(args.Length(), 2);

if (UNLIKELY(env->permission()->enabled())) {
return THROW_ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED(
env,
"Programmatic selection of OpenSSL engines is unsupported while the "
"experimental permission model is enabled");
}

CryptoErrorStore errors;
Utf8Value engine_id(env->isolate(), args[1]);
EnginePointer engine = LoadEngineById(*engine_id, &errors);
Expand Down Expand Up @@ -1102,6 +1109,13 @@ void SecureContext::SetClientCertEngine(
// support multiple calls to SetClientCertEngine.
CHECK(!sc->client_cert_engine_provided_);

if (UNLIKELY(env->permission()->enabled())) {
return THROW_ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED(
env,
"Programmatic selection of OpenSSL engines is unsupported while the "
"experimental permission model is enabled");
}

CryptoErrorStore errors;
const Utf8Value engine_id(env->isolate(), args[0]);
EnginePointer engine = LoadEngineById(*engine_id, &errors);
Expand Down
7 changes: 7 additions & 0 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,13 @@ void SetEngine(const FunctionCallbackInfo<Value>& args) {

const node::Utf8Value engine_id(env->isolate(), args[0]);

if (UNLIKELY(env->permission()->enabled())) {
return THROW_ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED(
env,
"Programmatic selection of OpenSSL engines is unsupported while the "
"experimental permission model is enabled");
}

args.GetReturnValue().Set(SetEngine(*engine_id, flags));
}
#endif // !OPENSSL_NO_ENGINE
Expand Down
1 change: 1 addition & 0 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ void AppendExceptionLine(Environment* env,
V(ERR_CLOSED_MESSAGE_PORT, Error) \
V(ERR_CONSTRUCT_CALL_REQUIRED, TypeError) \
V(ERR_CONSTRUCT_CALL_INVALID, TypeError) \
V(ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED, Error) \
V(ERR_CRYPTO_INITIALIZATION_FAILED, Error) \
V(ERR_CRYPTO_INVALID_AUTH_TAG, TypeError) \
V(ERR_CRYPTO_INVALID_COUNTER, TypeError) \
Expand Down
2 changes: 2 additions & 0 deletions src/permission/permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class Permission {
return is_scope_granted(permission, res);
}

FORCE_INLINE bool enabled() const { return enabled_; }

static PermissionScope StringToPermission(const std::string& perm);
static const char* PermissionToString(PermissionScope perm);
static void ThrowAccessDenied(Environment* env,
Expand Down

0 comments on commit d274c3b

Please sign in to comment.