Applied some CodeRabbitAI suggestions

Author: antoinelochet

.github/workflows/ci.yml

@@ -85,7 +85,8 @@ jobs:
         env:
           OPENSSL_VERSION: 3.5.2
           OPENSSL_INSTALL_DIR: /usr/local/openssl-3.5
-          LDFLAGS: "-Wl,-R/usr/local/openssl-3.5/lib64 -L/usr/local/openssl-3.5/lib64"
+          LDFLAGS: "-Wl,-rpath,/usr/local/openssl-3.5/lib64 -L/usr/local/openssl-3.5/lib64"
+          PKG_CONFIG_PATH: "/usr/local/openssl-3.5/lib64/pkgconfig"
         run: |
           sudo apt-get update -qq
           sudo apt-get install -y libcppunit-dev p11-kit build-essential checkinstall zlib1g-dev sudo autoconf libtool git
@@ -103,7 +104,8 @@ jobs:
           # Once all OpenSSL deprecations fixed, uncomment this
           #   CXXFLAGS: -Werror
           OPENSSL_INSTALL_DIR: /usr/local/openssl-3.5
-          LDFLAGS: "-Wl,-R/usr/local/openssl-3.5/lib64 -L/usr/local/openssl-3.5/lib64"
+          LDFLAGS: "-Wl,-rpath,/usr/local/openssl-3.5/lib64 -L/usr/local/openssl-3.5/lib64"
+          PKG_CONFIG_PATH: "/usr/local/openssl-3.5/lib64/pkgconfig"
         run: |
           ./autogen.sh
           ./configure --with-crypto-backend=openssl --with-openssl=${{ env.OPENSSL_INSTALL_DIR }}

CMakeLists.txt

@@ -8,7 +8,7 @@ option(DISABLE_VISIBILITY "Disables and unsets -fvisibility=hidden" OFF)
 option(ENABLE_64bit "Enable 64-bit compiling" OFF)
 option(ENABLE_ECC "Enable support for ECC" ON)
 option(ENABLE_EDDSA "Enable support for EDDSA" ON)
-option(ENABLE_MLDSA "Enable support for ML-DSA" ON)
+option(ENABLE_MLDSA "Enable support for ML-DSA" OFF)
 option(ENABLE_GOST "Enable support for GOST" OFF)
 option(ENABLE_FIPS "Enable support for FIPS 140-2 mode" OFF)
 option(ENABLE_P11_KIT "Enable p11-kit integration" ON)

m4/acx_botan_mldsa.m4

@@ -8,26 +8,21 @@ AC_DEFUN([ACX_BOTAN_MLDSA],[
 	LIBS="$CRYPTO_LIBS $LIBS"
 
 	AC_LANG_PUSH([C++])
-	AC_CACHE_VAL([acx_cv_lib_botan_mldsa_support],[
-		acx_cv_lib_botan_mldsa_support=no
-		AC_RUN_IFELSE([
-			AC_LANG_SOURCE([[
-				#include <botan/version.h>
-				int main()
-				{
-					// TODO
-					return 1;
-				}
-			]])
-		],[
-			AC_MSG_RESULT([yes])
+	AC_CACHE_VAL([acx_cv_lib_botan_mldsa_support], [
+		AC_COMPILE_IFELSE([
+			AC_LANG_SOURCE([
+			#include <botan/version.h>
+			#ifndef BOTAN_HAS_ML_DSA
+			# error "no ML-DSA support"
+			#endif
+			int main(void){ return 0; }
+			])
+		], [
 			acx_cv_lib_botan_mldsa_support=yes
-		],[
-			AC_MSG_RESULT([no])
-			acx_cv_lib_botan_mldsa_support=no
-		],[
-			AC_MSG_WARN([Cannot test, assuming no ML-DSA])
+			AC_MSG_RESULT([yes])
+		], [
 			acx_cv_lib_botan_mldsa_support=no
+			AC_MSG_RESULT([no])
 		])
 	])
 	AC_LANG_POP([C++])

src/lib/P11Attributes.cpp

@@ -2234,7 +2234,7 @@ bool P11AttrEcPoint::setDefault()
 // Set default value
 bool P11AttrParameterSet::setDefault()
 {
-	OSAttribute attr(ByteString(""));
+	OSAttribute attr((unsigned long)0);
 	return osobject->setAttribute(type, attr);
 }
 
@@ -2571,7 +2571,6 @@ bool P11AttrSeed::setDefault()
 CK_RV P11AttrSeed::updateAttr(Token *token, bool isPrivate, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int /*op*/)
 {
 	ByteString plaintext((unsigned char*)pValue, ulValueLen);
-	DEBUG_MSG("P11AttrSeed plaintext: %s", plaintext.hex_str().c_str());
 	ByteString value;
 
 	// Encrypt

src/lib/SoftHSM.cpp

@@ -10038,35 +10038,37 @@ CK_RV SoftHSM::generateMLDSA
 		return CKR_GENERAL_ERROR;
 
 	// Extract desired key information
-	unsigned long* params = 0;
+	CK_ULONG paramSet = 0;
 	for (CK_ULONG i = 0; i < ulPublicKeyAttributeCount; i++)
 	{
 		switch (pPublicKeyTemplate[i].type)
 		{
 			case CKA_PARAMETER_SET:
-				params = (unsigned long*)pPublicKeyTemplate[i].pValue;
+				if (pPublicKeyTemplate[i].ulValueLen != sizeof(CK_ULONG)) {
+                    INFO_MSG("CKA_PARAMETER_SET must be sizeof(CK_ULONG)");
+                    return CKR_ATTRIBUTE_VALUE_INVALID;
+                }
+                paramSet = *(CK_ULONG*)pPublicKeyTemplate[i].pValue;
 				break;
 			default:
 				break;
 		}
 	}
 
 	// The parameters must be specified to be able to generate a key pair.
-	if (params == 0) {
+	if (paramSet == 0) {
 		INFO_MSG("Missing parameter(s) in pPublicKeyTemplate");
 		return CKR_TEMPLATE_INCOMPLETE;
 	}
 
-	if (*params != 1UL && *params != 2UL && *params != 3UL) {
-		INFO_MSG("Wrong parameterSet: %ld", *params);
+	if (paramSet != 1UL && paramSet != 2UL && paramSet != 3UL) {
+		INFO_MSG("Wrong parameterSet: %ld", paramSet);
 		return CKR_PARAMETER_SET_NOT_SUPPORTED;
 	}
 
 	// Set the parameters
 	MLDSAParameters p;
-	p.setParameterSet(*params);
-
-	DEBUG_MSG("params=%d, p.parameterSet=%d", *params, p.getParameterSet());
+	p.setParameterSet(paramSet);
 
 	// Generate key pair
 	AsymmetricKeyPair* kp = NULL;
@@ -10132,8 +10134,6 @@ CK_RV SoftHSM::generateMLDSA
 				CK_ULONG ulKeyGenMechanism = (CK_ULONG)CKM_ML_DSA_KEY_PAIR_GEN;
 				bOK = bOK && osobject->setAttribute(CKA_KEY_GEN_MECHANISM,ulKeyGenMechanism);
 
-				DEBUG_MSG("pub->getParameterSet()=%d, pub->getValue()=%s", pub->getParameterSet(), pub->getValue().hex_str().c_str());
-
 				// ML-DSA Public Key Attributes
 				ByteString value;
 				if (isPublicKeyPrivate)
@@ -10215,7 +10215,6 @@ CK_RV SoftHSM::generateMLDSA
 				ByteString parameterSet;
 				ByteString value;
 				ByteString seed;
-				DEBUG_MSG("priv->getParameterSet()=%d, priv->getSeed()=%s, priv->getValue()=%s", priv->getParameterSet(), priv->getSeed().hex_str().c_str(), priv->getValue().hex_str().c_str());
 				if (isPrivateKeyPrivate)
 				{
 					token->encrypt(priv->getValue(), value);
@@ -10227,8 +10226,6 @@ CK_RV SoftHSM::generateMLDSA
 					seed = priv->getSeed();
 				}
 
-				DEBUG_MSG("parameterSet=%d, seed=%s, value=%s", priv->getParameterSet(), seed.hex_str().c_str(), value.hex_str().c_str());
-
 				bOK = bOK && osobject->setAttribute(CKA_PARAMETER_SET, priv->getParameterSet());
 				bOK = bOK && osobject->setAttribute(CKA_VALUE, value);
 				bOK = bOK && osobject->setAttribute(CKA_SEED, seed);

src/lib/SoftHSM.h

@@ -52,8 +52,10 @@
 #include "DHPrivateKey.h"
 #include "GOSTPublicKey.h"
 #include "GOSTPrivateKey.h"
+#ifdef WITH_ML_DSA
 #include "MLDSAPublicKey.h"
 #include "MLDSAPrivateKey.h"
+#endif
 
 #include <memory>
 
@@ -373,6 +375,7 @@ class SoftHSM
 		CK_BBOOL isOnToken,
 		CK_BBOOL isPrivate
 	);
+#ifdef WITH_ML_DSA
 	CK_RV generateMLDSA
 	(
 		CK_SESSION_HANDLE hSession,
@@ -387,6 +390,7 @@ class SoftHSM
 		CK_BBOOL isPrivateKeyOnToken,
 		CK_BBOOL isPrivateKeyPrivate
 	);
+#endif
 #ifdef WITH_ECC
 	CK_RV deriveECDH
 	(
@@ -451,8 +455,10 @@ class SoftHSM
 	CK_RV getGOSTPrivateKey(GOSTPrivateKey* privateKey, Token* token, OSObject* key);
 	CK_RV getGOSTPublicKey(GOSTPublicKey* publicKey, Token* token, OSObject* key);
 	CK_RV getSymmetricKey(SymmetricKey* skey, Token* token, OSObject* key);
+#ifdef WITH_ML_DSA
 	CK_RV getMLDSAPrivateKey(MLDSAPrivateKey* privateKey, Token* token, OSObject* key);
 	CK_RV getMLDSAPublicKey(MLDSAPublicKey* publicKey, Token* token, OSObject* key);
+#endif
 
 	ByteString getECDHPubData(ByteString& pubData);
 
@@ -462,7 +468,9 @@ class SoftHSM
 	bool setECPrivateKey(OSObject* key, const ByteString &ber, Token* token, bool isPrivate) const;
 	bool setEDPrivateKey(OSObject* key, const ByteString &ber, Token* token, bool isPrivate) const;
 	bool setGOSTPrivateKey(OSObject* key, const ByteString &ber, Token* token, bool isPrivate) const;
+#ifdef WITH_ML_DSA
 	bool setMLDSAPrivateKey(OSObject* key, const ByteString &ber, Token* token, bool isPrivate) const;
+#endif
 
 
 	CK_RV WrapKeyAsym

src/lib/crypto/MLDSAPublicKey.h

@@ -9,6 +9,7 @@
 
 #include "config.h"
 #include "PublicKey.h"
+#include "ByteString.h"
 
 class MLDSAPublicKey : public PublicKey
 {

src/lib/crypto/OSSLMLDSA.cpp

@@ -31,6 +31,12 @@ bool OSSLMLDSA::sign(PrivateKey *privateKey, const ByteString &dataToSign,
 		return false;
 	}
 
+	if (privateKey == NULL)
+    {
+        ERROR_MSG("No private key supplied");
+        return false;
+    }
+
 	// Check if the private key is the right type
 	if (!privateKey->isOfType(OSSLMLDSAPrivateKey::type))
 	{
@@ -68,6 +74,12 @@ bool OSSLMLDSA::sign(PrivateKey *privateKey, const ByteString &dataToSign,
 	memset(&signature[0], 0, len);
 
 	EVP_MD_CTX *ctx = EVP_MD_CTX_new();
+	if (ctx == NULL)
+    {
+        ERROR_MSG("ML-DSA sign ctx alloc failed");
+        return false;
+    }
+
 	if (!EVP_DigestSignInit(ctx, NULL, NULL, NULL, pkey))
 	{
 		ERROR_MSG("ML-DSA sign init failed (0x%08X)", ERR_get_error());
@@ -117,6 +129,12 @@ bool OSSLMLDSA::verify(PublicKey *publicKey, const ByteString &originalData,
 		return false;
 	}
 
+	if (publicKey == NULL)
+    {
+        ERROR_MSG("No public key supplied");
+        return false;
+    }
+
 	// Check if the private key is the right type
 	if (!publicKey->isOfType(OSSLMLDSAPublicKey::type))
 	{
@@ -159,6 +177,14 @@ bool OSSLMLDSA::verify(PublicKey *publicKey, const ByteString &originalData,
 
 	unsigned long parameterSet = pk->getParameterSet();
 	const char* name = OSSL::mldsaParameterSet2Name(parameterSet);
+	
+	if (name == NULL) 
+	{
+        ERROR_MSG("Unknown ML-DSA parameter set (%lu)", parameterSet);
+        EVP_PKEY_CTX_free(vctx);
+        return false;
+    }
+
 	sig_alg = EVP_SIGNATURE_fetch(NULL, name, NULL);
 	if (sig_alg == NULL) {
 		ERROR_MSG("ML-DSA EVP_SIGNATURE_fetch failed (0x%08X)", ERR_get_error());
@@ -176,12 +202,20 @@ bool OSSLMLDSA::verify(PublicKey *publicKey, const ByteString &originalData,
 	int verifyRV = EVP_PKEY_verify(vctx, signature.const_byte_str(), signature.size(),
                                             originalData.const_byte_str(), originalData.size());
 
-	if (verifyRV != 1) {
-		ERROR_MSG("ML-DSA verify failed (0x%08X)", verifyRV);
-		EVP_PKEY_CTX_free(vctx);
-		EVP_SIGNATURE_free(sig_alg);
-		return false;
-	}
+	if (verifyRV != 1) 
+	{
+        if (verifyRV == 0) 
+		{
+            ERROR_MSG("ML-DSA signature invalid");
+        } else 
+		{
+            ERROR_MSG("ML-DSA verify error (0x%08X)", ERR_get_error());
+        }
+        EVP_PKEY_CTX_free(vctx);
+        EVP_SIGNATURE_free(sig_alg);
+        return false;
+    }
+
     EVP_PKEY_CTX_free(vctx);
 	EVP_SIGNATURE_free(sig_alg);
 	return true;
@@ -265,6 +299,12 @@ bool OSSLMLDSA::generateKeyPair(AsymmetricKeyPair **ppKeyPair, AsymmetricParamet
 	unsigned long parameterSet = params->getParameterSet();
 	const char* name = OSSL::mldsaParameterSet2Name(parameterSet);
 
+	if (name == NULL) 
+	{
+        ERROR_MSG("Unknown ML-DSA parameter set (%lu)", parameterSet);
+        return false;
+    }
+
 	EVP_PKEY_CTX *ctx = NULL;
 	EVP_PKEY *pkey = NULL;
 	ctx = EVP_PKEY_CTX_new_from_name(NULL, name, NULL);
@@ -288,8 +328,11 @@ bool OSSLMLDSA::generateKeyPair(AsymmetricKeyPair **ppKeyPair, AsymmetricParamet
 	// Create an asymmetric key-pair object to return
 	OSSLMLDSAKeyPair *kp = new OSSLMLDSAKeyPair();
 
-	((OSSLMLDSAPrivateKey *)kp->getPrivateKey())->setFromOSSL(pkey);
-	((OSSLMLDSAPublicKey *)kp->getPublicKey())->setFromOSSL(pkey);
+	// bump refcount for each wrapper
+	EVP_PKEY_up_ref(pkey);
+	((OSSLMLDSAPrivateKey*)kp->getPrivateKey())->setFromOSSL(pkey);
+	EVP_PKEY_up_ref(pkey);
+	((OSSLMLDSAPublicKey*) kp->getPublicKey())->setFromOSSL(pkey);
 
 	*ppKeyPair = kp;
 

src/lib/crypto/OSSLMLDSA.h

@@ -51,5 +51,5 @@ class OSSLMLDSA : public AsymmetricAlgorithm
 private:
 };
 
-#endif // !_SOFTHSM_V2_OSSLEDDSA_H
+#endif // !_SOFTHSM_V2_OSSLMLDSA_H
 

src/lib/crypto/OSSLMLDSAKeyPair.cpp

@@ -12,13 +12,16 @@
 // Set the public key
 void OSSLMLDSAKeyPair::setPublicKey(OSSLMLDSAPublicKey& publicKey)
 {
-	pubKey = publicKey;
+	// Copy only the public material; avoid sharing OpenSSL handles
+	pubKey.setValue(publicKey.getValue());
 }
 
 // Set the private key
 void OSSLMLDSAKeyPair::setPrivateKey(OSSLMLDSAPrivateKey& privateKey)
 {
-	privKey = privateKey;
+	// Copy only the raw material; avoid sharing OpenSSL handles
+	privKey.setSeed(privateKey.getSeed());
+	privKey.setValue(privateKey.getValue());
 }
 
 // Return the public key