Fix AES GCM IV setting in KeyProvider 74/299874/1
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Fri, 29 Sep 2023 05:09:19 +0000 (07:09 +0200)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Wed, 11 Oct 2023 08:57:58 +0000 (10:57 +0200)
The proper order of setting an IV of custom length in GCM is to firstly
pass the length and then the IV during both encryption and decryption.
The KeyProvider code was doing the opposite which resulted in
truncating the IV to 12B. In openssl3.0 The effect is somewhat
diffferent but the output is also invalid.

Openssl silently ignores this issue allowing to call the API in wrong
order and get invalid encryption/decryption results.

The issue was not detected until now because the IV truncation was
working the same way during encryption and decryption and no other
module beside key-manager was accessing the encrypted keys.

This commit modifies KeyProvider code in 2 ways:
* Modify encryption and decryption to set the IV length properly.
* If decryption fails retry it with IV truncated to 12B to handle data
  encrypted the old way.

Change-Id: I72e237b0842234d80579f3e93b5e1012a0613140

src/manager/service/key-provider.cpp

index cf15ed8..2bbe45f 100644 (file)
@@ -37,6 +37,7 @@ namespace {
 constexpr int PBKDF2_ITERATIONS = 4096;
 constexpr uint32_t KEYCOMPONENT_VERSION = 2;
 constexpr int OPENSSL_ENGINE_ERROR = -4;
+constexpr int AUTHENTICATION_ERROR = -5;
 
 template<typename T>
 RawBuffer toRawBuffer(const T &data)
@@ -72,10 +73,10 @@ int encryptAes256Gcm(const unsigned char *plaintext,
        if (!EVP_EncryptInit_ex(ctx.get(), EVP_aes_256_gcm(), NULL, NULL, NULL))
                return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_EncryptInit_ex(ctx.get(), NULL, NULL, key, iv))
+       if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_IVLEN, MAX_IV_SIZE, NULL))
                return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_IVLEN, MAX_IV_SIZE, NULL))
+       if (!EVP_EncryptInit_ex(ctx.get(), NULL, NULL, key, iv))
                return OPENSSL_ENGINE_ERROR;
 
        if (!EVP_EncryptUpdate(ctx.get(), ciphertext, &len, plaintext, plaintext_len))
@@ -98,40 +99,48 @@ int decryptAes256Gcm(const unsigned char *ciphertext,
                      int ciphertext_len, unsigned char *tag, const unsigned char *key,
                      const unsigned char *iv, unsigned char *plaintext)
 {
-       int len;
-       int plaintext_len;
-       int ret;
+       auto decrypt = [&](size_t iv_len){
+               int len;
+               int plaintext_len;
+               int ret;
 
-       auto ctx = uptr<EVP_CIPHER_CTX_free>(EVP_CIPHER_CTX_new());
-       if (!ctx)
-               return OPENSSL_ENGINE_ERROR;
+               auto ctx = uptr<EVP_CIPHER_CTX_free>(EVP_CIPHER_CTX_new());
+               if (!ctx)
+                       return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_DecryptInit_ex(ctx.get(), EVP_aes_256_gcm(), NULL, NULL, NULL))
-               return OPENSSL_ENGINE_ERROR;
+               if (!EVP_DecryptInit_ex(ctx.get(), EVP_aes_256_gcm(), NULL, NULL, NULL))
+                       return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_DecryptInit_ex(ctx.get(), NULL, NULL, key, iv))
-               return OPENSSL_ENGINE_ERROR;
+               if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_IVLEN, iv_len, NULL))
+                       return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_IVLEN, MAX_IV_SIZE, NULL))
-               return OPENSSL_ENGINE_ERROR;
+               if (!EVP_DecryptInit_ex(ctx.get(), NULL, NULL, key, iv))
+                       return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_TAG, MAX_IV_SIZE, tag))
-               return OPENSSL_ENGINE_ERROR;
+               if (!EVP_DecryptUpdate(ctx.get(), plaintext, &len, ciphertext, ciphertext_len))
+                       return OPENSSL_ENGINE_ERROR;
 
-       if (!EVP_DecryptUpdate(ctx.get(), plaintext, &len, ciphertext, ciphertext_len))
-               return OPENSSL_ENGINE_ERROR;
+               plaintext_len = len;
 
-       plaintext_len = len;
+               if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_TAG, MAX_IV_SIZE, tag))
+                       return OPENSSL_ENGINE_ERROR;
 
-       if (!(ret = EVP_DecryptFinal_ex(ctx.get(), plaintext + len, &len)))
-               return OPENSSL_ENGINE_ERROR;
+               if (!(ret = EVP_DecryptFinal_ex(ctx.get(), plaintext + len, &len)))
+                       return AUTHENTICATION_ERROR;
 
-       if (ret > 0) {
-               plaintext_len += len;
-               return plaintext_len;
-       } else {
-               return -1;
-       }
+               if (ret > 0) {
+                       plaintext_len += len;
+                       return plaintext_len;
+               } else {
+                       return -1;
+               }
+       };
+
+       auto ret = decrypt(MAX_IV_SIZE);
+       if (ret == AUTHENTICATION_ERROR)
+               ret = decrypt(12); // retry with truncated IV
+
+       return ret;
 }
 
 typedef std::array<uint8_t, MAX_KEY_SIZE> KeyData;