address review comments

CMakeLists.txt

@@ -80,9 +80,15 @@ tristate_feature_auto(ENABLE_CAST5 "Enable CAST5 cipher support.")
 tristate_feature_auto(ENABLE_RIPEMD160 "Enable RIPEMD-160 hash support.")
 
 option(ENABLE_CRYPTO_REFRESH "Enable crypto-refresh support (v6)")
-option(ENABLE_PQC "Enable PQC support - requires ENABLE_CRYPTO_REFRESH")
-option(ENABLE_PQC_MLKEM_IPD "Use ML-KEM-ipd instead of Kyber")
-option(ENABLE_PQC_DBG_LOG "If enabled, logs intermediate values of computations. Used for debugging, do not enable for productive use.")
+option(ENABLE_PQC "Enable PQC support")
+
+# Note: The following two flags are only temporary and will be removed once POC is in a stable state
+if (DEFINED ENABLE_PQC_MLKEM_IPD)
+  add_definitions(-DENABLE_PQC_MLKEM_IPD)
+endif()
+if (DEFINED ENABLE_PQC_DBG_LOG)
+  add_definitions(-DENABLE_PQC_DBG_LOG)
+endif()
 
 set(ENABLE_DOC Auto CACHE STRING "Enable building documentation.")
 set_property(CACHE ENABLE_DOC PROPERTY STRINGS ${TRISTATE_VALUES})

include/rnp/rnp.h

@@ -1948,6 +1948,7 @@ RNP_API rnp_result_t rnp_key_get_default_key(rnp_key_handle_t  primary_key,
  */
 RNP_API rnp_result_t rnp_key_get_alg(rnp_key_handle_t key, char **alg);
 
+#if defined(RNP_EXPERIMENTAL_PQC)
 /** Get a SPHINCS+ key's parameter string
  *
  * @param key key handle
@@ -1958,6 +1959,7 @@ RNP_API rnp_result_t rnp_key_get_alg(rnp_key_handle_t key, char **alg);
  * time.
  */
 RNP_API rnp_result_t rnp_key_sphincsplus_get_param(rnp_key_handle_t handle, char **param);
+#endif
 
 /** Get number of bits in the key. For EC-based keys it will return size of the curve.
  *

src/lib/config.h.in

@@ -63,8 +63,6 @@
 #cmakedefine ENABLE_IDEA
 #cmakedefine ENABLE_CRYPTO_REFRESH
 #cmakedefine ENABLE_PQC
-#cmakedefine ENABLE_PQC_MLKEM_IPD
-#cmakedefine ENABLE_PQC_DBG_LOG
 #cmakedefine ENABLE_BLOWFISH
 #cmakedefine ENABLE_CAST5
 #cmakedefine ENABLE_RIPEMD160

src/lib/logging.h

@@ -124,4 +124,4 @@ class LogStop {
     } while (0)
 #endif
 
-#endif
+#endif

src/lib/rnp.cpp

@@ -6793,10 +6793,10 @@ try {
 }
 FFI_GUARD
 
+#if defined(RNP_EXPERIMENTAL_PQC)
 rnp_result_t
 rnp_key_sphincsplus_get_param(rnp_key_handle_t handle, char **param)
 try {
-#if defined(ENABLE_PQC)
     if (!handle || !param) {
         return RNP_ERROR_NULL_POINTER;
     }
@@ -6807,11 +6807,9 @@ try {
 
     return get_map_value(
       sphincsplus_params_map, key->material().sphincsplus.pub.param(), param);
-#else
-    return RNP_ERROR_NOT_IMPLEMENTED;
-#endif
 }
 FFI_GUARD
+#endif
 
 rnp_result_t
 rnp_key_get_bits(rnp_key_handle_t handle, uint32_t *bits)

src/lib/types.h

@@ -294,7 +294,6 @@ typedef struct pgp_sig_subpkt_t {
         } preferred; /* 5.2.3.7.  Preferred Symmetric Algorithms */
                      /* 5.2.3.8.  Preferred Hash Algorithms */
                      /* 5.2.3.9.  Preferred Compression Algorithms */
-                     /* Crypto Refresh 5.2.3.15. Preferred AEAD Ciphersuites  */
         struct {
             uint8_t          revclass;
             pgp_pubkey_alg_t pkalg;

src/librepgp/stream-parse.cpp

@@ -1519,10 +1519,15 @@ do_enforce_aes_v3pkesk(pgp_pubkey_alg_t alg)
     switch (alg) {
 #if defined(ENABLE_PQC)
     case PGP_PKA_KYBER768_X25519:
+        FALLTHROUGH_STATEMENT;
     case PGP_PKA_KYBER768_P256:
+        FALLTHROUGH_STATEMENT;
     case PGP_PKA_KYBER1024_P384:
+        FALLTHROUGH_STATEMENT;
     case PGP_PKA_KYBER768_BP256:
+        FALLTHROUGH_STATEMENT;
     case PGP_PKA_KYBER1024_BP384:
+        FALLTHROUGH_STATEMENT;
 #endif
 #if defined(ENABLE_CRYPTO_REFRESH)
     case PGP_PKA_X25519:

src/rnp/fficli.cpp

@@ -420,7 +420,7 @@ ffi_pass_callback_stdin(rnp_ffi_t        ffi,
         }
 
         rnp->reuse_password_for_subkey--;
-        if (rnp->reuse_password_for_subkey == 0) {
+        if (!rnp->reuse_password_for_subkey) {
             rnp_buffer_clear(rnp->reused_password, strnlen(rnp->reused_password, buf_len));
             free(rnp->reused_password);
             rnp->reused_password = NULL;

src/rnp/rnpcfg.h

@@ -115,22 +115,28 @@
 #define CFG_KG_SUBKEY_BITS "kg-subkey-bits"
 #define CFG_KG_SUBKEY_CURVE "kg-subkey-curve"
 #define CFG_KG_SUBKEY_EXPIRATION "kg-subkey-expiration"
+#if defined(ENABLE_PQC)
 #define CFG_KG_SUBKEY_2_ALG "kg-subkey-2-alg"
 #define CFG_KG_SUBKEY_2_BITS "kg-subkey-2-bits"
 #define CFG_KG_SUBKEY_2_CURVE "kg-subkey-2-curve"
 #define CFG_KG_SUBKEY_2_EXPIRATION "kg-subkey-2-expiration"
+#endif
 #define CFG_KG_HASH "kg-hash"
 #define CFG_KG_PROT_HASH "kg-prot-hash"
 #define CFG_KG_PROT_ALG "kg-prot-alg"
 #define CFG_KG_PROT_ITERATIONS "kg-prot-iterations"
+#if defined(ENABLE_CRYPTO_REFRESH)
 #define CFG_KG_V6_KEY \
     "kg-v6-key" /* represents a boolean property: non-empty string means 'true' */
+#endif
+#if defined(ENABLE_PQC)
 #define CFG_KG_PRIMARY_SPHINCSPLUS_PARAM \
     "kg-primary-sphincsplus-param" /* 128f, 128s, 192f, 192s, 256f, 256s */
 #define CFG_KG_SUBKEY_SPHINCSPLUS_PARAM \
     "kg-subkey-sphincsplus-param" /* 128f, 128s, 192f, 192s, 256f, 256s */
 #define CFG_KG_SUBKEY_2_SPHINCSPLUS_PARAM \
     "kg-subkey-2-sphincsplus-param" /* 128f, 128s, 192f, 192s, 256f, 256s */
+#endif
 
 /* rnp CLI config : contains all the system-dependent and specified by the user configuration
  * options */

src/rnpkeys/tui.cpp

@@ -32,6 +32,7 @@
 #endif
 #include <errno.h>
 #include <iterator>
+#include "config.h"
 #include "rnp/rnpcfg.h"
 #include "rnpkeys.h"
 #include "defaults.h"

src/tests/ffi-enc.cpp

@@ -762,9 +762,10 @@ TEST_F(rnp_tests, test_ffi_decrypt_v6_pkesk_test_vector)
 // The final implementation of the PQC draft implementation will use the final NIST standard.
 TEST_F(rnp_tests, test_ffi_decrypt_pqc_pkesk_test_vector)
 {
+    bool expect_success = true;
 #if !(defined(BOTAN_HAS_ML_KEM_INITIAL_PUBLIC_DRAFT) && defined(ENABLE_PQC_MLKEM_IPD))
     // we can only verify the test vectors with ML-KEM-ipd
-    GTEST_SKIP();
+    expect_success = false;
 #endif
 
     rnp_ffi_t    ffi = NULL;
@@ -779,29 +780,45 @@ TEST_F(rnp_tests, test_ffi_decrypt_pqc_pkesk_test_vector)
     assert_rnp_success(
       rnp_input_from_path(&input, "data/draft-ietf-openpgp-pqc/v6-seipdv2.asc"));
     assert_non_null(input);
-    assert_rnp_success(rnp_decrypt(ffi, input, output));
-    assert_string_equal(file_to_str("decrypted").c_str(), "Testing\n");
+    if (expect_success) {
+        assert_rnp_success(rnp_decrypt(ffi, input, output));
+        assert_string_equal(file_to_str("decrypted").c_str(), "Testing\n");
+    } else {
+        assert_rnp_failure(rnp_decrypt(ffi, input, output));
+    }
     assert_int_equal(unlink("decrypted"), 0);
+    rnp_input_destroy(input);
+    rnp_output_destroy(output);
 
     assert_rnp_success(rnp_output_to_path(&output, "decrypted"));
     assert_rnp_success(
       rnp_input_from_path(&input, "data/draft-ietf-openpgp-pqc/v4-seipdv1.asc"));
     assert_non_null(input);
-    assert_rnp_success(rnp_decrypt(ffi, input, output));
-    assert_string_equal(file_to_str("decrypted").c_str(), "Testing\n");
+    if (expect_success) {
+        assert_rnp_success(rnp_decrypt(ffi, input, output));
+        assert_string_equal(file_to_str("decrypted").c_str(), "Testing\n");
+    } else {
+        assert_rnp_failure(rnp_decrypt(ffi, input, output));
+    }
     assert_int_equal(unlink("decrypted"), 0);
+    rnp_input_destroy(input);
+    rnp_output_destroy(output);
 
     assert_rnp_success(rnp_output_to_path(&output, "decrypted"));
     assert_rnp_success(
       rnp_input_from_path(&input, "data/draft-ietf-openpgp-pqc/v4-seipdv1.asc"));
     assert_non_null(input);
-    assert_rnp_success(rnp_decrypt(ffi, input, output));
-    assert_string_equal(file_to_str("decrypted").c_str(), "Testing\n");
+    if (expect_success) {
+        assert_rnp_success(rnp_decrypt(ffi, input, output));
+        assert_string_equal(file_to_str("decrypted").c_str(), "Testing\n");
+    } else {
+        assert_rnp_failure(rnp_decrypt(ffi, input, output));
+    }
     assert_int_equal(unlink("decrypted"), 0);
-
-    // cleanup
     rnp_input_destroy(input);
     rnp_output_destroy(output);
+
+    rnp_ffi_destroy(ffi);
 }
 #endif
 

src/tests/ffi-key.cpp

@@ -3236,11 +3236,6 @@ TEST_F(rnp_tests, test_ffi_v6_cert_import)
 // The final implementation of the PQC draft implementation will use the final NIST standard.
 TEST_F(rnp_tests, test_ffi_pqc_certs)
 {
-#if !(defined(BOTAN_HAS_ML_KEM_INITIAL_PUBLIC_DRAFT) && defined(ENABLE_PQC_MLKEM_IPD))
-    // we can only verify the test vectors with ML-KEM-ipd
-    GTEST_SKIP();
-#endif
-
     rnp_ffi_t   ffi = NULL;
     rnp_input_t input = NULL;
     size_t      keycount = 255;
@@ -3259,6 +3254,7 @@ TEST_F(rnp_tests, test_ffi_pqc_certs)
     assert_int_equal(keycount, 2);
     assert_rnp_success(rnp_get_secret_key_count(ffi, &keycount));
     assert_int_equal(keycount, 0);
+    rnp_ffi_destroy(ffi);
 
     /* Private Key */
     assert_rnp_success(rnp_ffi_create(&ffi, "GPG", "GPG"));
@@ -3272,6 +3268,7 @@ TEST_F(rnp_tests, test_ffi_pqc_certs)
     rnp_input_destroy(input);
     assert_rnp_success(rnp_get_secret_key_count(ffi, &keycount));
     assert_int_equal(keycount, 2);
+    rnp_ffi_destroy(ffi);
 }
 
 #endif