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.

Sign up for GitHub

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: fix error code handling in ParsePrivateKey() #42400

Closed
wants to merge 1 commit into from
Closed

crypto: fix error code handling in ParsePrivateKey() #42400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/crypto/crypto_context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
X509Pointer* cert,
X509Pointer* issuer) {
// Just to ensure that `ERR_peek_last_error` below will return only errors
// that we are interested in
// that we are interested in.
ERR_clear_error();

X509Pointer x(
Expand Down
6 changes: 5 additions & 1 deletion src/crypto/crypto_keys.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
const PrivateKeyEncodingConfig& config,
const char* key,
size_t key_len) {
// Just to ensure that `ERR_peek_last_error` below will return only errors
// that we are interested in.
ERR_clear_error();

const ByteSource* passphrase = config.passphrase_.get();

if (config.format_ == kKeyFormatPEM) {
Expand Down Expand Up @@ -255,7 +259,7 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
}

// OpenSSL can fail to parse the key but still return a non-null pointer.
unsigned long err = ERR_peek_error(); // NOLINT(runtime/int)
unsigned long err = ERR_peek_last_error(); // NOLINT(runtime/int)
if (err != 0)
pkey->reset();

Expand Down
6 changes: 1 addition & 5 deletions test/parallel/test-crypto-key-objects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,7 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',

{
// Reading an encrypted key without a passphrase should fail.
assert.throws(() => createPrivateKey(privateDsa), common.hasOpenSSL3 ? {
name: 'Error',
message: 'error:07880109:common libcrypto routines::interrupted or ' +
'cancelled',
} : {
assert.throws(() => createPrivateKey(privateDsa), {
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
Expand Down
35 changes: 7 additions & 28 deletions test/parallel/test-crypto-keygen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);

// Since the private key is encrypted, signing shouldn't work anymore.
const publicKey = { key: publicKeyDER, ...publicKeyEncoding };
const expectedError = common.hasOpenSSL3 ? {
name: 'Error',
message: 'error:07880109:common libcrypto routines::interrupted or ' +
'cancelled'
} : {
assert.throws(() => testSignVerify(publicKey, privateKey), {
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
};
assert.throws(() => testSignVerify(publicKey, privateKey), expectedError);
});

const key = { key: privateKey, passphrase: 'secret' };
testEncryptDecrypt(publicKey, key);
Expand Down Expand Up @@ -565,10 +560,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);

// Since the private key is encrypted, signing shouldn't work anymore.
assert.throws(() => testSignVerify(publicKey, privateKey),
common.hasOpenSSL3 ? {
message: 'error:07880109:common libcrypto ' +
'routines::interrupted or cancelled'
} : {
{
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
Expand Down Expand Up @@ -599,10 +591,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);

// Since the private key is encrypted, signing shouldn't work anymore.
assert.throws(() => testSignVerify(publicKey, privateKey),
common.hasOpenSSL3 ? {
message: 'error:07880109:common libcrypto ' +
'routines::interrupted or cancelled'
} : {
{
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
Expand Down Expand Up @@ -636,10 +625,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);

// Since the private key is encrypted, signing shouldn't work anymore.
assert.throws(() => testSignVerify(publicKey, privateKey),
common.hasOpenSSL3 ? {
message: 'error:07880109:common libcrypto ' +
'routines::interrupted or cancelled'
} : {
{
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
Expand Down Expand Up @@ -674,10 +660,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);

// Since the private key is encrypted, signing shouldn't work anymore.
assert.throws(() => testSignVerify(publicKey, privateKey),
common.hasOpenSSL3 ? {
message: 'error:07880109:common libcrypto ' +
'routines::interrupted or cancelled'
} : {
{
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
Expand Down Expand Up @@ -1571,11 +1554,7 @@ for (const type of ['pkcs1', 'pkcs8']) {
// the key, and not specifying a passphrase should fail when decoding it.
assert.throws(() => {
return testSignVerify(publicKey, privateKey);
}, common.hasOpenSSL3 ? {
name: 'Error',
code: 'ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED',
message: 'error:07880109:common libcrypto routines::interrupted or cancelled'
} : {
}, {
name: 'TypeError',
code: 'ERR_MISSING_PASSPHRASE',
message: 'Passphrase required for encrypted key'
Expand Down
5 changes: 1 addition & 4 deletions test/parallel/test-crypto-private-decrypt-gh32240.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,5 @@ function decrypt(key) {
}

decrypt(pkey);
assert.throws(() => decrypt(pkeyEncrypted), common.hasOpenSSL3 ?
{ message: 'error:07880109:common libcrypto routines::interrupted or ' +
'cancelled' } :
{ code: 'ERR_MISSING_PASSPHRASE' });
assert.throws(() => decrypt(pkeyEncrypted), { code: 'ERR_MISSING_PASSPHRASE' });
decrypt(pkey); // Should not throw.