Skip to content

Commit

Permalink
crypto: do not abort when setting throws
Browse files Browse the repository at this point in the history
PR-URL: #27157
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
sam-github committed Apr 17, 2019
1 parent 3ef1512 commit 69140bc
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 17 deletions.
38 changes: 22 additions & 16 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,9 @@ static int NoPasswordCallback(char* buf, int size, int rwflag, void* u) {

// namespace node::crypto::error
namespace error {
void Decorate(Environment* env, Local<Object> obj,
Maybe<bool> Decorate(Environment* env, Local<Object> obj,
unsigned long err) { // NOLINT(runtime/int)
if (err == 0) return; // No decoration possible.
if (err == 0) return Just(true); // No decoration necessary.

const char* ls = ERR_lib_error_string(err);
const char* fs = ERR_func_error_string(err);
Expand All @@ -240,19 +240,19 @@ void Decorate(Environment* env, Local<Object> obj,
if (ls != nullptr) {
if (obj->Set(context, env->library_string(),
OneByteString(isolate, ls)).IsNothing()) {
return;
return Nothing<bool>();
}
}
if (fs != nullptr) {
if (obj->Set(context, env->function_string(),
OneByteString(isolate, fs)).IsNothing()) {
return;
return Nothing<bool>();
}
}
if (rs != nullptr) {
if (obj->Set(context, env->reason_string(),
OneByteString(isolate, rs)).IsNothing()) {
return;
return Nothing<bool>();
}

// SSL has no API to recover the error name from the number, so we
Expand Down Expand Up @@ -325,8 +325,10 @@ void Decorate(Environment* env, Local<Object> obj,
if (obj->Set(env->isolate()->GetCurrentContext(),
env->code_string(),
OneByteString(env->isolate(), code)).IsNothing())
return;
return Nothing<bool>();
}

return Just(true);
}
} // namespace error

Expand All @@ -342,7 +344,7 @@ struct CryptoErrorVector : public std::vector<std::string> {
std::reverse(begin(), end());
}

inline Local<Value> ToException(
inline MaybeLocal<Value> ToException(
Environment* env,
Local<String> exception_string = Local<String>()) const {
if (exception_string.IsEmpty()) {
Expand All @@ -364,10 +366,11 @@ struct CryptoErrorVector : public std::vector<std::string> {
if (!empty()) {
CHECK(exception_v->IsObject());
Local<Object> exception = exception_v.As<Object>();
exception->Set(env->context(),
Maybe<bool> ok = exception->Set(env->context(),
env->openssl_error_stack(),
ToV8Value(env->context(), *this).ToLocalChecked())
.Check();
ToV8Value(env->context(), *this).ToLocalChecked());
if (ok.IsNothing())
return MaybeLocal<Value>();
}

return exception_v;
Expand All @@ -386,16 +389,19 @@ void ThrowCryptoError(Environment* env,
message = message_buffer;
}
HandleScope scope(env->isolate());
auto exception_string =
Local<String> exception_string =
String::NewFromUtf8(env->isolate(), message, NewStringType::kNormal)
.ToLocalChecked();
CryptoErrorVector errors;
errors.Capture();
Local<Value> exception = errors.ToException(env, exception_string);
Local<Value> exception;
if (!errors.ToException(env, exception_string).ToLocal(&exception))
return;
Local<Object> obj;
if (!exception->ToObject(env->context()).ToLocal(&obj))
return;
error::Decorate(env, obj, err);
if (error::Decorate(env, obj, err).IsNothing())
return;
env->isolate()->ThrowException(exception);
}

Expand Down Expand Up @@ -5872,7 +5878,7 @@ struct RandomBytesJob : public CryptoJob {

inline Local<Value> ToResult() const {
if (errors.empty()) return Undefined(env->isolate());
return errors.ToException(env);
return errors.ToException(env).ToLocalChecked();
}
};

Expand Down Expand Up @@ -6009,7 +6015,7 @@ struct ScryptJob : public CryptoJob {

inline Local<Value> ToResult() const {
if (errors.empty()) return Undefined(env->isolate());
return errors.ToException(env);
return errors.ToException(env).ToLocalChecked();
}

inline void Cleanse() {
Expand Down Expand Up @@ -6285,7 +6291,7 @@ class GenerateKeyPairJob : public CryptoJob {
if (errors_.empty())
errors_.Capture();
CHECK(!errors_.empty());
*err = errors_.ToException(env);
*err = errors_.ToException(env).ToLocalChecked();
*pubkey = Undefined(env->isolate());
*privkey = Undefined(env->isolate());
}
Expand Down
48 changes: 47 additions & 1 deletion test/parallel/test-crypto-sign-verify.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,45 @@ const modSize = 1024;
'instance when called without `new`');
}

// Test handling of exceptional conditions
{
const library = {
configurable: true,
set() {
throw new Error('bye, bye, library');
}
};
Object.defineProperty(Object.prototype, 'library', library);

assert.throws(() => {
crypto.createSign('sha1').sign(
`-----BEGIN RSA PRIVATE KEY-----
AAAAAAAAAAAA
-----END RSA PRIVATE KEY-----`);
}, { message: 'bye, bye, library' });

delete Object.prototype.library;

const errorStack = {
configurable: true,
set() {
throw new Error('bye, bye, error stack');
}
};
Object.defineProperty(Object.prototype, 'opensslErrorStack', errorStack);

assert.throws(() => {
crypto.createSign('SHA1')
.update('Test123')
.sign({
key: keyPem,
padding: crypto.constants.RSA_PKCS1_OAEP_PADDING
});
}, { message: 'bye, bye, error stack' });

delete Object.prototype.opensslErrorStack;
}

common.expectsError(
() => crypto.createVerify('SHA256').verify({
key: certPem,
Expand Down Expand Up @@ -300,7 +339,14 @@ common.expectsError(
key: keyPem,
padding: crypto.constants.RSA_PKCS1_OAEP_PADDING
});
}, /^Error:.*illegal or unsupported padding mode$/);
}, {
code: 'ERR_OSSL_RSA_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE',
message: /illegal or unsupported padding mode/,
opensslErrorStack: [
'error:06089093:digital envelope routines:EVP_PKEY_CTX_ctrl:' +
'command not supported',
],
});
}

// Test throws exception when key options is null
Expand Down

0 comments on commit 69140bc

Please sign in to comment.