From 39729d992c7f3832e1a164dfead89bcd88d87ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sat, 4 Aug 2018 18:13:05 +0200 Subject: [PATCH 1/4] crypto: deprecate useless crypto APIs The APIs were probably exposed by accident. getAuthTag and setAuthTag are not a usual getter/setter pair: Getting the authentication tag only makes sense in the context of encryption, setting it only makes sense in the context of decryption. Currently, both functions throw. Neither has been documented publicly. --- doc/api/deprecations.md | 10 ++++++++++ lib/internal/crypto/cipher.js | 30 ++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 6262fe4cf54e40..48e4523b7179d4 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1021,6 +1021,16 @@ accessed outside of Node.js core: `Socket.prototype._handle`, `Socket.prototype._healthCheck()`, `Socket.prototype._stopReceiving()`, and `dgram._createSocketHandle()`. + +### DEP00XX: Cipher.setAuthTag(), Decipher.getAuthTag() + +Type: Runtime + +With the current crypto API, having `Cipher.setAuthTag()` and +`Decipher.getAuthTag()` is not helpful and both functions will throw an error +when called. They have never been documented and will be removed in a future +release. + [`--pending-deprecation`]: cli.html#cli_pending_deprecation [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index 94acc40639105b..2f54a194c309ae 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -31,7 +31,7 @@ const assert = require('assert'); const LazyTransform = require('internal/streams/lazy_transform'); const { inherits } = require('util'); -const { normalizeEncoding } = require('internal/util'); +const { deprecate, normalizeEncoding } = require('internal/util'); // Lazy loaded for startup performance. let StringDecoder; @@ -194,7 +194,7 @@ Cipher.prototype.getAuthTag = function getAuthTag() { }; -Cipher.prototype.setAuthTag = function setAuthTag(tagbuf) { +function setAuthTag(tagbuf) { if (!isArrayBufferView(tagbuf)) { throw new ERR_INVALID_ARG_TYPE('buffer', ['Buffer', 'TypedArray', 'DataView'], @@ -205,6 +205,13 @@ Cipher.prototype.setAuthTag = function setAuthTag(tagbuf) { return this; }; +Object.defineProperty(Cipher.prototype, 'setAuthTag', { + get: deprecate(() => setAuthTag, + 'Cipher.setAuthTag is deprecated and will be removed in a ' + + 'future version of Node.js.', + 'DEP00XX') +}); + Cipher.prototype.setAAD = function setAAD(aadbuf, options) { if (!isArrayBufferView(aadbuf)) { throw new ERR_INVALID_ARG_TYPE('buffer', @@ -231,8 +238,23 @@ function addCipherPrototypeFunctions(constructor) { constructor.prototype.update = Cipher.prototype.update; constructor.prototype.final = Cipher.prototype.final; constructor.prototype.setAutoPadding = Cipher.prototype.setAutoPadding; - constructor.prototype.getAuthTag = Cipher.prototype.getAuthTag; - constructor.prototype.setAuthTag = Cipher.prototype.setAuthTag; + if (constructor === Cipheriv) { + constructor.prototype.getAuthTag = Cipher.prototype.getAuthTag; + Object.defineProperty(constructor.prototype, 'setAuthTag', { + get: deprecate(() => setAuthTag, + 'Cipher.setAuthTag is deprecated and will be removed in ' + + 'a future version of Node.js.', + 'DEP00XX') + }); + } else { + constructor.prototype.setAuthTag = setAuthTag; + Object.defineProperty(constructor.prototype, 'getAuthTag', { + get: deprecate(() => constructor.prototype.getAuthTag, + 'Decipher.getAuthTag is deprecated and will be removed ' + + 'in a future version of Node.js.', + 'DEP00XX') + }); + } constructor.prototype.setAAD = Cipher.prototype.setAAD; } From f010e96cba5c9e61e05138f68f499c888299f733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sat, 4 Aug 2018 18:34:01 +0200 Subject: [PATCH 2/4] fixup! crypto: deprecate useless crypto APIs --- test/parallel/test-crypto-authenticated.js | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index c7e89d6244d2d7..5c0fbb6a95fded 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -207,27 +207,6 @@ for (const test of TEST_CASES) { assert.throws(function() { encrypt.getAuthTag(); }, errMessages.state); } - { - // trying to set tag on encryption object: - const encrypt = crypto.createCipheriv(test.algo, - Buffer.from(test.key, 'hex'), - Buffer.from(test.iv, 'hex'), - options); - assert.throws(() => { encrypt.setAuthTag(Buffer.from(test.tag, 'hex')); }, - errMessages.state); - } - - { - if (!isCCM || !common.hasFipsCrypto) { - // trying to read tag from decryption object: - const decrypt = crypto.createDecipheriv(test.algo, - Buffer.from(test.key, 'hex'), - Buffer.from(test.iv, 'hex'), - options); - assert.throws(function() { decrypt.getAuthTag(); }, errMessages.state); - } - } - { // trying to create cipher with incorrect IV length assert.throws(function() { From a05c14009bf6e7d5a4601da36604138b3c61fc21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sat, 4 Aug 2018 18:39:45 +0200 Subject: [PATCH 3/4] test: fix crypto test case --- test/parallel/test-crypto-cipher-decipher.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-crypto-cipher-decipher.js b/test/parallel/test-crypto-cipher-decipher.js index 81c5f32d492839..3c0658762f478b 100644 --- a/test/parallel/test-crypto-cipher-decipher.js +++ b/test/parallel/test-crypto-cipher-decipher.js @@ -112,15 +112,6 @@ testCipher2(Buffer.from('0123456789abcdef')); 'TypedArray, or DataView. Received type object' }); - common.expectsError( - () => crypto.createCipher('aes-256-cbc', 'secret').setAuthTag(null), - { - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "buffer" argument must be one of type Buffer, ' + - 'TypedArray, or DataView. Received type object' - }); - common.expectsError( () => crypto.createCipher('aes-256-cbc', 'secret').setAAD(null), { @@ -146,6 +137,15 @@ testCipher2(Buffer.from('0123456789abcdef')); 'Received type object' }); + common.expectsError( + () => crypto.createDecipher('aes-256-cbc', 'secret').setAuthTag(null), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "buffer" argument must be one of type Buffer, ' + + 'TypedArray, or DataView. Received type object' + }); + common.expectsError( () => crypto.createDecipher('aes-256-cbc', null), { From 24bace5b1e33c331f61b3781941f818d59040d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 5 Aug 2018 00:31:18 +0200 Subject: [PATCH 4/4] fixup! deprecate useless crypto APIs --- lib/internal/crypto/cipher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index 2f54a194c309ae..b79db6761cd8f7 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -203,7 +203,7 @@ function setAuthTag(tagbuf) { if (!this._handle.setAuthTag(tagbuf)) throw new ERR_CRYPTO_INVALID_STATE('setAuthTag'); return this; -}; +} Object.defineProperty(Cipher.prototype, 'setAuthTag', { get: deprecate(() => setAuthTag,