From 29c7d9889850a2f325d9aa152fe320d8bbfd646a Mon Sep 17 00:00:00 2001
From: Albert Astals Cid <aacid@kde.org>
Date: Tue, 21 Jan 2020 01:12:07 +0100
Subject: [PATCH] Fix MACContext clone in botan/gcrypt/nss

And excercise that from the unittest
---
 plugins/qca-botan/qca-botan.cpp      | 50 +++++++++++++++++++---------
 plugins/qca-gcrypt/qca-gcrypt.cpp    |  2 +-
 plugins/qca-nss/qca-nss.cpp          |  6 ++--
 unittest/macunittest/macunittest.cpp | 21 ++++++++++++
 4 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/plugins/qca-botan/qca-botan.cpp b/plugins/qca-botan/qca-botan.cpp
index 099a331c..405b86b4 100644
--- a/plugins/qca-botan/qca-botan.cpp
+++ b/plugins/qca-botan/qca-botan.cpp
@@ -126,13 +126,31 @@ private:
     Botan::HashFunction *m_hashObj;
 };
 
+static QString qcaHmacToBotanHmac(const QString &type)
+{
+    if ( type == "hmac(md5)" )
+	return QString("MD5");
+    else if ( type == "hmac(sha1)" )
+	return QString("SHA-1");
+    else if ( type == "hmac(sha256)" )
+	return QString("SHA-256");
+    else if ( type == "hmac(sha384)" )
+	return QString("SHA-384");
+    else if ( type == "hmac(sha512)" )
+	return QString("SHA-512");
+    else if ( type == "hmac(ripemd160)" )
+	return QString("RIPEMD-160");
+
+    return {};
+}
 
 //-----------------------------------------------------------
 class BotanHMACContext : public QCA::MACContext
 {
 public:
-    BotanHMACContext( const QString &hashName, QCA::Provider *p, const QString &type) : QCA::MACContext(p, type)
+    BotanHMACContext(QCA::Provider *p, const QString &type) : QCA::MACContext(p, type)
     {
+	const QString hashName = qcaHmacToBotanHmac(type);
 	m_hashObj = new Botan::HMAC(Botan::HashFunction::create_or_throw(hashName.toStdString()).release());
 	if (0 == m_hashObj) {
 	    std::cout << "null context object" << std::endl;
@@ -141,6 +159,7 @@ public:
 
     ~BotanHMACContext()
     {
+	delete m_hashObj;
     }
 
     void setup(const QCA::SymmetricKey &key) override
@@ -155,7 +174,7 @@ public:
 
     Context *clone() const override
     {
-	return new BotanHMACContext(*this);
+	return new BotanHMACContext(provider(), type());
     }
 
     void clear() 
@@ -628,11 +647,10 @@ public:
 	return supported;
     }
 
-    QStringList features() const override
+    const QStringList &hmacTypes() const
     {
 	static QStringList list;
 	if (list.isEmpty()) {
-	    list += "random";
 	    list += "hmac(md5)";
 	    list += "hmac(sha1)";
 	    // HMAC with SHA2 doesn't appear to work correctly in Botan.
@@ -640,6 +658,16 @@ public:
 	    // list += "hmac(sha384)";
 	    // list += "hmac(sha512)";
 	    list += "hmac(ripemd160)";
+	}
+	return list;
+    }
+
+    QStringList features() const override
+    {
+	static QStringList list;
+	if (list.isEmpty()) {
+	    list += "random";
+	    list += hmacTypes();
 	    list += pbkdfTypes();
 	    list += "hkdf(sha256)";
 	    list += cipherTypes();
@@ -654,18 +682,8 @@ public:
 	    return new botanRandomContext( this );
 	else if ( hashTypes().contains(type) )
 	    return new BotanHashContext( this, type );
-	else if ( type == "hmac(md5)" )
-	    return new BotanHMACContext( QString("MD5"), this, type );
-	else if ( type == "hmac(sha1)" )
-	    return new BotanHMACContext( QString("SHA-1"), this, type );
-	else if ( type == "hmac(sha256)" )
-	    return new BotanHMACContext( QString("SHA-256"), this, type );
-	else if ( type == "hmac(sha384)" )
-	    return new BotanHMACContext( QString("SHA-384"), this, type );
-	else if ( type == "hmac(sha512)" )
-	    return new BotanHMACContext( QString("SHA-512"), this, type );
-	else if ( type == "hmac(ripemd160)" )
-	    return new BotanHMACContext( QString("RIPEMD-160"), this, type );
+	else if ( hmacTypes().contains(type) )
+	    return new BotanHMACContext( this, type );
 	else if ( pbkdfTypes().contains(type) )
 	    return new BotanPBKDFContext( qcaPbkdfToBotanPbkdf(type), this, type );
 	else if ( type == "hkdf(sha256)" )
diff --git a/plugins/qca-gcrypt/qca-gcrypt.cpp b/plugins/qca-gcrypt/qca-gcrypt.cpp
index cab93b82..57507ce6 100644
--- a/plugins/qca-gcrypt/qca-gcrypt.cpp
+++ b/plugins/qca-gcrypt/qca-gcrypt.cpp
@@ -117,7 +117,7 @@ public:
 
     Context *clone() const override
     {
-        return new gcryHMACContext(*this);
+        return new gcryHMACContext(m_hashAlgorithm, provider(), type());
     }
 
     void clear()
diff --git a/plugins/qca-nss/qca-nss.cpp b/plugins/qca-nss/qca-nss.cpp
index 3e5236e3..6d7191a1 100644
--- a/plugins/qca-nss/qca-nss.cpp
+++ b/plugins/qca-nss/qca-nss.cpp
@@ -144,6 +144,7 @@ public:
     {
 	NSS_NoDB_Init(".");
 
+	m_context = 0;
 	m_status = 0;
 
 	/* Get a slot to use for the crypto operations */
@@ -181,14 +182,15 @@ public:
 
     ~nssHmacContext()
     {
-	PK11_DestroyContext(m_context, PR_TRUE);
+	if (m_context)
+	    PK11_DestroyContext(m_context, PR_TRUE);
 	if (m_slot)
 	    PK11_FreeSlot(m_slot);
     }
 
     Context *clone() const override
     {
-	return new nssHmacContext(*this);
+	return new nssHmacContext(provider(), type());
     }
 
     void clear()
diff --git a/unittest/macunittest/macunittest.cpp b/unittest/macunittest/macunittest.cpp
index e4f1ef5b..8f5f24e7 100644
--- a/unittest/macunittest/macunittest.cpp
+++ b/unittest/macunittest/macunittest.cpp
@@ -77,6 +77,9 @@ void MACUnitTest::HMACMD5()
 	    QCOMPARE( md5hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( md5hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = md5hmacLenTest;
+	    copy.context(); // detach
+
 	    // These tests are from RFC2202, Section 2.
 	    // The first three are also in the Appendix to RFC2104
 	    QCA::MessageAuthenticationCode md5hmac1( "hmac(md5)", QCA::SymmetricKey(), provider );
@@ -154,6 +157,9 @@ void MACUnitTest::HMACSHA256()
 	    QCOMPARE( hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = hmacLenTest;
+	    copy.context(); // detach
+
 	    QCA::MessageAuthenticationCode hmac1( "hmac(sha256)", QCA::SymmetricKey(), provider );
 	    QCA::SymmetricKey key1( QCA::SecureArray( "Jefe" ) );
 	    hmac1.setup( key1 );
@@ -235,6 +241,9 @@ void MACUnitTest::HMACSHA224()
 	    QCOMPARE( hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = hmacLenTest;
+	    copy.context(); // detach
+
 	    QCA::MessageAuthenticationCode hmac1( "hmac(sha224)", QCA::SymmetricKey(), provider );
 	    QCA::SymmetricKey key1( QCA::SecureArray( "Jefe" ) );
 	    hmac1.setup( key1 );
@@ -317,6 +326,9 @@ void MACUnitTest::HMACSHA384()
 	    QCOMPARE( hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = hmacLenTest;
+	    copy.context(); // detach
+
 	    QCA::MessageAuthenticationCode hmac1( "hmac(sha384)", QCA::SymmetricKey(), provider );
 	    QCA::SymmetricKey key1( QCA::SecureArray( "Jefe" ) );
 	    hmac1.setup( key1 );
@@ -399,6 +411,9 @@ void MACUnitTest::HMACSHA512()
 	    QCOMPARE( hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = hmacLenTest;
+	    copy.context(); // detach
+
 	    QCA::MessageAuthenticationCode hmac1( "hmac(sha512)", QCA::SymmetricKey(), provider );
 	    QCA::SymmetricKey key1( QCA::SecureArray( "Jefe" ) );
 	    hmac1.setup( key1 );
@@ -481,6 +496,9 @@ void MACUnitTest::HMACSHA1()
 	    QCOMPARE( sha1hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( sha1hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = sha1hmacLenTest;
+	    copy.context(); // detach
+
 	    // These tests are from RFC2202, Section 3.
 	    QCA::MessageAuthenticationCode test1(  "hmac(sha1)", QCA::SecureArray() );
 	    QCA::SymmetricKey key1( QCA::hexToArray( "0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b" ) );
@@ -556,6 +574,9 @@ void MACUnitTest::HMACRMD160()
 	    QCOMPARE( ripemd160hmacLenTest.validKeyLength( 848888 ), true );
 	    QCOMPARE( ripemd160hmacLenTest.validKeyLength( -2 ), false );
 
+	    QCA::MessageAuthenticationCode copy = ripemd160hmacLenTest;
+	    copy.context(); // detach
+
 	    // These tests are from RFC2286, Section 2.
 	    QCA::MessageAuthenticationCode test1( "hmac(ripemd160)", QCA::SymmetricKey(), provider );
 	    QCA::SymmetricKey key1 ( QCA::hexToArray( "0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b" ) );