From 49e01590d040772080ccba583c77d6b2b9495243 Mon Sep 17 00:00:00 2001 From: Johannes Roth Date: Fri, 1 Nov 2024 11:42:14 +0100 Subject: [PATCH] fix pqc signature hash binding --- src/lib/crypto/dilithium.cpp | 44 +++++++++++++++++++++++++++------- src/lib/crypto/dilithium.h | 4 ++-- src/lib/crypto/sphincsplus.cpp | 12 ++-------- src/lib/crypto/sphincsplus.h | 2 -- src/lib/generate-key.cpp | 2 +- src/lib/key_material.cpp | 10 ++++++-- src/lib/key_material.hpp | 1 + src/rnpkeys/tui.cpp | 6 ++--- 8 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/lib/crypto/dilithium.cpp b/src/lib/crypto/dilithium.cpp index 1561b24ef6..acb44b85fa 100644 --- a/src/lib/crypto/dilithium.cpp +++ b/src/lib/crypto/dilithium.cpp @@ -25,6 +25,8 @@ */ #include "dilithium.h" +#include "logging.h" +#include "types.h" #include namespace { @@ -119,19 +121,45 @@ pgp_dilithium_private_key_t::is_valid(rnp::RNG *rng) const } bool -dilithium_hash_allowed(pgp_hash_alg_t hash_alg) +dilithium_hash_allowed(pgp_pubkey_alg_t pk_alg, pgp_hash_alg_t hash_alg) { - switch (hash_alg) { - case PGP_HASH_SHA3_256: - case PGP_HASH_SHA3_512: - return true; + switch (pk_alg) { + case PGP_PKA_DILITHIUM3_ED25519: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM3_P256: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM3_BP256: + return hash_alg == PGP_HASH_SHA3_256; + case PGP_PKA_DILITHIUM5_ED448: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM5_P384: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM5_BP384: + return hash_alg == PGP_HASH_SHA3_512; default: - return false; + RNP_LOG("invalid algorithm ID given"); + throw rnp::rnp_exception(RNP_ERROR_BAD_STATE); } } pgp_hash_alg_t -dilithium_default_hash_alg() +dilithium_default_hash_alg(pgp_pubkey_alg_t pk_alg) { - return PGP_HASH_SHA3_256; + switch (pk_alg) { + case PGP_PKA_DILITHIUM3_ED25519: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM3_P256: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM3_BP256: + return PGP_HASH_SHA3_256; + case PGP_PKA_DILITHIUM5_ED448: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM5_P384: + FALLTHROUGH_STATEMENT; + case PGP_PKA_DILITHIUM5_BP384: + return PGP_HASH_SHA3_512; + default: + RNP_LOG("invalid algorithm ID given"); + throw rnp::rnp_exception(RNP_ERROR_BAD_STATE); + } } \ No newline at end of file diff --git a/src/lib/crypto/dilithium.h b/src/lib/crypto/dilithium.h index 18035ce0c1..43ddec016e 100644 --- a/src/lib/crypto/dilithium.h +++ b/src/lib/crypto/dilithium.h @@ -109,8 +109,8 @@ class pgp_dilithium_public_key_t { std::pair dilithium_generate_keypair( rnp::RNG *rng, dilithium_parameter_e dilithium_param); -bool dilithium_hash_allowed(pgp_hash_alg_t hash_alg); +bool dilithium_hash_allowed(pgp_pubkey_alg_t pk_alg, pgp_hash_alg_t hash_alg); -pgp_hash_alg_t dilithium_default_hash_alg(); +pgp_hash_alg_t dilithium_default_hash_alg(pgp_pubkey_alg_t pk_alg); #endif diff --git a/src/lib/crypto/sphincsplus.cpp b/src/lib/crypto/sphincsplus.cpp index 3f673ff849..bd24b6f41f 100644 --- a/src/lib/crypto/sphincsplus.cpp +++ b/src/lib/crypto/sphincsplus.cpp @@ -148,14 +148,6 @@ pgp_sphincsplus_generate(rnp::RNG *rng, pgp_sphincsplus_key_t *material, pgp_pub return RNP_SUCCESS; } -bool -pgp_sphincsplus_public_key_t::validate_signature_hash_requirements( - pgp_hash_alg_t hash_alg) const -{ - /* check if key is allowed with the hash algorithm */ - return sphincsplus_hash_allowed(pk_alg_, hash_alg); -} - bool pgp_sphincsplus_public_key_t::is_valid(rnp::RNG *rng) const { @@ -244,7 +236,7 @@ sphincsplus_hash_allowed(pgp_pubkey_alg_t pk_alg, pgp_hash_alg_t hash_alg) return hash_alg == PGP_HASH_SHA3_512; default: RNP_LOG("invalid algorithm ID given"); - throw rnp::rnp_exception(RNP_ERROR_BAD_PARAMETERS); + throw rnp::rnp_exception(RNP_ERROR_BAD_STATE); } } @@ -260,6 +252,6 @@ sphincsplus_default_hash_alg(pgp_pubkey_alg_t alg) return PGP_HASH_SHA3_512; default: RNP_LOG("invalid algorithm ID given"); - throw rnp::rnp_exception(RNP_ERROR_BAD_PARAMETERS); + throw rnp::rnp_exception(RNP_ERROR_BAD_STATE); } } \ No newline at end of file diff --git a/src/lib/crypto/sphincsplus.h b/src/lib/crypto/sphincsplus.h index 81bbfad996..614662fbcc 100644 --- a/src/lib/crypto/sphincsplus.h +++ b/src/lib/crypto/sphincsplus.h @@ -105,8 +105,6 @@ class pgp_sphincsplus_public_key_t { bool is_valid(rnp::RNG *rng) const; - bool validate_signature_hash_requirements(pgp_hash_alg_t hash_alg) const; - pgp_pubkey_alg_t alg() const { diff --git a/src/lib/generate-key.cpp b/src/lib/generate-key.cpp index 95379ac4f0..4ea88cef3a 100644 --- a/src/lib/generate-key.cpp +++ b/src/lib/generate-key.cpp @@ -232,7 +232,7 @@ pgp_check_key_hash_requirements(const rnp_keygen_crypto_params_t &crypto) case PGP_PKA_DILITHIUM3_BP256: FALLTHROUGH_STATEMENT; case PGP_PKA_DILITHIUM5_BP384: - if (!dilithium_hash_allowed(crypto.hash_alg)) { + if (!dilithium_hash_allowed(crypto.key_alg, crypto.hash_alg)) { return false; } break; diff --git a/src/lib/key_material.cpp b/src/lib/key_material.cpp index e68396dfec..d8957a290d 100644 --- a/src/lib/key_material.cpp +++ b/src/lib/key_material.cpp @@ -2131,7 +2131,13 @@ DilithiumEccKeyMaterial::sign(rnp::SecurityContext & ctx, pgp_hash_alg_t DilithiumEccKeyMaterial::adjust_hash(pgp_hash_alg_t hash) const { - return dilithium_default_hash_alg(); + return dilithium_default_hash_alg(alg()); +} + +bool +DilithiumEccKeyMaterial::sig_hash_allowed(pgp_hash_alg_t hash) const +{ + return dilithium_hash_allowed(alg(), hash); } size_t @@ -2266,7 +2272,7 @@ SlhdsaKeyMaterial::adjust_hash(pgp_hash_alg_t hash) const bool SlhdsaKeyMaterial::sig_hash_allowed(pgp_hash_alg_t hash) const { - return key_.pub.validate_signature_hash_requirements(hash); + return sphincsplus_hash_allowed(alg(), hash); } size_t diff --git a/src/lib/key_material.hpp b/src/lib/key_material.hpp index 4f61149b27..39356a43a3 100644 --- a/src/lib/key_material.hpp +++ b/src/lib/key_material.hpp @@ -658,6 +658,7 @@ class DilithiumEccKeyMaterial : public KeyMaterial { pgp_signature_material_t & sig, const rnp::secure_vector &hash) const override; pgp_hash_alg_t adjust_hash(pgp_hash_alg_t hash) const override; + bool sig_hash_allowed(pgp_hash_alg_t hash) const override; size_t bits() const noexcept override; const pgp_dilithium_exdsa_composite_public_key_t & pub() const noexcept; diff --git a/src/rnpkeys/tui.cpp b/src/rnpkeys/tui.cpp index 1b24f1b9f8..52d68bbddb 100644 --- a/src/rnpkeys/tui.cpp +++ b/src/rnpkeys/tui.cpp @@ -348,7 +348,7 @@ rnpkeys_ask_generate_params(rnp_cfg &cfg, FILE *input_fp) break; case 26: cfg.set_str(CFG_KG_PRIMARY_ALG, RNP_ALGNAME_DILITHIUM5_ED448); - cfg.set_str(CFG_KG_HASH, RNP_ALGNAME_SHA3_256); + cfg.set_str(CFG_KG_HASH, RNP_ALGNAME_SHA3_512); cfg.set_str(CFG_KG_SUBKEY_ALG, RNP_ALGNAME_KYBER1024_X448); cfg.set_str(CFG_KG_V6_KEY, "true"); break; @@ -360,7 +360,7 @@ rnpkeys_ask_generate_params(rnp_cfg &cfg, FILE *input_fp) break; case 28: cfg.set_str(CFG_KG_PRIMARY_ALG, RNP_ALGNAME_DILITHIUM5_P384); - cfg.set_str(CFG_KG_HASH, RNP_ALGNAME_SHA3_256); + cfg.set_str(CFG_KG_HASH, RNP_ALGNAME_SHA3_512); cfg.set_str(CFG_KG_SUBKEY_ALG, RNP_ALGNAME_KYBER1024_P384); cfg.set_str(CFG_KG_V6_KEY, "true"); break; @@ -372,7 +372,7 @@ rnpkeys_ask_generate_params(rnp_cfg &cfg, FILE *input_fp) break; case 30: cfg.set_str(CFG_KG_PRIMARY_ALG, RNP_ALGNAME_DILITHIUM5_BP384); - cfg.set_str(CFG_KG_HASH, RNP_ALGNAME_SHA3_256); + cfg.set_str(CFG_KG_HASH, RNP_ALGNAME_SHA3_512); cfg.set_str(CFG_KG_SUBKEY_ALG, RNP_ALGNAME_KYBER1024_BP384); cfg.set_str(CFG_KG_V6_KEY, "true"); break;