Skip to content

Commit

Permalink
crypto: deprecate useless crypto APIs
Browse files Browse the repository at this point in the history
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.

PR-URL: #22126
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
tniessen committed Aug 10, 2018
1 parent 34f56e2 commit 39dd3a4
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 26 deletions.
10 changes: 10 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,16 @@ accessed outside of Node.js core: `Socket.prototype._handle`,
`Socket.prototype._healthCheck()`, `Socket.prototype._stopReceiving()`, and
`dgram._createSocketHandle()`.
<a id="DEP0113"></a>
### DEP0113: 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
Expand Down
32 changes: 27 additions & 5 deletions lib/internal/crypto/cipher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'],
Expand All @@ -203,7 +203,14 @@ Cipher.prototype.setAuthTag = 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,
'Cipher.setAuthTag is deprecated and will be removed in a ' +
'future version of Node.js.',
'DEP0113')
});

Cipher.prototype.setAAD = function setAAD(aadbuf, options) {
if (!isArrayBufferView(aadbuf)) {
Expand Down Expand Up @@ -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.',
'DEP0113')
});
} 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.',
'DEP0113')
});
}
constructor.prototype.setAAD = Cipher.prototype.setAAD;
}

Expand Down
21 changes: 0 additions & 21 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 39dd3a4

Please sign in to comment.