Skip to content

Ensure no trailing data for PKCS8 EVP_parse_private_key #3242

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

Merged
torben-hansen merged 2 commits into from
May 12, 2026
+30 −0
Merged

Ensure no trailing data for PKCS8 EVP_parse_private_key #3242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc8a098
Ensure no trailing data for PKCS8 EVP_parse_private_key
torben-hansen May 11, 2026
5003d9c
Remove comment
torben-hansen May 11, 2026
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
6 changes: 6 additions & 0 deletions crypto/evp_extra/evp_asn1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ EVP_PKEY *EVP_parse_private_key(CBS *cbs) {
has_pub = 1;
}

// Reject trailing data within the SEQUENCE.
if (CBS_len(&pkcs8) != 0) {
OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR);
return NULL;
}

// Set up an |EVP_PKEY| of the appropriate type.
EVP_PKEY *ret = EVP_PKEY_new();
if (ret == NULL) {
Expand Down
24 changes: 24 additions & 0 deletions crypto/evp_extra/evp_extra_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,30 @@ TEST(EVPExtraTest, Ed25519) {
ERR_clear_error();
}

// EVP_parse_private_key must reject trailing data inside the PrivateKeyInfo
// SEQUENCE.
TEST(EVPExtraTest, ParsePrivateKeyRejectsTrailingData) {
// Ed25519 PKCS#8 (the kPrivateKeyPKCS8 vector from EVPExtraTest.Ed25519)
// with two extra bytes (ASN.1 NULL: 05 00) appended inside the outer
// SEQUENCE. The SEQUENCE length byte is bumped 0x2e -> 0x30 to cover
// the extra bytes, so the outer TLV is itself well-formed.
static const uint8_t kDERWithTrailing[] = {
0x30, 0x30, 0x02, 0x01, 0x00, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x70,
0x04, 0x22, 0x04, 0x20, 0x9d, 0x61, 0xb1, 0x9d, 0xef, 0xfd, 0x5a, 0x60,
0xba, 0x84, 0x4a, 0xf4, 0x92, 0xec, 0x2c, 0xc4, 0x44, 0x49, 0xc5, 0x69,
0x7b, 0x32, 0x69, 0x19, 0x70, 0x3b, 0xac, 0x03, 0x1c, 0xae, 0x7f, 0x60,
0x05, 0x00,
};

CBS cbs;
CBS_init(&cbs, kDERWithTrailing, sizeof(kDERWithTrailing));
ERR_clear_error();
bssl::UniquePtr<EVP_PKEY> parsed(EVP_parse_private_key(&cbs));
EXPECT_FALSE(parsed);
EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_EVP, EVP_R_DECODE_ERROR));
ERR_clear_error();
}

Copy link
Copy Markdown
Contributor

@justsmth justsmth May 12, 2026

Choose a reason for hiding this comment

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

Could we also test a V2?

// EVP_parse_private_key must also reject trailing data inside the SEQUENCE
// after the optional [1] publicKey of an RFC 5958 v2 OneAsymmetricKey. This
// pins the invariant that the trailing-data check fires after the optional
// fields are consumed.
TEST(EVPExtraTest, ParsePrivateKeyV2RejectsTrailingData) {
  // Base vector is the v2 OneAsymmetricKey from
  // EVPExtraTest.Ed25519PKCS8v2WithAttributes (empty [0] attributes and a
  // [1] publicKey), with two extra bytes (ASN.1 NULL: 05 00) appended inside
  // the outer SEQUENCE. The outer SEQUENCE length byte is bumped
  // 0x53 -> 0x55 to cover the extra bytes, so the outer TLV is itself
  // well-formed.
  static const uint8_t kDERWithTrailing[] = {
    0x30, 0x55,                                     // SEQUENCE, len 0x55
      0x02, 0x01, 0x01,                             // version = 1 (v2)
      0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x70,     // AlgId { OID 1.3.101.112 }
      0x04, 0x22,                                   // OCTET STRING, len 0x22
        0x04, 0x20,                                 //   OCTET STRING, len 32
        0x9d, 0x61, 0xb1, 0x9d, 0xef, 0xfd, 0x5a, 0x60,
        0xba, 0x84, 0x4a, 0xf4, 0x92, 0xec, 0x2c, 0xc4,
        0x44, 0x49, 0xc5, 0x69, 0x7b, 0x32, 0x69, 0x19,
        0x70, 0x3b, 0xac, 0x03, 0x1c, 0xae, 0x7f, 0x60,
      0xa0, 0x00,                                   // [0] attributes = { }
      0x81, 0x21, 0x00,                             // [1] publicKey BIT STRING
        0xd7, 0x5a, 0x98, 0x01, 0x82, 0xb1, 0x0a, 0xb7,
        0xd5, 0x4b, 0xfe, 0xd3, 0xc9, 0x64, 0x07, 0x3a,
        0x0e, 0xe1, 0x72, 0xf3, 0xda, 0xa6, 0x23, 0x25,
        0xaf, 0x02, 0x1a, 0x68, 0xf7, 0x07, 0x51, 0x1a,
      0x05, 0x00,                                   // trailing ASN.1 NULL
  };

  CBS cbs;
  CBS_init(&cbs, kDERWithTrailing, sizeof(kDERWithTrailing));
  ERR_clear_error();
  bssl::UniquePtr<EVP_PKEY> parsed(EVP_parse_private_key(&cbs));
  EXPECT_FALSE(parsed);
  EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_EVP, EVP_R_DECODE_ERROR));
  ERR_clear_error();
}

Copy link
Copy Markdown
Contributor Author

@torben-hansen torben-hansen May 12, 2026

Choose a reason for hiding this comment

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

It's basically redundant because the strict no trailing data is not an invariant that depends on the version. The sequence exist in both versions.

// EVP_PKEY_get_private_seed returns an error for key types that don't
// provide a get_priv_seed method. It must also reject NULL |key| regardless of
// key type.
Expand Down
Loading