Skip to content

Commit

Permalink
crypto: always accept certificates as public keys
Browse files Browse the repository at this point in the history
PR-URL: #24234
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
tniessen authored and MylesBorins committed Dec 25, 2018
1 parent 7c4dd14 commit 25c6053
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 19 deletions.
2 changes: 2 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1831,6 +1831,8 @@ Creates and returns a new key object containing a public key. If `key` is a
string or `Buffer`, `format` is assumed to be `'pem'`; otherwise, `key`
must be an object with the properties described above.

If the format is `'pem'`, the `'key'` may also be an X.509 certificate.

### crypto.createSecretKey(key)
<!-- YAML
added: REPLACEME
Expand Down
31 changes: 12 additions & 19 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2710,8 +2710,7 @@ static ParsePublicKeyResult TryParsePublicKey(

static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
const char* key_pem,
int key_pem_len,
bool allow_certificate) {
int key_pem_len) {
BIOPointer bp(BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len));
if (!bp)
return ParsePublicKeyResult::kParsePublicFailed;
Expand All @@ -2732,8 +2731,7 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
[](const unsigned char** p, long l) { // NOLINT(runtime/int)
return d2i_PublicKey(EVP_PKEY_RSA, nullptr, p, l);
});
if (ret != ParsePublicKeyResult::kParsePublicNotRecognized ||
!allow_certificate)
if (ret != ParsePublicKeyResult::kParsePublicNotRecognized)
return ret;

// X.509 fallback.
Expand All @@ -2748,11 +2746,10 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
static bool ParsePublicKey(EVPKeyPointer* pkey,
const PublicKeyEncodingConfig& config,
const char* key,
size_t key_len,
bool allow_certificate) {
size_t key_len) {
if (config.format_ == kKeyFormatPEM) {
ParsePublicKeyResult r =
ParsePublicKeyPEM(pkey, key, key_len, allow_certificate);
ParsePublicKeyPEM(pkey, key, key_len);
return r == ParsePublicKeyResult::kParsePublicOk;
} else {
CHECK_EQ(config.format_, kKeyFormatDER);
Expand Down Expand Up @@ -3002,15 +2999,14 @@ static PublicKeyEncodingConfig GetPublicKeyEncodingFromJs(
static ManagedEVPPKey GetPublicKeyFromJs(
const FunctionCallbackInfo<Value>& args,
unsigned int* offset,
bool allow_key_object,
bool allow_certificate) {
bool allow_key_object) {
if (args[*offset]->IsString() || Buffer::HasInstance(args[*offset])) {
Environment* env = Environment::GetCurrent(args);
ByteSource key = ByteSource::FromStringOrBuffer(env, args[(*offset)++]);
PublicKeyEncodingConfig config =
GetPublicKeyEncodingFromJs(args, offset, kKeyContextInput);
EVPKeyPointer pkey;
ParsePublicKey(&pkey, config, key.get(), key.size(), allow_certificate);
ParsePublicKey(&pkey, config, key.get(), key.size());
if (!pkey)
ThrowCryptoError(env, ERR_get_error(), "Failed to read public key");
return ManagedEVPPKey(pkey.release());
Expand Down Expand Up @@ -3131,8 +3127,7 @@ static bool IsRSAPrivateKey(const unsigned char* data, size_t size) {
static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
const FunctionCallbackInfo<Value>& args,
unsigned int* offset,
bool allow_key_object,
bool allow_certificate) {
bool allow_key_object) {
if (args[*offset]->IsString() || Buffer::HasInstance(args[*offset])) {
Environment* env = Environment::GetCurrent(args);
ByteSource data = ByteSource::FromStringOrBuffer(env, args[(*offset)++]);
Expand All @@ -3146,8 +3141,7 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
// For PEM, we can easily determine whether it is a public or private key
// by looking for the respective PEM tags.
ParsePublicKeyResult ret = ParsePublicKeyPEM(&pkey, data.get(),
data.size(),
allow_certificate);
data.size());
if (ret == ParsePublicKeyResult::kParsePublicNotRecognized) {
pkey = ParsePrivateKey(config, data.get(), data.size());
}
Expand All @@ -3172,8 +3166,7 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
}

if (is_public) {
ParsePublicKey(&pkey, config, data.get(), data.size(),
allow_certificate);
ParsePublicKey(&pkey, config, data.get(), data.size());
} else {
pkey = ParsePrivateKey(config, data.get(), data.size());
}
Expand Down Expand Up @@ -3386,7 +3379,7 @@ void KeyObject::Init(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(args.Length(), 3);

offset = 0;
pkey = GetPublicKeyFromJs(args, &offset, false, false);
pkey = GetPublicKeyFromJs(args, &offset, false);
if (!pkey)
return;
key->InitPublic(pkey);
Expand Down Expand Up @@ -4668,7 +4661,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());

unsigned int offset = 0;
ManagedEVPPKey pkey = GetPublicKeyFromJs(args, &offset, true, true);
ManagedEVPPKey pkey = GetPublicKeyFromJs(args, &offset, true);

char* hbuf = Buffer::Data(args[offset]);
ssize_t hlen = Buffer::Length(args[offset]);
Expand Down Expand Up @@ -4724,7 +4717,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

unsigned int offset = 0;
ManagedEVPPKey pkey = GetPublicOrPrivateKeyFromJs(args, &offset, true, true);
ManagedEVPPKey pkey = GetPublicOrPrivateKeyFromJs(args, &offset, true);
if (!pkey)
return;

Expand Down

0 comments on commit 25c6053

Please sign in to comment.