Skip to content
This repository was archived by the owner on Aug 31, 2018. It is now read-only.

Commit 1b0e1c4

Browse files
jasnelladdaleax
jasnell
authored and
addaleax
committed
crypto: migrate crypto sign to internal/errors
Improve argument type checking and move into js, use internal/errors PR-URL: nodejs/node#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 28ba810 commit 1b0e1c4

File tree

4 files changed

+141
-77
lines changed

4 files changed

+141
-77
lines changed

‎lib/internal/crypto/sig.js‎

Lines changed: 32 additions & 7 deletions
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‎

Lines changed: 6 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4039,13 +4039,6 @@ SignBase::Error Sign::SignInit(const char* sign_type) {
40394039
void Sign::SignInit(const FunctionCallbackInfo<Value>& args) {
40404040
Sign* sign;
40414041
ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
4042-
Environment* env = sign->env();
4043-
4044-
if (args.Length() == 0) {
4045-
return env->ThrowError("Sign type argument is mandatory");
4046-
}
4047-
4048-
THROW_AND_RETURN_IF_NOT_STRING(args[0], "Sign type");
40494042

40504043
const node::Utf8Value sign_type(args.GetIsolate(), args[0]);
40514044
sign->CheckThrow(sign->SignInit(*sign_type));
@@ -4062,25 +4055,13 @@ SignBase::Error Sign::SignUpdate(const char* data, int len) {
40624055

40634056

40644057
void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args) {
4065-
Environment* env = Environment::GetCurrent(args);
4066-
40674058
Sign* sign;
40684059
ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
40694060

4070-
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Data");
4071-
4072-
// Only copy the data if we have to, because it's a string
40734061
Error err;
4074-
if (args[0]->IsString()) {
4075-
StringBytes::InlineDecoder decoder;
4076-
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
4077-
return;
4078-
err = sign->SignUpdate(decoder.out(), decoder.size());
4079-
} else {
4080-
char* buf = Buffer::Data(args[0]);
4081-
size_t buflen = Buffer::Length(args[0]);
4082-
err = sign->SignUpdate(buf, buflen);
4083-
}
4062+
char* buf = Buffer::Data(args[0]);
4063+
size_t buflen = Buffer::Length(args[0]);
4064+
err = sign->SignUpdate(buf, buflen);
40844065

40854066
sign->CheckThrow(err);
40864067
}
@@ -4198,7 +4179,6 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {
41984179

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

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

@@ -4272,13 +4252,6 @@ SignBase::Error Verify::VerifyInit(const char* verify_type) {
42724252
void Verify::VerifyInit(const FunctionCallbackInfo<Value>& args) {
42734253
Verify* verify;
42744254
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
4275-
Environment* env = verify->env();
4276-
4277-
if (args.Length() == 0) {
4278-
return env->ThrowError("Verify type argument is mandatory");
4279-
}
4280-
4281-
THROW_AND_RETURN_IF_NOT_STRING(args[0], "Verify type");
42824255

42834256
const node::Utf8Value verify_type(args.GetIsolate(), args[0]);
42844257
verify->CheckThrow(verify->VerifyInit(*verify_type));
@@ -4297,25 +4270,13 @@ SignBase::Error Verify::VerifyUpdate(const char* data, int len) {
42974270

42984271

42994272
void Verify::VerifyUpdate(const FunctionCallbackInfo<Value>& args) {
4300-
Environment* env = Environment::GetCurrent(args);
4301-
43024273
Verify* verify;
43034274
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
43044275

4305-
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Data");
4306-
4307-
// Only copy the data if we have to, because it's a string
43084276
Error err;
4309-
if (args[0]->IsString()) {
4310-
StringBytes::InlineDecoder decoder;
4311-
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
4312-
return;
4313-
err = verify->VerifyUpdate(decoder.out(), decoder.size());
4314-
} else {
4315-
char* buf = Buffer::Data(args[0]);
4316-
size_t buflen = Buffer::Length(args[0]);
4317-
err = verify->VerifyUpdate(buf, buflen);
4318-
}
4277+
char* buf = Buffer::Data(args[0]);
4278+
size_t buflen = Buffer::Length(args[0]);
4279+
err = verify->VerifyUpdate(buf, buflen);
43194280

43204281
verify->CheckThrow(err);
43214282
}
@@ -4424,12 +4385,9 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
44244385
Verify* verify;
44254386
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
44264387

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

4431-
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Hash");
4432-
44334391
char* hbuf = Buffer::Data(args[1]);
44344392
ssize_t hlen = Buffer::Length(args[1]);
44354393

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

Lines changed: 103 additions & 0 deletions
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‎

Lines changed: 0 additions & 22 deletions
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)