diff --git a/crypto/evp_extra/p_pqdsa.c b/crypto/evp_extra/p_pqdsa.c index caed4935ad..57584765fe 100644 --- a/crypto/evp_extra/p_pqdsa.c +++ b/crypto/evp_extra/p_pqdsa.c @@ -280,13 +280,9 @@ EVP_PKEY *EVP_PKEY_pqdsa_new_raw_public_key(int nid, const uint8_t *in, size_t l goto err; } - const PQDSA *pqdsa = PQDSA_KEY_get0_dsa(ret->pkey.pqdsa_key); - if (pqdsa->public_key_len != len) { - OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_BUFFER_SIZE); - goto err; - } - - if (!PQDSA_KEY_set_raw_public_key(ret->pkey.pqdsa_key, in)) { + CBS cbs; + CBS_init(&cbs, in, len); + if (!PQDSA_KEY_set_raw_public_key(ret->pkey.pqdsa_key, &cbs)) { // PQDSA_KEY_set_raw_public_key sets the appropriate error. goto err; } @@ -316,7 +312,9 @@ EVP_PKEY *EVP_PKEY_pqdsa_new_raw_private_key(int nid, const uint8_t *in, size_t goto err; } - if (!PQDSA_KEY_set_raw_private_key(ret->pkey.pqdsa_key, in)) { + CBS cbs; + CBS_init(&cbs, in, len); + if (!PQDSA_KEY_set_raw_private_key(ret->pkey.pqdsa_key, &cbs)) { // PQDSA_KEY_set_raw_private_key sets the appropriate error. goto err; } diff --git a/crypto/evp_extra/p_pqdsa_asn1.c b/crypto/evp_extra/p_pqdsa_asn1.c index b809407005..aa47734593 100644 --- a/crypto/evp_extra/p_pqdsa_asn1.c +++ b/crypto/evp_extra/p_pqdsa_asn1.c @@ -89,18 +89,19 @@ static int pqdsa_get_pub_raw(const EVP_PKEY *pkey, uint8_t *out, } static int pqdsa_pub_decode(EVP_PKEY *out, CBS *params, CBS *key) { - // See https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/ section 4. - // the only parameter that can be included is the OID which has length 9 + // See https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/ + // section 4. the only parameter that can be included is the OID which has + // length 9 if (CBS_len(params) != 9) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return 0; } - // set the pqdsa params on the fresh pkey + // Set the pqdsa params on |out|. if (!EVP_PKEY_pqdsa_set_params(out, OBJ_cbs2nid(params))) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return 0; } - return PQDSA_KEY_set_raw_public_key(out->pkey.pqdsa_key,CBS_data(key)); + return PQDSA_KEY_set_raw_public_key(out->pkey.pqdsa_key, key); } static int pqdsa_pub_encode(CBB *out, const EVP_PKEY *pkey) { @@ -138,21 +139,22 @@ static int pqdsa_pub_cmp(const EVP_PKEY *a, const EVP_PKEY *b) { } static int pqdsa_priv_decode(EVP_PKEY *out, CBS *params, CBS *key, CBS *pubkey) { - // See https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/ section 6. - // the only parameter that can be included is the OID which has length 9 - if (CBS_len(params) != 9 ) { + // See https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/ + // section 6. the only parameter that can be included is the OID which has + // length 9. + if (CBS_len(params) != 9) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return 0; } - // Set the pqdsa params on the fresh pkey + // Set the pqdsa params on |out|. if (!EVP_PKEY_pqdsa_set_params(out, OBJ_cbs2nid(params))) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return 0; } // Set the private key - if (!PQDSA_KEY_set_raw_private_key(out->pkey.pqdsa_key, CBS_data(key))) { + if (!PQDSA_KEY_set_raw_private_key(out->pkey.pqdsa_key, key)) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return 0; } @@ -167,7 +169,8 @@ static int pqdsa_priv_decode(EVP_PKEY *out, CBS *params, CBS *key, CBS *pubkey) } // Construct the public key from the private key - if (!out->pkey.pqdsa_key->pqdsa->method->pqdsa_pack_pk_from_sk(public_key, CBS_data(key))) { + if (!out->pkey.pqdsa_key->pqdsa->method->pqdsa_pack_pk_from_sk( + public_key, CBS_data(key))) { OPENSSL_free(public_key); OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return 0; diff --git a/crypto/evp_extra/p_pqdsa_test.cc b/crypto/evp_extra/p_pqdsa_test.cc index 590107705c..a01e27bb0d 100644 --- a/crypto/evp_extra/p_pqdsa_test.cc +++ b/crypto/evp_extra/p_pqdsa_test.cc @@ -1854,3 +1854,27 @@ TEST_P(PerMLDSATest, ACVPSigVer) { } }); } + +static const uint8_t mldsa87kPublicKeyInvalidLength[] = { + 0x30, 0x11, 0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, + 0x65, 0x03, 0x04, 0x03, 0x13, 0x03, 0x02, 0x00, 0xe4}; + +TEST(PQDSAParameterTest, ParsePublicKeyInvalidLength) { + CBS cbs; + CBS_init(&cbs, mldsa87kPublicKeyInvalidLength, + sizeof(mldsa87kPublicKeyInvalidLength)); + bssl::UniquePtr pub_pkey_from_der(EVP_parse_public_key(&cbs)); + ASSERT_FALSE(pub_pkey_from_der.get()); +} + +static const uint8_t mldsa44kPrivateKeyInvalidLength[] = { + 0x30, 0x16, 0x02, 0x01, 0x00, 0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, + 0x01, 0x65, 0x03, 0x04, 0x03, 0x11, 0x04, 0x04, 0x82, 0x45, 0x52, 0xd8}; + +TEST(PQDSAParameterTest, ParsePrivateKeyInvalidLength) { + CBS cbs; + CBS_init(&cbs, mldsa44kPrivateKeyInvalidLength, + sizeof(mldsa44kPrivateKeyInvalidLength)); + bssl::UniquePtr private_pkey_from_der(EVP_parse_private_key(&cbs)); + ASSERT_FALSE(private_pkey_from_der.get()); +} diff --git a/crypto/pqdsa/internal.h b/crypto/pqdsa/internal.h index 3ecd5334b5..7a3aabdd66 100644 --- a/crypto/pqdsa/internal.h +++ b/crypto/pqdsa/internal.h @@ -76,8 +76,8 @@ PQDSA_KEY *PQDSA_KEY_new(void); void PQDSA_KEY_free(PQDSA_KEY *key); int EVP_PKEY_pqdsa_set_params(EVP_PKEY *pkey, int nid); -int PQDSA_KEY_set_raw_public_key(PQDSA_KEY *key, const uint8_t *in); -int PQDSA_KEY_set_raw_private_key(PQDSA_KEY *key, const uint8_t *in); +int PQDSA_KEY_set_raw_public_key(PQDSA_KEY *key, CBS *in); +int PQDSA_KEY_set_raw_private_key(PQDSA_KEY *key, CBS *in); #if defined(__cplusplus) } // extern C #endif diff --git a/crypto/pqdsa/pqdsa.c b/crypto/pqdsa/pqdsa.c index b32dae90f6..49f5f9d58d 100644 --- a/crypto/pqdsa/pqdsa.c +++ b/crypto/pqdsa/pqdsa.c @@ -66,8 +66,14 @@ const PQDSA *PQDSA_KEY_get0_dsa(PQDSA_KEY* key) { return key->pqdsa; } -int PQDSA_KEY_set_raw_public_key(PQDSA_KEY *key, const uint8_t *in) { - key->public_key = OPENSSL_memdup(in, key->pqdsa->public_key_len); +int PQDSA_KEY_set_raw_public_key(PQDSA_KEY *key, CBS *in) { + // Check if the parsed length corresponds with the expected length. + if (CBS_len(in) != key->pqdsa->public_key_len) { + OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_BUFFER_SIZE); + return 0; + } + + key->public_key = OPENSSL_memdup(CBS_data(in), key->pqdsa->public_key_len); if (key->public_key == NULL) { return 0; } @@ -75,8 +81,14 @@ int PQDSA_KEY_set_raw_public_key(PQDSA_KEY *key, const uint8_t *in) { return 1; } -int PQDSA_KEY_set_raw_private_key(PQDSA_KEY *key, const uint8_t *in) { - key->private_key = OPENSSL_memdup(in, key->pqdsa->private_key_len); +int PQDSA_KEY_set_raw_private_key(PQDSA_KEY *key, CBS *in) { + // Check if the parsed length corresponds with the expected length. + if (CBS_len(in) != key->pqdsa->private_key_len) { + OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_BUFFER_SIZE); + return 0; + } + + key->private_key = OPENSSL_memdup(CBS_data(in), key->pqdsa->private_key_len); if (key->private_key == NULL) { return 0; }