Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: Deprecate createCipher for createCipheriv #13941

Closed
wants to merge 13 commits into from
7 changes: 5 additions & 2 deletions benchmark/crypto/cipher-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ function main(conf) {
// alice_secret and bob_secret should be the same
assert(alice_secret === bob_secret);

var alice_cipher = crypto.createCipher(conf.cipher, alice_secret);
var bob_cipher = crypto.createDecipher(conf.cipher, bob_secret);
const key = crypto.generateLegacyKey(conf.cipher, alice_secret);
const iv = crypto.generateLegacyIV(conf.cipher, alice_secret);

var alice_cipher = crypto.createCipheriv(conf.cipher, key, iv);
var bob_cipher = crypto.createDecipheriv(conf.cipher, key, iv);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use const instead of var.


var message;
var encoding;
Expand Down
42 changes: 42 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,11 @@ currently in use. Setting to true requires a FIPS build of Node.js.
### crypto.createCipher(algorithm, password)
<!-- YAML
added: v0.1.94
deprecated: vx.x.x
-->

> Stability: 0 - Deprecated: Use [`crypto.createCipheriv()`][] instead.

- `algorithm` {string}
- `password` {string | Buffer | TypedArray | DataView}

Expand Down Expand Up @@ -1214,6 +1218,44 @@ The `key` is the raw key used by the `algorithm` and `iv` is an
[initialization vector][]. Both arguments must be `'utf8'` encoded strings,
[Buffers][`Buffer`], `TypedArray`, or `DataView`s.

### crypto.generateLegacyKey(algorithm, key)
- `algorithm` {string}
- `key` {string | Buffer | TypedArray | DataView}

Creates and returns a [Buffer][`Buffer`] object, with the given `algorithm` and
`key`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your docs seem to miss information about what the functions actually do.


Use this function for applications previously reliant on
[`crypto.createCipher()`][]. Pass its return value to
[`crypto.createCipheriv()`][] as the `key`.

The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On
recent OpenSSL releases, `openssl list-cipher-algorithms` will display the
available cipher algorithms.

The `key` must be a `'utf8'` encoded string, [Buffer][`Buffer`], `TypedArray`,
or `DataView`.

### crypto.generateLegacyIV(algorithm, iv)
- `algorithm` {string}
- `iv` {string | Buffer | TypedArray | DataView}

Creates and returns a [Buffer][`Buffer`] object, with the given `algorithm` and
`iv`.

Use this function for applications previously reliant on
[`crypto.createCipher()`][]. Pass its return value to
[`crypto.createCipheriv()`][] as the `iv`.

The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On
recent OpenSSL releases, `openssl list-cipher-algorithms` will display the
available cipher algorithms.

This function will throw an error if a cipher without an IV is passed.

The `iv` must be a `'utf8'` encoded string, [Buffer][`Buffer`], `TypedArray`,
or `DataView`.

### crypto.createCredentials(details)
<!-- YAML
added: v0.1.92
Expand Down
34 changes: 34 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,36 @@ Type: Runtime

*Note*: change was made while `async_hooks` was an experimental API.

<a id="DEP00XX"></a>
### DEP00XX: crypto.createCipher()

Type: Runtime

[`crypto.createCipher()`][] generates keys from strings in an insecure manner,
and, when used with a cipher that utilizes an initialization vector, will
dangerously re-use initialization vectors. As such, it is immediately marked as
deprecated, and will be fully removed in a later version.

[`crypto.createCipheriv()`][] should be used in place of
[`crypto.createCipher()`][]. Since [`crypto.createCipheriv()`][] will no longer
attempt to derive a proper encryption key from a string, you must use a
key-derivation function such as [`crypto.pbkdf2()`][] to obtain a valid key if
you normally supply a string to [`crypto.createCipher()`][].

If the previous key-derivation is required for backward compatiability, the new
APIs [`crypto.generateLegacyKey()`][] and [`crypto.generateLegacyIV()`][] have
been added.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really a causality.


Additionally, for ciphers that require an initialization vector, a proper-length
initialization vector must be passed to [`crypto.createCipheriv()`][].
Initialization vectors must never be re-used, especially in modes such as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mustshould. The word must indicates that calling the function with such parameters leads to undefined behavior or an error, while this is actually just a strong recommendation.

AES-CTR, where encryption is effectively removed upon reuse. Applications will
need to store this initialization vector along with the encrypted data, as it is
required for decryption.

If an initialization vector is not needed by the cipher, pass `null` or omit the
argument.

[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
[`Buffer.from(buffer)`]: buffer.html#buffer_class_method_buffer_from_buffer
Expand All @@ -647,6 +677,10 @@ Type: Runtime
[`child_process`]: child_process.html
[`console.error()`]: console.html#console_console_error_data_args
[`console.log()`]: console.html#console_console_log_data_args
[`crypto.createCipher()`]: crypto.html#crypto_crypto_createcipher_algorithm_password
[`crypto.createCipheriv()`]: crypto.html#crypto_crypto_createcipheriv_algorithm_key_iv
[`crypto.generateLegacyKey()`]: crypto.html#crypto_crypto_generatelegacykey_algorithm_key
[`crypto.generateLegacyIV()`]: crypto.html#crypto_crypto_generatelegacyiv_algorithm_iv
[`crypto.createCredentials()`]: crypto.html#crypto_crypto_createcredentials_details
[`crypto.pbkdf2()`]: crypto.html#crypto_crypto_pbkdf2_password_salt_iterations_keylen_digest_callback
[`domain`]: domain.html
Expand Down
35 changes: 32 additions & 3 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,40 @@ function getDecoder(decoder, encoding) {
return decoder;
}

exports.generateLegacyKey = generateLegacyKey;

function generateLegacyKey(cipher, key) {
return binding.generateLegacyKey(cipher, key);
}

exports.generateLegacyIV = generateLegacyIV;

function generateLegacyIV(cipher, key) {
return binding.generateLegacyIV(cipher, key);
}

Object.defineProperty(exports, 'createCipher', {
configurable: true,
enumerable: true,
get: internalUtil.deprecate(function() {
return Cipher;
}, 'crypto.createCipher is deprecated. ' +
'Use crypto.createCipheriv instead.', 'DEP00XX')
});

Object.defineProperty(exports, 'Cipher', {
configurable: true,
enumerable: true,
get: internalUtil.deprecate(function() {
return Cipher;
}, 'crypto.Cipher is deprecated. ' +
'Use crypto.createCipheriv instead.', 'DEP00XX')
});

exports.createCipher = exports.Cipher = Cipher;
function Cipher(cipher, password, options) {
if (!(this instanceof Cipher))
return new Cipher(cipher, password, options);

this._handle = new binding.CipherBase(true);

this._handle.init(cipher, toBuf(password));
Expand Down Expand Up @@ -214,7 +243,7 @@ function Cipheriv(cipher, key, iv, options) {
if (!(this instanceof Cipheriv))
return new Cipheriv(cipher, key, iv, options);
this._handle = new binding.CipherBase(true);
this._handle.initiv(cipher, toBuf(key), toBuf(iv));
this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer.alloc(0));
this._decoder = null;

LazyTransform.call(this, options);
Expand Down Expand Up @@ -262,7 +291,7 @@ function Decipheriv(cipher, key, iv, options) {
return new Decipheriv(cipher, key, iv, options);

this._handle = new binding.CipherBase(false);
this._handle.initiv(cipher, toBuf(key), toBuf(iv));
this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer.alloc(0));
this._decoder = null;

LazyTransform.call(this, options);
Expand Down
101 changes: 98 additions & 3 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3293,6 +3293,90 @@ void Connection::SetSNICallback(const FunctionCallbackInfo<Value>& args) {
}
#endif

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

if (args.Length() != 2) {
return env->ThrowError("Cipher and key must be supplied.");
}

THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher");
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Key");

const node::Utf8Value cipher_type(env->isolate(), args[0]);
const node::Utf8Value key_value(env->isolate(), args[1]);

const EVP_CIPHER * cipher = EVP_get_cipherbyname(*cipher_type);
if (cipher == nullptr) {
return env->ThrowError("Unknown cipher");
}

const char* key_buf = *key_value;
ssize_t key_buf_len = key_value.length();

unsigned char * key = (unsigned char *) node::Malloc(EVP_MAX_KEY_LENGTH);

int key_len = EVP_BytesToKey(cipher,
EVP_md5(),
nullptr,
reinterpret_cast<const unsigned char*>(key_buf),
key_buf_len,
1,
key,
nullptr);

Local<Object> buf = Buffer::New(env,
reinterpret_cast<char *>(key),
(unsigned) key_len)
.ToLocalChecked();

args.GetReturnValue().Set(buf);
}

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

if (args.Length() != 2) {
return env->ThrowError("Cipher and key must be supplied.");
}

THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher");
THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Key");

const node::Utf8Value cipher_type(env->isolate(), args[0]);
const node::Utf8Value key_value(env->isolate(), args[1]);

const EVP_CIPHER * cipher = EVP_get_cipherbyname(*cipher_type);
if (cipher == nullptr) {
return env->ThrowError("Unknown cipher");
}

const int expected_iv_len = EVP_CIPHER_iv_length(cipher);
if (expected_iv_len == 0) {
return env->ThrowError("Cipher has no IV");
}

const char* key_buf = *key_value;
ssize_t key_buf_len = key_value.length();

unsigned char * iv = (unsigned char *) node::Malloc(EVP_MAX_IV_LENGTH);

EVP_BytesToKey(cipher,
EVP_md5(),
nullptr,
reinterpret_cast<const unsigned char*>(key_buf),
key_buf_len,
1,
nullptr,
iv);

Local<Object> buf = Buffer::New(env,
reinterpret_cast<char *>(iv),
(unsigned) expected_iv_len)
.ToLocalChecked();

args.GetReturnValue().Set(buf);
}

void CipherBase::Initialize(Environment* env, Local<Object> target) {
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
Expand Down Expand Up @@ -3353,7 +3437,11 @@ void CipherBase::Init(const char* cipher_type,

EVP_CIPHER_CTX_init(&ctx_);
const bool encrypt = (kind_ == kCipher);

EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt);

const int iv_len = EVP_CIPHER_iv_length(cipher_);

if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) {
EVP_CIPHER_CTX_cleanup(&ctx_);
return env()->ThrowError("Invalid key length");
Expand Down Expand Up @@ -3448,9 +3536,14 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& args) {
const node::Utf8Value cipher_type(env->isolate(), args[0]);
ssize_t key_len = Buffer::Length(args[1]);
const char* key_buf = Buffer::Data(args[1]);
ssize_t iv_len = Buffer::Length(args[2]);
const char* iv_buf = Buffer::Data(args[2]);
cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len);

if (args.Length() < 3 || args[2]->IsNull()) {
cipher->InitIv(*cipher_type, key_buf, key_len, nullptr, 0);
} else {
ssize_t iv_len = Buffer::Length(args[2]);
const char* iv_buf = Buffer::Data(args[2]);
cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len);
}
}


Expand Down Expand Up @@ -6231,6 +6324,8 @@ void InitCrypto(Local<Object> target,
env->SetMethod(target, "certVerifySpkac", VerifySpkac);
env->SetMethod(target, "certExportPublicKey", ExportPublicKey);
env->SetMethod(target, "certExportChallenge", ExportChallenge);
env->SetMethod(target, "generateLegacyKey", GenerateLegacyKey);
env->SetMethod(target, "generateLegacyIV", GenerateLegacyIV);
#ifndef OPENSSL_NO_ENGINE
env->SetMethod(target, "setEngine", SetEngine);
#endif // !OPENSSL_NO_ENGINE
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ const errMessages = {
auth: / auth/,
state: / state/,
FIPS: /not supported in FIPS mode/,
IV: /no longer supported with ciphers that require initialization vectors/,
length: /Invalid IV length/,
};

Expand Down Expand Up @@ -390,7 +391,7 @@ for (const i in TEST_CASES) {
}

if (test.password) {
if (common.hasFipsCrypto) {
if (!test.algo.endsWith('ecb') && common.hasFipsCrypto) {
assert.throws(() => { crypto.createCipher(test.algo, test.password); },
errMessages.FIPS);
} else {
Expand Down
16 changes: 12 additions & 4 deletions test/parallel/test-crypto-binary-default.js
Original file line number Diff line number Diff line change
Expand Up @@ -456,17 +456,21 @@ assert.strictEqual(s3Verified, true, 'sign and verify (buffer)');


function testCipher1(key) {
const derived_key = crypto.generateLegacyKey('aes-192-cbc', key);
const iv = crypto.generateLegacyIV('aes-192-cbc', key);

// Test encryption and decryption
const plaintext = 'Keep this a secret? No! Tell everyone about node.js!';
const cipher = crypto.createCipher('aes192', key);
const cipher = crypto.createCipheriv('aes-192-cbc', derived_key, iv);

// encrypt plaintext which is in utf8 format
// to a ciphertext which will be in hex
let ciph = cipher.update(plaintext, 'utf8', 'hex');
// Only use binary or hex, not base64.
ciph += cipher.final('hex');

const decipher = crypto.createDecipher('aes192', key);
const decipher = crypto.createDecipheriv('aes-192-cbc', derived_key, iv);

let txt = decipher.update(ciph, 'hex', 'utf8');
txt += decipher.final('utf8');

Expand All @@ -481,14 +485,18 @@ function testCipher2(key) {
'32|RmVZZkFUVmpRRkp0TmJaUm56ZU9qcnJkaXNNWVNpTTU*|iXmckfRWZBGWWELw' +
'eCBsThSsfUHLeRe0KCsK8ooHgxie0zOINpXxfZi/oNG7uq9JWFVCk70gfzQH8ZUJ' +
'jAfaFg**';
const cipher = crypto.createCipher('aes256', key);

const derived_key = crypto.generateLegacyKey('aes-256-cbc', key);
const iv = crypto.generateLegacyIV('aes-256-cbc', key);

const cipher = crypto.createCipheriv('aes-256-cbc', derived_key, iv);

// encrypt plaintext which is in utf8 format
// to a ciphertext which will be in Base64
let ciph = cipher.update(plaintext, 'utf8', 'base64');
ciph += cipher.final('base64');

const decipher = crypto.createDecipher('aes256', key);
const decipher = crypto.createDecipheriv('aes-256-cbc', derived_key, iv);
let txt = decipher.update(ciph, 'base64', 'utf8');
txt += decipher.final('utf8');

Expand Down
Loading