Skip to content

Commit

Permalink
crypto: migrate crypto sign to internal/errors
Browse files Browse the repository at this point in the history 
Improve argument type checking and move into js, use internal/errors

PR-URL: #15757
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
@jasnell
jasnell committed Oct 24, 2017
1 parent 7124b46 commit b8bc652
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 77 deletions.
39 changes: 32 additions & 7 deletions lib/internal/crypto/sig.js
View file
Expand Up @@ -13,12 +13,15 @@ const {
getDefaultEncoding,
toBuf
} = require('internal/crypto/util');
const { isArrayBufferView } = require('internal/util/types');
const { Writable } = require('stream');
const { inherits } = require('util');

function Sign(algorithm, options) {
if (!(this instanceof Sign))
return new Sign(algorithm, options);
if (typeof algorithm !== 'string')
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'algorithm', 'string');
this._handle = new _Sign();
this._handle.init(algorithm);

Expand All @@ -28,13 +31,18 @@ function Sign(algorithm, options) {
inherits(Sign, Writable);

Sign.prototype._write = function _write(chunk, encoding, callback) {
this._handle.update(chunk, encoding);
this.update(chunk, encoding);
callback();
};

Sign.prototype.update = function update(data, encoding) {
encoding = encoding || getDefaultEncoding();
this._handle.update(data, encoding);
data = toBuf(data, encoding);
if (!isArrayBufferView(data)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'data',
['string', 'Buffer', 'TypedArray', 'DataView']);
}
this._handle.update(data);
return this;
};

Expand Down Expand Up @@ -68,8 +76,13 @@ Sign.prototype.sign = function sign(options, encoding) {
}
}

var ret = this._handle.sign(toBuf(key), passphrase, rsaPadding,
pssSaltLength);
key = toBuf(key);
if (!isArrayBufferView(key)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'key',
['string', 'Buffer', 'TypedArray', 'DataView']);
}

var ret = this._handle.sign(key, passphrase, rsaPadding, pssSaltLength);

encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
Expand All @@ -82,7 +95,8 @@ Sign.prototype.sign = function sign(options, encoding) {
function Verify(algorithm, options) {
if (!(this instanceof Verify))
return new Verify(algorithm, options);

if (typeof algorithm !== 'string')
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'algorithm', 'string');
this._handle = new _Verify();
this._handle.init(algorithm);

Expand Down Expand Up @@ -121,8 +135,19 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
}
}

return this._handle.verify(toBuf(key), toBuf(signature, sigEncoding),
rsaPadding, pssSaltLength);
key = toBuf(key);
if (!isArrayBufferView(key)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'key',
['string', 'Buffer', 'TypedArray', 'DataView']);
}

signature = toBuf(signature, sigEncoding);
if (!isArrayBufferView(signature)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'signature',
['string', 'Buffer', 'TypedArray', 'DataView']);
}

return this._handle.verify(key, signature, rsaPadding, pssSaltLength);
};

module.exports = {
Expand Down
54 changes: 6 additions & 48 deletions src/node_crypto.cc
View file
Expand Up @@ -4036,13 +4036,6 @@ SignBase::Error Sign::SignInit(const char* sign_type) {
void Sign::SignInit(const FunctionCallbackInfo<Value>& args) {
Sign* sign;
ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
Environment* env = sign->env();

if (args.Length() == 0) {
return env->ThrowError("Sign type argument is mandatory");
}

THROW_AND_RETURN_IF_NOT_STRING(args[0], "Sign type");

const node::Utf8Value sign_type(args.GetIsolate(), args[0]);
sign->CheckThrow(sign->SignInit(*sign_type));
Expand All @@ -4059,25 +4052,13 @@ SignBase::Error Sign::SignUpdate(const char* data, int len) {


void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Sign* sign;
ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());

THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Data");

// Only copy the data if we have to, because it's a string
Error err;
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
return;
err = sign->SignUpdate(decoder.out(), decoder.size());
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
err = sign->SignUpdate(buf, buflen);
}
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
err = sign->SignUpdate(buf, buflen);

sign->CheckThrow(err);
}
Expand Down Expand Up @@ -4195,7 +4176,6 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {

node::Utf8Value passphrase(env->isolate(), args[1]);

THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Data");
size_t buf_len = Buffer::Length(args[0]);
char* buf = Buffer::Data(args[0]);

Expand Down Expand Up @@ -4269,13 +4249,6 @@ SignBase::Error Verify::VerifyInit(const char* verify_type) {
void Verify::VerifyInit(const FunctionCallbackInfo<Value>& args) {
Verify* verify;
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
Environment* env = verify->env();

if (args.Length() == 0) {
return env->ThrowError("Verify type argument is mandatory");
}

THROW_AND_RETURN_IF_NOT_STRING(args[0], "Verify type");

const node::Utf8Value verify_type(args.GetIsolate(), args[0]);
verify->CheckThrow(verify->VerifyInit(*verify_type));
Expand All @@ -4294,25 +4267,13 @@ SignBase::Error Verify::VerifyUpdate(const char* data, int len) {


void Verify::VerifyUpdate(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Verify* verify;
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());

THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Data");

// Only copy the data if we have to, because it's a string
Error err;
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
return;
err = verify->VerifyUpdate(decoder.out(), decoder.size());
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
err = verify->VerifyUpdate(buf, buflen);
}
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
err = verify->VerifyUpdate(buf, buflen);

verify->CheckThrow(err);
}
Expand Down Expand Up @@ -4421,12 +4382,9 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
Verify* verify;
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());

THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Key");
char* kbuf = Buffer::Data(args[0]);
ssize_t klen = Buffer::Length(args[0]);

THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Hash");

char* hbuf = Buffer::Data(args[1]);
ssize_t hlen = Buffer::Length(args[1]);

Expand Down
103 changes: 103 additions & 0 deletions test/parallel/test-crypto-sign-verify.js
View file
Expand Up @@ -277,3 +277,106 @@ const modSize = 1024;
assert(stdout.includes('Verified OK'));
}));
}

[1, [], {}, undefined, null, true, Infinity].forEach((i) => {
common.expectsError(
() => crypto.createSign(),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "algorithm" argument must be of type string'
}
);
common.expectsError(
() => crypto.createVerify(),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "algorithm" argument must be of type string'
}
);
});

{
const sign = crypto.createSign('SHA1');
const verify = crypto.createVerify('SHA1');

[1, [], {}, undefined, null, true, Infinity].forEach((i) => {
common.expectsError(
() => sign.update(i),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "data" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView'
}
);
common.expectsError(
() => verify.update(i),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "data" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView'
}
);
common.expectsError(
() => sign._write(i, 'utf8', () => {}),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "data" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView'
}
);
common.expectsError(
() => verify._write(i, 'utf8', () => {}),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "data" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView'
}
);
});

[
Uint8Array, Uint16Array, Uint32Array, Float32Array, Float64Array
].forEach((i) => {
// These should all just work
sign.update(new i());
verify.update(new i());
});

[1, {}, [], Infinity].forEach((i) => {
common.expectsError(
() => sign.sign(i),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "key" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView'
}
);

common.expectsError(
() => verify.verify(i),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "key" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView'
}
);

common.expectsError(
() => verify.verify('test', i),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "signature" argument must be one of type string, ' +
'Buffer, TypedArray, or DataView'
}
);
});
}
22 changes: 0 additions & 22 deletions test/parallel/test-crypto.js
View file
Expand Up @@ -201,28 +201,6 @@ assert.throws(function() {
}
});

assert.throws(function() {
crypto.createSign('SHA1').update('0', 'hex');
}, (err) => {
// Throws TypeError, so there is no opensslErrorStack property.
if ((err instanceof Error) &&
/^TypeError: Bad input string$/.test(err) &&
err.opensslErrorStack === undefined) {
return true;
}
});

assert.throws(function() {
crypto.createVerify('SHA1').update('0', 'hex');
}, (err) => {
// Throws TypeError, so there is no opensslErrorStack property.
if ((err instanceof Error) &&
/^TypeError: Bad input string$/.test(err) &&
err.opensslErrorStack === undefined) {
return true;
}
});

assert.throws(function() {
const priv = [
'-----BEGIN RSA PRIVATE KEY-----',
Expand Down

0 comments on commit b8bc652

Please sign in to comment.