Skip to content

Commit b8bc652

Browse files
jasnelljasnell
committed
crypto: migrate crypto sign to internal/errors
Improve argument type checking and move into js, use internal/errors PR-URL: #15757 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 7124b46 commit b8bc652

File tree

4 files changed

+141
-77
lines changed

4 files changed

+141
-77
lines changed

‎lib/internal/crypto/sig.js

+32-7
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@ const {
1313
getDefaultEncoding,
1414
toBuf
1515
} = require('internal/crypto/util');
16+
const { isArrayBufferView } = require('internal/util/types');
1617
const { Writable } = require('stream');
1718
const { inherits } = require('util');
1819

1920
function Sign(algorithm, options) {
2021
if (!(this instanceof Sign))
2122
return new Sign(algorithm, options);
23+
if (typeof algorithm !== 'string')
24+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'algorithm', 'string');
2225
this._handle = new _Sign();
2326
this._handle.init(algorithm);
2427

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

3033
Sign.prototype._write = function _write(chunk, encoding, callback) {
31-
this._handle.update(chunk, encoding);
34+
this.update(chunk, encoding);
3235
callback();
3336
};
3437

3538
Sign.prototype.update = function update(data, encoding) {
3639
encoding = encoding || getDefaultEncoding();
37-
this._handle.update(data, encoding);
40+
data = toBuf(data, encoding);
41+
if (!isArrayBufferView(data)) {
42+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'data',
43+
['string', 'Buffer', 'TypedArray', 'DataView']);
44+
}
45+
this._handle.update(data);
3846
return this;
3947
};
4048

@@ -68,8 +76,13 @@ Sign.prototype.sign = function sign(options, encoding) {
6876
}
6977
}
7078

71-
var ret = this._handle.sign(toBuf(key), passphrase, rsaPadding,
72-
pssSaltLength);
79+
key = toBuf(key);
80+
if (!isArrayBufferView(key)) {
81+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'key',
82+
['string', 'Buffer', 'TypedArray', 'DataView']);
83+
}
84+
85+
var ret = this._handle.sign(key, passphrase, rsaPadding, pssSaltLength);
7386

7487
encoding = encoding || getDefaultEncoding();
7588
if (encoding && encoding !== 'buffer')
@@ -82,7 +95,8 @@ Sign.prototype.sign = function sign(options, encoding) {
8295
function Verify(algorithm, options) {
8396
if (!(this instanceof Verify))
8497
return new Verify(algorithm, options);
85-
98+
if (typeof algorithm !== 'string')
99+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'algorithm', 'string');
86100
this._handle = new _Verify();
87101
this._handle.init(algorithm);
88102

@@ -121,8 +135,19 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
121135
}
122136
}
123137

124-
return this._handle.verify(toBuf(key), toBuf(signature, sigEncoding),
125-
rsaPadding, pssSaltLength);
138+
key = toBuf(key);
139+
if (!isArrayBufferView(key)) {
140+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'key',
141+
['string', 'Buffer', 'TypedArray', 'DataView']);
142+
}
143+
144+
signature = toBuf(signature, sigEncoding);
145+
if (!isArrayBufferView(signature)) {
146+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'signature',
147+
['string', 'Buffer', 'TypedArray', 'DataView']);
148+
}
149+
150+
return this._handle.verify(key, signature, rsaPadding, pssSaltLength);
126151
};
127152

128153
module.exports = {

‎src/node_crypto.cc

+6-48
Original file line numberDiff line numberDiff line change
@@ -4036,13 +4036,6 @@ SignBase::Error Sign::SignInit(const char* sign_type) {
40364036
void Sign::SignInit(const FunctionCallbackInfo<Value>& args) {
40374037
Sign* sign;
40384038
ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
4039-
Environment* env = sign->env();
4040-
4041-
if (args.Length() == 0) {
4042-
return env->ThrowError("Sign type argument is mandatory");
4043-
}
4044-
4045-
THROW_AND_RETURN_IF_NOT_STRING(args[0], "Sign type");
40464039

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

40604053

40614054
void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args) {
4062-
Environment* env = Environment::GetCurrent(args);
4063-
40644055
Sign* sign;
40654056
ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
40664057

4067-
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Data");
4068-
4069-
// Only copy the data if we have to, because it's a string
40704058
Error err;
4071-
if (args[0]->IsString()) {
4072-
StringBytes::InlineDecoder decoder;
4073-
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
4074-
return;
4075-
err = sign->SignUpdate(decoder.out(), decoder.size());
4076-
} else {
4077-
char* buf = Buffer::Data(args[0]);
4078-
size_t buflen = Buffer::Length(args[0]);
4079-
err = sign->SignUpdate(buf, buflen);
4080-
}
4059+
char* buf = Buffer::Data(args[0]);
4060+
size_t buflen = Buffer::Length(args[0]);
4061+
err = sign->SignUpdate(buf, buflen);
40814062

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

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

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

@@ -4269,13 +4249,6 @@ SignBase::Error Verify::VerifyInit(const char* verify_type) {
42694249
void Verify::VerifyInit(const FunctionCallbackInfo<Value>& args) {
42704250
Verify* verify;
42714251
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
4272-
Environment* env = verify->env();
4273-
4274-
if (args.Length() == 0) {
4275-
return env->ThrowError("Verify type argument is mandatory");
4276-
}
4277-
4278-
THROW_AND_RETURN_IF_NOT_STRING(args[0], "Verify type");
42794252

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

42954268

42964269
void Verify::VerifyUpdate(const FunctionCallbackInfo<Value>& args) {
4297-
Environment* env = Environment::GetCurrent(args);
4298-
42994270
Verify* verify;
43004271
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
43014272

4302-
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Data");
4303-
4304-
// Only copy the data if we have to, because it's a string
43054273
Error err;
4306-
if (args[0]->IsString()) {
4307-
StringBytes::InlineDecoder decoder;
4308-
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
4309-
return;
4310-
err = verify->VerifyUpdate(decoder.out(), decoder.size());
4311-
} else {
4312-
char* buf = Buffer::Data(args[0]);
4313-
size_t buflen = Buffer::Length(args[0]);
4314-
err = verify->VerifyUpdate(buf, buflen);
4315-
}
4274+
char* buf = Buffer::Data(args[0]);
4275+
size_t buflen = Buffer::Length(args[0]);
4276+
err = verify->VerifyUpdate(buf, buflen);
43164277

43174278
verify->CheckThrow(err);
43184279
}
@@ -4421,12 +4382,9 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
44214382
Verify* verify;
44224383
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
44234384

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

4428-
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Hash");
4429-
44304388
char* hbuf = Buffer::Data(args[1]);
44314389
ssize_t hlen = Buffer::Length(args[1]);
44324390

‎test/parallel/test-crypto-sign-verify.js

+103
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,106 @@ const modSize = 1024;
277277
assert(stdout.includes('Verified OK'));
278278
}));
279279
}
280+
281+
[1, [], {}, undefined, null, true, Infinity].forEach((i) => {
282+
common.expectsError(
283+
() => crypto.createSign(),
284+
{
285+
code: 'ERR_INVALID_ARG_TYPE',
286+
type: TypeError,
287+
message: 'The "algorithm" argument must be of type string'
288+
}
289+
);
290+
common.expectsError(
291+
() => crypto.createVerify(),
292+
{
293+
code: 'ERR_INVALID_ARG_TYPE',
294+
type: TypeError,
295+
message: 'The "algorithm" argument must be of type string'
296+
}
297+
);
298+
});
299+
300+
{
301+
const sign = crypto.createSign('SHA1');
302+
const verify = crypto.createVerify('SHA1');
303+
304+
[1, [], {}, undefined, null, true, Infinity].forEach((i) => {
305+
common.expectsError(
306+
() => sign.update(i),
307+
{
308+
code: 'ERR_INVALID_ARG_TYPE',
309+
type: TypeError,
310+
message: 'The "data" argument must be one of type string, Buffer, ' +
311+
'TypedArray, or DataView'
312+
}
313+
);
314+
common.expectsError(
315+
() => verify.update(i),
316+
{
317+
code: 'ERR_INVALID_ARG_TYPE',
318+
type: TypeError,
319+
message: 'The "data" argument must be one of type string, Buffer, ' +
320+
'TypedArray, or DataView'
321+
}
322+
);
323+
common.expectsError(
324+
() => sign._write(i, 'utf8', () => {}),
325+
{
326+
code: 'ERR_INVALID_ARG_TYPE',
327+
type: TypeError,
328+
message: 'The "data" argument must be one of type string, Buffer, ' +
329+
'TypedArray, or DataView'
330+
}
331+
);
332+
common.expectsError(
333+
() => verify._write(i, 'utf8', () => {}),
334+
{
335+
code: 'ERR_INVALID_ARG_TYPE',
336+
type: TypeError,
337+
message: 'The "data" argument must be one of type string, Buffer, ' +
338+
'TypedArray, or DataView'
339+
}
340+
);
341+
});
342+
343+
[
344+
Uint8Array, Uint16Array, Uint32Array, Float32Array, Float64Array
345+
].forEach((i) => {
346+
// These should all just work
347+
sign.update(new i());
348+
verify.update(new i());
349+
});
350+
351+
[1, {}, [], Infinity].forEach((i) => {
352+
common.expectsError(
353+
() => sign.sign(i),
354+
{
355+
code: 'ERR_INVALID_ARG_TYPE',
356+
type: TypeError,
357+
message: 'The "key" argument must be one of type string, Buffer, ' +
358+
'TypedArray, or DataView'
359+
}
360+
);
361+
362+
common.expectsError(
363+
() => verify.verify(i),
364+
{
365+
code: 'ERR_INVALID_ARG_TYPE',
366+
type: TypeError,
367+
message: 'The "key" argument must be one of type string, Buffer, ' +
368+
'TypedArray, or DataView'
369+
}
370+
);
371+
372+
common.expectsError(
373+
() => verify.verify('test', i),
374+
{
375+
code: 'ERR_INVALID_ARG_TYPE',
376+
type: TypeError,
377+
message: 'The "signature" argument must be one of type string, ' +
378+
'Buffer, TypedArray, or DataView'
379+
}
380+
);
381+
});
382+
}

‎test/parallel/test-crypto.js

-22
Original file line numberDiff line numberDiff line change
@@ -201,28 +201,6 @@ assert.throws(function() {
201201
}
202202
});
203203

204-
assert.throws(function() {
205-
crypto.createSign('SHA1').update('0', 'hex');
206-
}, (err) => {
207-
// Throws TypeError, so there is no opensslErrorStack property.
208-
if ((err instanceof Error) &&
209-
/^TypeError: Bad input string$/.test(err) &&
210-
err.opensslErrorStack === undefined) {
211-
return true;
212-
}
213-
});
214-
215-
assert.throws(function() {
216-
crypto.createVerify('SHA1').update('0', 'hex');
217-
}, (err) => {
218-
// Throws TypeError, so there is no opensslErrorStack property.
219-
if ((err instanceof Error) &&
220-
/^TypeError: Bad input string$/.test(err) &&
221-
err.opensslErrorStack === undefined) {
222-
return true;
223-
}
224-
});
225-
226204
assert.throws(function() {
227205
const priv = [
228206
'-----BEGIN RSA PRIVATE KEY-----',

0 commit comments

Comments
 (0)