diff --git a/src/lib/asn1/alg_id.cpp b/src/lib/asn1/alg_id.cpp index ec594e4eba3..75e569af103 100644 --- a/src/lib/asn1/alg_id.cpp +++ b/src/lib/asn1/alg_id.cpp @@ -57,6 +57,7 @@ bool operator==(const AlgorithmIdentifier& a1, const AlgorithmIdentifier& a2) { /* * Treat NULL and empty as equivalent + * TODO(Botan4) remove this */ if(a1.parameters_are_null_or_empty() && a2.parameters_are_null_or_empty()) { return true; diff --git a/src/lib/asn1/pss_params.cpp b/src/lib/asn1/pss_params.cpp index 956a3944f18..4e44d15b2e8 100644 --- a/src/lib/asn1/pss_params.cpp +++ b/src/lib/asn1/pss_params.cpp @@ -76,6 +76,10 @@ void PSS_Params::decode_from(BER_Decoder& from) { .end_cons(); BER_Decoder(m_mgf.parameters(), from.limits()).decode(m_mgf_hash).verify_end(); + + if(!m_hash.parameters_are_null_or_empty() || !m_mgf_hash.parameters_are_null_or_empty()) { + throw Decoding_Error("Unexpected parameters for PSS hash algorithm identifier"); + } } } // namespace Botan diff --git a/src/lib/pubkey/curve448/ed448/ed448.cpp b/src/lib/pubkey/curve448/ed448/ed448.cpp index 3812e32a05a..bf99d6ac416 100644 --- a/src/lib/pubkey/curve448/ed448/ed448.cpp +++ b/src/lib/pubkey/curve448/ed448/ed448.cpp @@ -283,7 +283,7 @@ std::unique_ptr Ed448_PublicKey::create_verification_op(st std::unique_ptr Ed448_PublicKey::create_x509_verification_op(const AlgorithmIdentifier& alg_id, std::string_view provider) const { if(provider == "base" || provider.empty()) { - if(alg_id != this->algorithm_identifier()) { + if(alg_id.oid() != this->object_identifier() || !alg_id.parameters_are_empty()) { throw Decoding_Error("Unexpected AlgorithmIdentifier for Ed448 X509 signature"); } diff --git a/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp b/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp index ae071d7bacc..fd04b2f05ee 100644 --- a/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp +++ b/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp @@ -405,7 +405,7 @@ std::unique_ptr Dilithium_PublicKey::create_verification_o std::unique_ptr Dilithium_PublicKey::create_x509_verification_op( const AlgorithmIdentifier& alg_id, std::string_view provider) const { if(provider.empty() || provider == "base") { - if(alg_id != this->algorithm_identifier()) { + if(alg_id.oid() != this->object_identifier() || !alg_id.parameters_are_empty()) { throw Decoding_Error("Unexpected AlgorithmIdentifier for Dilithium X.509 signature"); } return std::make_unique(m_public); diff --git a/src/lib/pubkey/ecc_key/ecc_key.cpp b/src/lib/pubkey/ecc_key/ecc_key.cpp index 9258e968642..9321e4201b5 100644 --- a/src/lib/pubkey/ecc_key/ecc_key.cpp +++ b/src/lib/pubkey/ecc_key/ecc_key.cpp @@ -43,6 +43,16 @@ EC_Group_Encoding default_encoding_for(const EC_Group& group) { } // namespace +const AlgorithmIdentifier& EC_PublicKey::assert_algorithm_identifier(const AlgorithmIdentifier& alg_id, + std::string_view alg_name) { + if(alg_id.oid() != OID::from_string(alg_name)) { + throw Decoding_Error( + fmt("Unexpected AlgorithmIdentifier OID {} in association with {} key", alg_id.oid(), alg_name)); + } + + return alg_id; +} + #if defined(BOTAN_HAS_LEGACY_EC_POINT) EC_PublicKey::EC_PublicKey(const EC_Group& group, const EC_Point& pub_point) { auto pt = EC_AffinePoint(group, pub_point); diff --git a/src/lib/pubkey/ecc_key/ecc_key.h b/src/lib/pubkey/ecc_key/ecc_key.h index b33b20b2b8d..ce0d250a790 100644 --- a/src/lib/pubkey/ecc_key/ecc_key.h +++ b/src/lib/pubkey/ecc_key/ecc_key.h @@ -134,6 +134,9 @@ class BOTAN_PUBLIC_API(2, 0) EC_PublicKey : public virtual Public_Key { */ EC_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits); + static const AlgorithmIdentifier& assert_algorithm_identifier(const AlgorithmIdentifier& alg_id, + std::string_view alg_name); + EC_PublicKey() = default; std::shared_ptr m_public_key; // NOLINT(*non-private-member-variable*) diff --git a/src/lib/pubkey/ecdh/ecdh.h b/src/lib/pubkey/ecdh/ecdh.h index 0bb747209bd..7a457bb0e63 100644 --- a/src/lib/pubkey/ecdh/ecdh.h +++ b/src/lib/pubkey/ecdh/ecdh.h @@ -25,7 +25,7 @@ class BOTAN_PUBLIC_API(2, 0) ECDH_PublicKey : public virtual EC_PublicKey { * @param key_bits DER encoded public key bits */ ECDH_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PublicKey(alg_id, key_bits) {} + EC_PublicKey(assert_algorithm_identifier(alg_id, "ECDH"), key_bits) {} #if defined(BOTAN_HAS_LEGACY_EC_POINT) /** @@ -84,7 +84,7 @@ class BOTAN_PUBLIC_API(2, 0) ECDH_PrivateKey final : public ECDH_PublicKey, * @param key_bits ECPrivateKey bits */ ECDH_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PrivateKey(alg_id, key_bits) {} + EC_PrivateKey(assert_algorithm_identifier(alg_id, "ECDH"), key_bits) {} /** * Create a private key from a given secret @p x diff --git a/src/lib/pubkey/ecdsa/ecdsa.cpp b/src/lib/pubkey/ecdsa/ecdsa.cpp index 1c79b3a506a..28c580df975 100644 --- a/src/lib/pubkey/ecdsa/ecdsa.cpp +++ b/src/lib/pubkey/ecdsa/ecdsa.cpp @@ -218,7 +218,7 @@ class ECDSA_Verification_Operation final : public PK_Ops::Verification_with_Hash PK_Ops::Verification_with_Hash(padding), m_group(ecdsa.domain()), m_gy_mul(ecdsa._public_ec_point()) {} ECDSA_Verification_Operation(const ECDSA_PublicKey& ecdsa, const AlgorithmIdentifier& alg_id) : - PK_Ops::Verification_with_Hash(alg_id, "ECDSA", true), + PK_Ops::Verification_with_Hash(alg_id, "ECDSA"), m_group(ecdsa.domain()), m_gy_mul(ecdsa._public_ec_point()) {} diff --git a/src/lib/pubkey/ecdsa/ecdsa.h b/src/lib/pubkey/ecdsa/ecdsa.h index d77110c5eca..1013ecfb24c 100644 --- a/src/lib/pubkey/ecdsa/ecdsa.h +++ b/src/lib/pubkey/ecdsa/ecdsa.h @@ -41,7 +41,7 @@ class BOTAN_PUBLIC_API(2, 0) ECDSA_PublicKey : public virtual EC_PublicKey { * @param key_bits DER encoded public key bits */ ECDSA_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PublicKey(alg_id, key_bits) {} + EC_PublicKey(assert_algorithm_identifier(alg_id, "ECDSA"), key_bits) {} /** * Recover a public key from a signature/msg pair @@ -95,7 +95,7 @@ class BOTAN_PUBLIC_API(2, 0) ECDSA_PrivateKey final : public ECDSA_PublicKey, * @param key_bits ECPrivateKey bits */ ECDSA_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PrivateKey(alg_id, key_bits) {} + EC_PrivateKey(assert_algorithm_identifier(alg_id, "ECDSA"), key_bits) {} /** * Create a private key from a given secret @p x diff --git a/src/lib/pubkey/ecgdsa/ecgdsa.h b/src/lib/pubkey/ecgdsa/ecgdsa.h index 8c00b203c7e..0fc5b05bee3 100644 --- a/src/lib/pubkey/ecgdsa/ecgdsa.h +++ b/src/lib/pubkey/ecgdsa/ecgdsa.h @@ -39,7 +39,7 @@ class BOTAN_PUBLIC_API(2, 0) ECGDSA_PublicKey : public virtual EC_PublicKey { * @param key_bits DER encoded public key bits */ ECGDSA_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PublicKey(alg_id, key_bits) {} + EC_PublicKey(assert_algorithm_identifier(alg_id, "ECGDSA"), key_bits) {} /** * Get this keys algorithm name. @@ -79,7 +79,7 @@ class BOTAN_PUBLIC_API(2, 0) ECGDSA_PrivateKey final : public ECGDSA_PublicKey, * @param key_bits ECPrivateKey bits */ ECGDSA_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PrivateKey(alg_id, key_bits, true) {} + EC_PrivateKey(assert_algorithm_identifier(alg_id, "ECGDSA"), key_bits, true) {} /** * Create a private key from a given secret @p x diff --git a/src/lib/pubkey/eckcdsa/eckcdsa.h b/src/lib/pubkey/eckcdsa/eckcdsa.h index 0773574996d..1f0ce518409 100644 --- a/src/lib/pubkey/eckcdsa/eckcdsa.h +++ b/src/lib/pubkey/eckcdsa/eckcdsa.h @@ -39,7 +39,7 @@ class BOTAN_PUBLIC_API(2, 0) ECKCDSA_PublicKey : public virtual EC_PublicKey { * @param key_bits DER encoded public key bits */ ECKCDSA_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PublicKey(alg_id, key_bits) {} + EC_PublicKey(assert_algorithm_identifier(alg_id, "ECKCDSA"), key_bits) {} /** * Get this keys algorithm name. @@ -78,7 +78,7 @@ class BOTAN_PUBLIC_API(2, 0) ECKCDSA_PrivateKey final : public ECKCDSA_PublicKey * @param key_bits ECPrivateKey bits */ ECKCDSA_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PrivateKey(alg_id, key_bits, true) {} + EC_PrivateKey(assert_algorithm_identifier(alg_id, "ECKCDSA"), key_bits, true) {} /** * Create a private key from a given secret @p x diff --git a/src/lib/pubkey/ed25519/ed25519_key.cpp b/src/lib/pubkey/ed25519/ed25519_key.cpp index a088b74f07e..ecfa2d039cb 100644 --- a/src/lib/pubkey/ed25519/ed25519_key.cpp +++ b/src/lib/pubkey/ed25519/ed25519_key.cpp @@ -346,7 +346,7 @@ std::unique_ptr Ed25519_PublicKey::create_verification_op( std::unique_ptr Ed25519_PublicKey::create_x509_verification_op(const AlgorithmIdentifier& alg_id, std::string_view provider) const { if(provider == "base" || provider.empty()) { - if(alg_id != this->algorithm_identifier()) { + if(alg_id.oid() != this->object_identifier() || !alg_id.parameters_are_empty()) { throw Decoding_Error("Unexpected AlgorithmIdentifier for Ed25519 X509 signature"); } diff --git a/src/lib/pubkey/gost_3410/gost_3410.cpp b/src/lib/pubkey/gost_3410/gost_3410.cpp index 0969260e639..cee75d6a00e 100644 --- a/src/lib/pubkey/gost_3410/gost_3410.cpp +++ b/src/lib/pubkey/gost_3410/gost_3410.cpp @@ -27,6 +27,82 @@ EC_Group check_domain(EC_Group domain) { return domain; } +bool is_gost_3410_key_oid(const OID& oid) { + return oid == OID::from_string("GOST-34.10") || oid == OID::from_string("GOST-34.10-2012-256") || + oid == OID::from_string("GOST-34.10-2012-512"); +} + +const AlgorithmIdentifier& assert_gost_algorithm_identifier(const AlgorithmIdentifier& alg_id) { + if(!is_gost_3410_key_oid(alg_id.oid())) { + throw Decoding_Error( + fmt("Unexpected AlgorithmIdentifier OID {} in association with GOST 34.10 key", alg_id.oid())); + } + + return alg_id; +} + +OID decode_gost_key_parameters(const AlgorithmIdentifier& alg_id) { + OID ecc_param_id; + + auto params = BER_Decoder(alg_id.parameters(), BER_Decoder::Limits::DER()).start_sequence(); + params.decode(ecc_param_id); + + if(params.more_items()) { + OID digest_param_id; + params.decode(digest_param_id); + + if(alg_id.oid() == OID::from_string("GOST-34.10-2012-256") && + digest_param_id != OID::from_string("Streebog-256")) { + throw Decoding_Error("Unexpected digest parameters for GOST-34.10-2012-256 public key"); + } + + if(alg_id.oid() == OID::from_string("GOST-34.10-2012-512") && + digest_param_id != OID::from_string("Streebog-512")) { + throw Decoding_Error("Unexpected digest parameters for GOST-34.10-2012-512 public key"); + } + } + + if(params.more_items()) { + if(alg_id.oid() != OID::from_string("GOST-34.10")) { + throw Decoding_Error("Unexpected extra parameters for GOST-34.10-2012 public key"); + } + + OID encryption_param_id; + params.decode(encryption_param_id); + } + + params.verify_end(); + + return ecc_param_id; +} + +void check_gost_key_oid_matches_group(const OID& key_oid, const EC_Group& group) { + if(key_oid == OID::from_string("GOST-34.10-2012-256") && group.get_p_bits() != 256) { + throw Decoding_Error("GOST-34.10-2012-256 public key has unexpected parameters"); + } + + if(key_oid == OID::from_string("GOST-34.10-2012-512") && group.get_p_bits() != 512) { + throw Decoding_Error("GOST-34.10-2012-512 public key has unexpected parameters"); + } +} + +AlgorithmIdentifier gost_private_key_alg_id(const AlgorithmIdentifier& alg_id) { + assert_gost_algorithm_identifier(alg_id); + + OID ecc_param_id; + BER_Decoder decoder(alg_id.parameters(), BER_Decoder::Limits::DER()); + if(decoder.peek_next_object().type_tag() == ASN1_Type::ObjectId) { + decoder.decode(ecc_param_id).verify_end(); + } else { + ecc_param_id = decode_gost_key_parameters(alg_id); + } + + auto group = check_domain(EC_Group::from_OID(ecc_param_id)); + check_gost_key_oid_matches_group(alg_id.oid(), group); + + return AlgorithmIdentifier(alg_id.oid(), group.DER_encode()); +} + } // namespace std::optional GOST_3410_PublicKey::_signature_element_size_for_DER_encoding() const { @@ -71,17 +147,12 @@ AlgorithmIdentifier GOST_3410_PublicKey::algorithm_identifier() const { } GOST_3410_PublicKey::GOST_3410_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) { - OID ecc_param_id; + assert_gost_algorithm_identifier(alg_id); - // The parameters also includes hash and cipher OIDs - BER_Decoder(alg_id.parameters(), BER_Decoder::Limits::DER()) - .start_sequence() - .decode(ecc_param_id) - .discard_remaining() - .end_cons() - .verify_end(); + const OID ecc_param_id = decode_gost_key_parameters(alg_id); auto group = check_domain(EC_Group::from_OID(ecc_param_id)); + check_gost_key_oid_matches_group(alg_id.oid(), group); std::vector bits; BER_Decoder(key_bits, BER_Decoder::Limits::DER()).decode(bits, ASN1_Type::OctetString).verify_end(); @@ -111,6 +182,9 @@ GOST_3410_PrivateKey::GOST_3410_PrivateKey(RandomNumberGenerator& rng, EC_Group GOST_3410_PrivateKey::GOST_3410_PrivateKey(RandomNumberGenerator& rng, const EC_Group& domain, const BigInt& x) : EC_PrivateKey(rng, check_domain(domain), x) {} +GOST_3410_PrivateKey::GOST_3410_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : + EC_PrivateKey(gost_private_key_alg_id(alg_id), key_bits) {} + std::unique_ptr GOST_3410_PrivateKey::public_key() const { return std::make_unique(domain(), _public_ec_point()); } diff --git a/src/lib/pubkey/gost_3410/gost_3410.h b/src/lib/pubkey/gost_3410/gost_3410.h index 27a6aa93c32..27389331062 100644 --- a/src/lib/pubkey/gost_3410/gost_3410.h +++ b/src/lib/pubkey/gost_3410/gost_3410.h @@ -87,8 +87,7 @@ class BOTAN_PUBLIC_API(2, 0) GOST_3410_PrivateKey final : public GOST_3410_Publi * @param alg_id the X.509 algorithm identifier * @param key_bits ECPrivateKey bits */ - GOST_3410_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PrivateKey(alg_id, key_bits) {} + GOST_3410_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits); /** * Create a private key from a given secret @p x @@ -116,7 +115,7 @@ class BOTAN_PUBLIC_API(2, 0) GOST_3410_PrivateKey final : public GOST_3410_Publi std::unique_ptr public_key() const override; AlgorithmIdentifier pkcs8_algorithm_identifier() const override { - return EC_PublicKey::algorithm_identifier(); // NOLINT(bugprone-parent-virtual-call) + return GOST_3410_PublicKey::algorithm_identifier(); // NOLINT(bugprone-parent-virtual-call) } std::unique_ptr create_signature_op(RandomNumberGenerator& rng, diff --git a/src/lib/pubkey/hss_lms/hss_lms.cpp b/src/lib/pubkey/hss_lms/hss_lms.cpp index 36f325c3a7d..34394b499f4 100644 --- a/src/lib/pubkey/hss_lms/hss_lms.cpp +++ b/src/lib/pubkey/hss_lms/hss_lms.cpp @@ -111,7 +111,7 @@ std::unique_ptr HSS_LMS_PublicKey::create_verification_op( std::unique_ptr HSS_LMS_PublicKey::create_x509_verification_op( const AlgorithmIdentifier& signature_algorithm, std::string_view provider) const { if(provider.empty() || provider == "base") { - if(signature_algorithm != this->algorithm_identifier()) { + if(signature_algorithm.oid() != this->object_identifier() || !signature_algorithm.parameters_are_empty()) { throw Decoding_Error("Unexpected AlgorithmIdentifier for HSS-LMS signature"); } return std::make_unique(m_public); diff --git a/src/lib/pubkey/pk_algs.cpp b/src/lib/pubkey/pk_algs.cpp index 4e8a4e13a3a..ae26ebc0628 100644 --- a/src/lib/pubkey/pk_algs.cpp +++ b/src/lib/pubkey/pk_algs.cpp @@ -131,10 +131,7 @@ std::unique_ptr load_public_key(const AlgorithmIdentifier& alg_id, [[maybe_unused]] std::span key_bits) { const std::string alg_name = [&]() -> std::string { if(const auto name = alg_id.oid().registered_name()) { - const std::vector alg_info = split_on(*name, '/'); - if(!alg_info.empty()) { - return alg_info[0]; - } + return *name; } throw Decoding_Error( @@ -298,10 +295,7 @@ std::unique_ptr load_private_key(const AlgorithmIdentifier& alg_id, [[maybe_unused]] std::span key_bits) { const std::string alg_name = [&]() -> std::string { if(const auto name = alg_id.oid().registered_name()) { - const std::vector alg_info = split_on(*name, '/'); - if(!alg_info.empty()) { - return alg_info[0]; - } + return *name; } throw Decoding_Error( diff --git a/src/lib/pubkey/rsa/rsa.cpp b/src/lib/pubkey/rsa/rsa.cpp index 9fddac6b231..d5ef7ef152b 100644 --- a/src/lib/pubkey/rsa/rsa.cpp +++ b/src/lib/pubkey/rsa/rsa.cpp @@ -166,9 +166,12 @@ void RSA_PublicKey::init(BigInt&& n, BigInt&& e) { RSA_PublicKey::RSA_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) { // RFC 4055 Section 1.2 has that parameters MUST be NULL, but historical // reasons make that difficult to enforce, so absent is also accepted. - // - // This only checks rsaEncryption; PSS/OAEP key identifiers have their own parameter encoding - if(alg_id.oid().registered_name() == "RSA" && !alg_id.parameters_are_null_or_empty()) { + if(alg_id.oid().registered_name() != "RSA") { + throw Decoding_Error( + fmt("Unexpected AlgorithmIdentifier OID {} in association with RSA public key", alg_id.oid())); + } + + if(!alg_id.parameters_are_null_or_empty()) { throw Decoding_Error("Unexpected parameters for RSA public key"); } @@ -280,7 +283,12 @@ void RSA_PrivateKey::init(BigInt&& d, BigInt&& p, BigInt&& q, BigInt&& d1, BigIn } RSA_PrivateKey::RSA_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) { - if(alg_id.oid().registered_name() == "RSA" && !alg_id.parameters_are_null_or_empty()) { + if(alg_id.oid().registered_name() != "RSA") { + throw Decoding_Error( + fmt("Unexpected AlgorithmIdentifier OID {} in association with RSA private key", alg_id.oid())); + } + + if(!alg_id.parameters_are_null_or_empty()) { throw Decoding_Error("Unexpected parameters for RSA private key"); } @@ -682,7 +690,7 @@ AlgorithmIdentifier RSA_Signature_Operation::algorithm_identifier() const { try { const std::string full_name = "RSA/" + padding_name; const OID oid = OID::from_string(full_name); - return AlgorithmIdentifier(oid, AlgorithmIdentifier::USE_EMPTY_PARAM); + return AlgorithmIdentifier(oid, AlgorithmIdentifier::USE_NULL_PARAM); } catch(Lookup_Error&) {} if(padding_name.starts_with("PSS(")) { diff --git a/src/lib/pubkey/sm2/sm2.cpp b/src/lib/pubkey/sm2/sm2.cpp index d0b007633af..6e288651d8d 100644 --- a/src/lib/pubkey/sm2/sm2.cpp +++ b/src/lib/pubkey/sm2/sm2.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -20,6 +21,21 @@ std::string SM2_PublicKey::algo_name() const { return "SM2"; } +namespace { + +const AlgorithmIdentifier& assert_sm2_algorithm_identifier(const AlgorithmIdentifier& alg_id) { + if(alg_id.oid() != OID::from_string("SM2") && alg_id.oid() != OID::from_string("SM2_Enc")) { + throw Decoding_Error(fmt("Unexpected AlgorithmIdentifier OID {} in association with SM2 key", alg_id.oid())); + } + + return alg_id; +} + +} // namespace + +SM2_PublicKey::SM2_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : + EC_PublicKey(assert_sm2_algorithm_identifier(alg_id), key_bits) {} + std::optional SM2_PublicKey::_signature_element_size_for_DER_encoding() const { return domain().get_order_bytes(); } @@ -50,7 +66,7 @@ bool SM2_PrivateKey::check_key(RandomNumberGenerator& rng, bool strong) const { } SM2_PrivateKey::SM2_PrivateKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PrivateKey(alg_id, key_bits), + EC_PrivateKey(assert_sm2_algorithm_identifier(alg_id), key_bits), m_da_inv((this->_private_key() + EC_Scalar::one(domain())).invert()), m_da_inv_legacy(m_da_inv.to_bigint()) {} diff --git a/src/lib/pubkey/sm2/sm2.h b/src/lib/pubkey/sm2/sm2.h index 827bc3668e9..6a16c1b98bc 100644 --- a/src/lib/pubkey/sm2/sm2.h +++ b/src/lib/pubkey/sm2/sm2.h @@ -40,8 +40,7 @@ class BOTAN_PUBLIC_API(2, 2) SM2_PublicKey : public virtual EC_PublicKey { * @param alg_id the X.509 algorithm identifier * @param key_bits DER encoded public key bits */ - SM2_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits) : - EC_PublicKey(alg_id, key_bits) {} + SM2_PublicKey(const AlgorithmIdentifier& alg_id, std::span key_bits); /** * Get this keys algorithm name. diff --git a/src/lib/pubkey/sphincsplus/sphincsplus_common/sphincsplus.cpp b/src/lib/pubkey/sphincsplus/sphincsplus_common/sphincsplus.cpp index c4bd4a57564..7cc58b159d0 100644 --- a/src/lib/pubkey/sphincsplus/sphincsplus_common/sphincsplus.cpp +++ b/src/lib/pubkey/sphincsplus/sphincsplus_common/sphincsplus.cpp @@ -265,7 +265,7 @@ std::unique_ptr SphincsPlus_PublicKey::create_verification std::unique_ptr SphincsPlus_PublicKey::create_x509_verification_op( const AlgorithmIdentifier& signature_algorithm, std::string_view provider) const { if(provider.empty() || provider == "base") { - if(signature_algorithm != this->algorithm_identifier()) { + if(signature_algorithm.oid() != this->object_identifier() || !signature_algorithm.parameters_are_empty()) { throw Decoding_Error("Unexpected AlgorithmIdentifier for SLH-DSA (or SPHINCS+) signature"); } return std::make_unique(m_public); diff --git a/src/lib/pubkey/xmss/xmss_publickey.cpp b/src/lib/pubkey/xmss/xmss_publickey.cpp index 2c750f4fe16..9d6ea111fbf 100644 --- a/src/lib/pubkey/xmss/xmss_publickey.cpp +++ b/src/lib/pubkey/xmss/xmss_publickey.cpp @@ -175,7 +175,7 @@ std::unique_ptr XMSS_PublicKey::create_verification_op(std std::unique_ptr XMSS_PublicKey::create_x509_verification_op(const AlgorithmIdentifier& alg_id, std::string_view provider) const { if(provider == "base" || provider.empty()) { - if(alg_id != this->algorithm_identifier()) { + if(alg_id.oid() != this->object_identifier() || !alg_id.parameters_are_empty()) { throw Decoding_Error("Unexpected AlgorithmIdentifier for XMSS X509 signature"); } return std::make_unique(*this); diff --git a/src/lib/x509/x509_crl.cpp b/src/lib/x509/x509_crl.cpp index 37f45c230dd..d1726fb59d8 100644 --- a/src/lib/x509/x509_crl.cpp +++ b/src/lib/x509/x509_crl.cpp @@ -139,7 +139,7 @@ std::unique_ptr decode_crl_body(const std::vector& body, cons AlgorithmIdentifier sig_algo_inner; tbs_crl.decode(sig_algo_inner); - if(sig_algo != sig_algo_inner) { + if(sig_algo.oid() != sig_algo_inner.oid() || sig_algo.parameters() != sig_algo_inner.parameters()) { throw Decoding_Error("Algorithm identifier mismatch in CRL"); } diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp index f59c3ab6f55..6df772abc76 100644 --- a/src/lib/x509/x509cert.cpp +++ b/src/lib/x509/x509cert.cpp @@ -136,7 +136,8 @@ std::unique_ptr parse_x509_cert_body(const X509_Object& o if(data->m_version > 2) { throw Decoding_Error("Unknown X.509 cert version " + std::to_string(data->m_version)); } - if(obj.signature_algorithm() != data->m_sig_algo_inner) { + if(obj.signature_algorithm().oid() != data->m_sig_algo_inner.oid() || + obj.signature_algorithm().parameters() != data->m_sig_algo_inner.parameters()) { throw Decoding_Error("X.509 Certificate had differing algorithm identifiers in inner and outer ID fields"); } diff --git a/src/tests/test_asn1.cpp b/src/tests/test_asn1.cpp index 4181cb4759f..fb6b958d9de 100644 --- a/src/tests/test_asn1.cpp +++ b/src/tests/test_asn1.cpp @@ -480,12 +480,12 @@ Test::Result test_pss_params_rejects_trailing_data_in_mgf1_params() { const Botan::AlgorithmIdentifier sha256_alg_id("SHA-256", Botan::AlgorithmIdentifier::USE_NULL_PARAM); const auto sha256_der = sha256_alg_id.BER_encode(); - auto encode_pss_params = [&](const std::vector& mgf_params) { - const Botan::AlgorithmIdentifier mgf("MGF1", mgf_params); + auto encode_pss_params = [](const Botan::AlgorithmIdentifier& hash, const Botan::AlgorithmIdentifier& mgf_hash) { + const Botan::AlgorithmIdentifier mgf("MGF1", mgf_hash.BER_encode()); Botan::DER_Encoder enc; enc.start_sequence() .start_context_specific(0) - .encode(sha256_alg_id) + .encode(hash) .end_cons() .start_context_specific(1) .encode(mgf) @@ -498,7 +498,7 @@ Test::Result test_pss_params_rejects_trailing_data_in_mgf1_params() { }; try { - const auto clean_der = encode_pss_params(sha256_der); + const auto clean_der = encode_pss_params(sha256_alg_id, sha256_alg_id); const Botan::PSS_Params clean(clean_der); result.test_success("control: clean PSS-Params decodes"); } catch(const std::exception& e) { @@ -508,11 +508,37 @@ Test::Result test_pss_params_rejects_trailing_data_in_mgf1_params() { std::vector mgf_params_with_junk = sha256_der; const std::vector trailing_junk{0x02, 0x01, 0x00}; mgf_params_with_junk.insert(mgf_params_with_junk.end(), trailing_junk.begin(), trailing_junk.end()); - const auto bad_der = encode_pss_params(mgf_params_with_junk); + const Botan::AlgorithmIdentifier mgf_with_trailing_junk("MGF1", mgf_params_with_junk); + + Botan::DER_Encoder bad_mgf_params_enc; + bad_mgf_params_enc.start_sequence() + .start_context_specific(0) + .encode(sha256_alg_id) + .end_cons() + .start_context_specific(1) + .encode(mgf_with_trailing_junk) + .end_cons() + .start_context_specific(2) + .encode(static_cast(32)) + .end_cons() + .end_cons(); + const auto bad_der = bad_mgf_params_enc.get_contents(); result.test_throws("PSS-Params rejects trailing data in MGF1 parameters", [&]() { const Botan::PSS_Params bad(bad_der); }); + const Botan::AlgorithmIdentifier sha256_with_params("SHA-256", std::vector{0x04, 0x00}); + + result.test_throws("PSS-Params rejects hash AlgorithmIdentifier parameters", [&]() { + const auto bad_hash_der = encode_pss_params(sha256_with_params, sha256_alg_id); + const Botan::PSS_Params bad(bad_hash_der); + }); + + result.test_throws("PSS-Params rejects MGF1 hash AlgorithmIdentifier parameters", [&]() { + const auto bad_mgf_hash_der = encode_pss_params(sha256_alg_id, sha256_with_params); + const Botan::PSS_Params bad(bad_mgf_hash_der); + }); + return result; } diff --git a/src/tests/test_x509_path.cpp b/src/tests/test_x509_path.cpp index d4544ea2877..1fe44fda9d6 100644 --- a/src/tests/test_x509_path.cpp +++ b/src/tests/test_x509_path.cpp @@ -10,6 +10,7 @@ #if defined(BOTAN_HAS_X509_CERTIFICATES) #include "test_arb_eq.h" #include + #include #include #include #include @@ -344,6 +345,18 @@ BOTAN_REGISTER_TEST("x509", "x509_path_extended", Extended_Path_Validation_Tests #if defined(BOTAN_HAS_PSS) +bool spki_uses_rsa_pss(std::span spki) { + Botan::AlgorithmIdentifier key_alg_id; + Botan::BER_Decoder(spki, Botan::BER_Decoder::Limits::DER()) + .start_sequence() + .decode(key_alg_id) + .discard_remaining() + .end_cons() + .verify_end(); + + return key_alg_id.oid() == Botan::OID::from_string("RSA/PSS"); +} + class PSS_Path_Validation_Tests : public Test { public: std::vector run() override; @@ -412,14 +425,24 @@ std::vector PSS_Path_Validation_Tests::run() { result.test_str_eq(test_name + " path validation result", validation_result.result_string(), expected_result); } else if(end && !root) { // CRT self signed test - auto pubkey = end->subject_public_key(); - const bool accept = expected_result == "Verified"; - result.test_bool_eq(test_name + " verify signature", end->check_signature(*pubkey), accept); + if(end->subject_public_key_algo().oid() == Botan::OID::from_string("RSA/PSS")) { + result.test_throws(test_name + " rejects RSA-PSS SubjectPublicKeyInfo", + [&]() { auto pubkey = end->subject_public_key(); }); + } else { + auto pubkey = end->subject_public_key(); + const bool accept = expected_result == "Verified"; + result.test_bool_eq(test_name + " verify signature", end->check_signature(*pubkey), accept); + } } else if(csr) { // PKCS#10 Request test - auto pubkey = csr->subject_public_key(); - const bool accept = expected_result == "Verified"; - result.test_bool_eq(test_name + " verify signature", csr->check_signature(*pubkey), accept); + if(spki_uses_rsa_pss(csr->raw_public_key())) { + result.test_throws(test_name + " rejects RSA-PSS SubjectPublicKeyInfo", + [&]() { auto pubkey = csr->subject_public_key(); }); + } else { + auto pubkey = csr->subject_public_key(); + const bool accept = expected_result == "Verified"; + result.test_bool_eq(test_name + " verify signature", csr->check_signature(*pubkey), accept); + } } result.end_timer(); diff --git a/src/tests/unit_ecdsa.cpp b/src/tests/unit_ecdsa.cpp index 0083537b3da..56272e325ad 100644 --- a/src/tests/unit_ecdsa.cpp +++ b/src/tests/unit_ecdsa.cpp @@ -48,7 +48,11 @@ Test::Result test_decode_ecdsa_X509() { "3B:6C:99:1C:D6:5A:51:FC:EB:17:E3:AA:F6:3C:1A:DA:14:1F:82:41:30:6F:64:EE:FF:63:F3:1F:D6:07:14:9F"); auto pubkey = cert.subject_public_key(); - result.test_is_true("verify self-signed signature", cert.check_signature(*pubkey)); + if(cert.signature_algorithm().parameters_are_null()) { + result.test_is_false("reject NULL ECDSA signature parameters", cert.check_signature(*pubkey)); + } else { + result.test_is_true("verify self-signed signature", cert.check_signature(*pubkey)); + } } catch(Botan::Exception& e) { result.test_failure(e.what()); } @@ -66,7 +70,11 @@ Test::Result test_decode_ver_link_SHA256() { const Botan::X509_Certificate link_cert(Test::data_file("x509/ecc/link_SHA256.cer")); auto pubkey = root_cert.subject_public_key(); - result.test_is_true("verified self-signed signature", link_cert.check_signature(*pubkey)); + if(link_cert.signature_algorithm().parameters_are_null()) { + result.test_is_false("reject NULL ECDSA signature parameters", link_cert.check_signature(*pubkey)); + } else { + result.test_is_true("verified self-signed signature", link_cert.check_signature(*pubkey)); + } } catch(Botan::Exception& e) { result.test_failure(e.what()); } @@ -92,7 +100,11 @@ Test::Result test_decode_ver_link_SHA1() { !link_cert.check_signature(*pubkey)); return result; } - result.test_is_true("verified self-signed signature", link_cert.check_signature(*pubkey)); + if(link_cert.signature_algorithm().parameters_are_null()) { + result.test_is_false("reject NULL ECDSA signature parameters", link_cert.check_signature(*pubkey)); + } else { + result.test_is_true("verified self-signed signature", link_cert.check_signature(*pubkey)); + } } catch(Botan::Exception& e) { result.test_failure(e.what()); } diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp index 3c620cbb37d..a1ade1b3c89 100644 --- a/src/tests/unit_x509.cpp +++ b/src/tests/unit_x509.cpp @@ -619,8 +619,8 @@ Test::Result test_rsa_oaep() { #if defined(BOTAN_HAS_RSA) const Botan::X509_Certificate cert(Test::data_file("x509/misc/rsa_oaep.pem")); - auto public_key = cert.subject_public_key(); - result.test_not_null("Decoding RSA-OAEP worked", public_key.get()); + result.test_throws("RSA-OAEP SubjectPublicKeyInfo is not decoded as generic RSA", + [&]() { auto public_key = cert.subject_public_key(); }); const auto& pk_info = cert.subject_public_key_algo(); result.test_str_eq("RSA-OAEP OID", pk_info.oid().to_string(), Botan::OID::from_string("RSA/OAEP").to_string()); @@ -927,6 +927,8 @@ Test::Result test_padding_config() { test_result.test_opt_str_eq("CA certificate signature algorithm (default)", ca_cert_def.signature_algorithm().oid().registered_name(), "RSA/PKCS1v15(SHA-512)"); + test_result.test_is_true("CA certificate PKCS1v15 signature parameters are NULL", + ca_cert_def.signature_algorithm().parameters_are_null()); // Create X509 CA certificate; RSA-PSS is explicitly set opt.set_padding_scheme("PSSR"); @@ -980,6 +982,8 @@ Test::Result test_padding_config() { test_result.test_opt_str_eq("End certificate signature algorithm", end_cert_pkcs1.signature_algorithm().oid().registered_name(), "RSA/PKCS1v15(SHA-512)"); + test_result.test_is_true("End certificate PKCS1v15 signature parameters are NULL", + end_cert_pkcs1.signature_algorithm().parameters_are_null()); // Create X509 CA object: its signer will use the explicitly configured padding scheme, which is different from the CA certificate's scheme const Botan::X509_CA ca_diff(ca_cert_def, (*sk), "SHA-512", "PSS", *rng); @@ -1008,7 +1012,6 @@ Test::Result test_padding_config() { return test_result; } #endif - #endif Test::Result test_pkcs10_ext(const Botan::Private_Key& key,