From: Krzysztof Jackiewicz <k.jackiewicz@samsung.com>
Date: Mon, 22 May 2023 19:29:17 +0000 (+0200)
Subject: Fix GCM IV length setting
X-Git-Tag: accepted/tizen/7.0/unified/20230620.164235~1^2~17
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1e6e268703fc313a973f06581e21cf62c112e903;p=platform%2Fcore%2Fsecurity%2Fkey-manager.git

Fix GCM IV length setting

IV length was not properly set in case of GCM. The default 12B IV was
used at all times.

GCM supports 1 to 2^64-1 byte long IVs. Reflect it in the SW backend
implementation, its tests and client API description.

Change-Id: Idfa18c1c3cfd06de6618b4ac7632133ace6ab8dc
---

diff --git a/src/include/ckmc/ckmc-type.h b/src/include/ckmc/ckmc-type.h
index 54c1f067..192598bb 100644
--- a/src/include/ckmc/ckmc-type.h
+++ b/src/include/ckmc/ckmc-type.h
@@ -293,7 +293,7 @@ typedef struct __ckmc_pkcs12 {
 typedef enum __ckmc_param_name {
 	CKMC_PARAM_ALGO_TYPE = 1, /**< integer - type of algorithm (see #ckmc_algo_type_e) */
 
-	CKMC_PARAM_ED_IV = 101, /**< 16B buffer (up to 2^64-1 bytes long in case of AES GCM) */
+	CKMC_PARAM_ED_IV = 101, /**< buffer - initialization vector */
 	CKMC_PARAM_ED_CTR_LEN, /**< integer - ctr length in bits*/
 	CKMC_PARAM_ED_AAD, /**< buffer - Additional Authentication Data for AES GCM */
 	CKMC_PARAM_ED_TAG_LEN, /**< integer - tag length in bits */
@@ -418,7 +418,8 @@ typedef enum __ckmc_algo_type {
 	CKMC_ALGO_AES_GCM, /**< AES-GCM algorithm
                        Supported parameters:
                        - #CKMC_PARAM_ALGO_TYPE = #CKMC_ALGO_AES_GCM (mandatory),
-                       - #CKMC_PARAM_ED_IV = initialization vector (mandatory)
+                       - #CKMC_PARAM_ED_IV = 1 to (2^64-1) bytes long initialization vector.
+                       Recommended length is 12B (mandatory)
                        - #CKMC_PARAM_ED_TAG_LEN = GCM tag length in bits. One of
                        {32, 64, 96, 104, 112, 120, 128} (optional, if not present, the
                        length 128 is used; since Tizen 5.0, if TrustZone backend is used,
diff --git a/src/manager/crypto/generic-backend/algo-validation.h b/src/manager/crypto/generic-backend/algo-validation.h
index dc149669..8946a35c 100644
--- a/src/manager/crypto/generic-backend/algo-validation.h
+++ b/src/manager/crypto/generic-backend/algo-validation.h
@@ -98,6 +98,21 @@ struct Type {
 	};
 };
 
+// Validates as true if parameter value is equal or greater than Min
+template <typename T, T Min>
+struct GreaterOrEqual {
+public:
+	static bool Check(const T &value)
+	{
+		return value >= Min;
+	}
+
+	static void Why(std::ostringstream &os)
+	{
+		os << "is smaller than " << static_cast<int>(Min);
+	}
+};
+
 template <typename T>
 struct Unsupported {
 	static bool Check(const T &)
diff --git a/src/manager/crypto/sw-backend/crypto.h b/src/manager/crypto/sw-backend/crypto.h
index 08ba272b..a8a522eb 100644
--- a/src/manager/crypto/sw-backend/crypto.h
+++ b/src/manager/crypto/sw-backend/crypto.h
@@ -80,12 +80,25 @@ public:
 			ThrowErr(Exc::Crypto::InternalError, "Wrong key size! Expected: ",
 					EVP_CIPHER_key_length(type), " Get: ", key.size());
 
-		if (static_cast<int>(iv.size()) < EVP_CIPHER_iv_length(type))
-			ThrowErr(Exc::Crypto::InternalError, "Wrong iv size! Expected: ",
-					 EVP_CIPHER_iv_length(type), " Get: ", iv.size());
+		bool gcm  = (EVP_CIPHER_mode(type) == EVP_CIPH_GCM_MODE);
+		int iv_len = EVP_CIPHER_iv_length(type);
 
-		OPENSSL_ERROR_HANDLE(EVP_CipherInit_ex(m_ctx, type, NULL, key.data(), iv.data(),
-		                               encryption ? 1 : 0));
+		OPENSSL_ERROR_HANDLE(EVP_CipherInit_ex(m_ctx, type, NULL, NULL, NULL, encryption ? 1 : 0));
+
+		if (gcm) {
+			if (iv.empty())
+				ThrowErr(Exc::Crypto::InternalError, "Empty iv provided!");
+
+			OPENSSL_ERROR_HANDLE(
+				EVP_CIPHER_CTX_ctrl(m_ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), NULL));
+		} else {
+			if (static_cast<int>(iv.size()) != iv_len)
+				ThrowErr(Exc::Crypto::InternalError, "Wrong iv size! Expected: ", iv_len, " Got: ",
+				         iv.size());
+		}
+
+		OPENSSL_ERROR_HANDLE(
+			EVP_CipherInit_ex(m_ctx, NULL, NULL, key.data(), iv.data(), encryption ? 1 : 0));
 
 		EVP_CIPHER_CTX_set_padding(m_ctx, 1);
 	}
diff --git a/src/manager/crypto/sw-backend/internals.cpp b/src/manager/crypto/sw-backend/internals.cpp
index 26a5f33a..b6aecae6 100644
--- a/src/manager/crypto/sw-backend/internals.cpp
+++ b/src/manager/crypto/sw-backend/internals.cpp
@@ -116,7 +116,8 @@ typedef ParamCheck<ParamName::ED_CTR_LEN,
 typedef ParamCheck<ParamName::ED_IV,
 		RawBuffer,
 		true,
-		DefaultValidator<RawBuffer>> GcmIvCheck;
+		GreaterOrEqual<size_t, 1>,
+		BufferSizeGetter> GcmIvCheck;
 
 typedef ParamCheck<ParamName::ED_TAG_LEN,
 		int,
diff --git a/unit-tests/test_crypto-logic.cpp b/unit-tests/test_crypto-logic.cpp
index 5c168a0b..62fdfd68 100644
--- a/unit-tests/test_crypto-logic.cpp
+++ b/unit-tests/test_crypto-logic.cpp
@@ -185,11 +185,6 @@ NEGATIVE_TEST_CASE(row_encryption)
 	BOOST_REQUIRE_NO_THROW(logic.removeKey(TEST_CLIENT));
 	BOOST_REQUIRE_NO_THROW(logic.pushKey(TEST_CLIENT, TEST_KEY));
 
-	// short IV
-	row.iv = RawBuffer(4);
-	BOOST_REQUIRE_THROW(logic.encryptRow(row), Exc::InternalError);
-	row.iv.clear();
-
 	// correct encryption
 	DB::Row encryptedRow;
 	BOOST_REQUIRE_NO_THROW(encryptedRow = logic.encryptRow(row));
diff --git a/unit-tests/test_sw-backend.cpp b/unit-tests/test_sw-backend.cpp
index 7c6a7608..9fd02642 100644
--- a/unit-tests/test_sw-backend.cpp
+++ b/unit-tests/test_sw-backend.cpp
@@ -528,6 +528,12 @@ POSITIVE_TEST_CASE(symmetricEncryptDecrypt)
 	ca.setParam(ParamName::ALGO_TYPE, AlgoType::AES_GCM);
 	ca.setParam(ParamName::ED_AAD, createRandom(42));
 	encryptDecrypt();
+	ca.setParam(ParamName::ED_IV, createRandom(11));
+	encryptDecrypt();
+	ca.setParam(ParamName::ED_IV, createRandom(1));
+	encryptDecrypt();
+	ca.setParam(ParamName::ED_IV, createRandom(99));
+	encryptDecrypt();
 }
 
 NEGATIVE_TEST_CASE(symmetricEncryptDecrypt)
@@ -598,9 +604,6 @@ NEGATIVE_TEST_CASE(symmetricEncryptDecryptGcm)
 	// no iv
 	BOOST_REQUIRE_THROW(key->encrypt(ca, data), Exc::Crypto::InputParam);
 
-	// short iv
-	ca.setParam(ParamName::ED_IV, RawBuffer(1));
-	BOOST_REQUIRE_THROW(key->encrypt(ca, data), Exc::Crypto::InternalError);
 	ca.setParam(ParamName::ED_IV, iv);
 
 	// short key
@@ -628,9 +631,6 @@ NEGATIVE_TEST_CASE(symmetricEncryptDecryptGcm)
 	// no iv
 	BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InputParam);
 
-	// short iv
-	ca2.setParam(ParamName::ED_IV, RawBuffer(1));
-	BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InternalError);
 	ca2.setParam(ParamName::ED_IV, iv);
 
 	// short key