fix pqc signature hash binding

rnpgp · Nov 4, 2024 · 3fe4fb5 · 3fe4fb5
1 parent d47072e
commit 3fe4fb5
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 28 deletions.
diff --git 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 <cassert>
 
 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);
+    }
 }
diff --git a/src/lib/crypto/dilithium.h b/src/lib/crypto/dilithium.h
@@ -109,8 +109,8 @@ class pgp_dilithium_public_key_t {
 std::pair<pgp_dilithium_public_key_t, pgp_dilithium_private_key_t> 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
@@ -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);
     }
 }
diff --git 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
@@ -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
@@ -2016,7 +2016,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
@@ -2150,7 +2156,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
@@ -529,6 +529,7 @@ class DilithiumEccKeyMaterial : public KeyMaterial {
                         pgp_signature_material_t &         sig,
                         const rnp::secure_vector<uint8_t> &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
@@ -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;