swtpm: Return TPM_FAIL if SWTPM_NVRAM_DecrytpData is called without key

Return TPM_FAIL if SWTPM_NVRAM_DecryptData() is called without a key or
if an unhandle type of encryption mode is encountered. Previously this
function would return no error but also would not do any decryption if
no key was provided. Consequently, it would then also not return a byte
array with decrypted data which in turn could led to potential NULL
pointer accesses in subsequent calls. However, all current callers check
whether they have a valid key before they call this function. So the
change is primarily done for static analyzers, such as gcc -fanalyzer,
to ease code analysis.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

diff --git a/src/swtpm/swtpm_nvstore.c b/src/swtpm/swtpm_nvstore.c
index 953d6bb09792c32bd0dea25c236f642f83154f0c..ff5cb80afa55e610f4a4e6278343770e1fab3e5d 100644 (file)
--- a/src/swtpm/swtpm_nvstore.c
+++ b/src/swtpm/swtpm_nvstore.c
@@ -886,7 +886,7 @@ SWTPM_NVRAM_DecryptData(const encryptionkey *key,
                         uint16_t tag_ivec, uint16_t hdrflags,
                         uint16_t flag_256bitkey)
 {
-    TPM_RESULT rc = 0;
+    TPM_RESULT rc = TPM_FAIL;
     unsigned char *tmp_data = NULL;
     uint32_t tmp_length = 0;
     tlv_data td[2];
@@ -894,63 +894,62 @@ SWTPM_NVRAM_DecryptData(const encryptionkey *key,
     uint32_t ivec_length = 0;
     size_t keylen;
 
-    if (key->symkey.userKeyLength > 0) {
-        switch (key->data_encmode) {
-        case ENCRYPTION_MODE_UNKNOWN:
-            rc = TPM_BAD_MODE;
-            break;
-        case ENCRYPTION_MODE_AES_CBC:
-            switch (hdrversion) {
-            case 1:
-                rc = SWTPM_SymmetricKeyData_Decrypt(&tmp_data,
-                                                    &tmp_length,
-                                                    data, length,
-                                                    &key->symkey,
-                                                    NULL, 0);
-                if (rc == 0) {
-                    rc = SWTPM_CheckHash(tmp_data, tmp_length,
-                                         decrypt_data, decrypt_length);
-                }
-            break;
-            case 2:
-                keylen = (hdrflags & flag_256bitkey)
-                          ? SWTPM_AES256_BLOCK_SIZE : SWTPM_AES128_BLOCK_SIZE;
-                if (keylen != key->symkey.userKeyLength) {
-                    logprintf(STDERR_FILENO,
-                              "Wrong decryption key. Need %zu bit key.\n",
-                              keylen * 8);
-                    rc = TPM_BAD_KEY_PROPERTY;
-                    break;
-                }
-
-                if (!tlv_data_find_tag(data, length, TAG_HMAC, &td[0]) ||
-                    !tlv_data_find_tag(data, length, tag_encrypted_data,
-                                       &td[1])) {
-                    logprintf(STDERR_FILENO,
-                              "Could not find HMAC or encrypted data (tag %u) "
-                              "in byte stream.\n", tag_encrypted_data);
-                    rc = TPM_FAIL;
-                    break;
-                }
-                /* get the IV, if there is one */
-                SWTPM_GetIvec(data, length, &ivec, &ivec_length, tag_ivec);
-
-                rc = SWTPM_CheckHMAC(&td[0], &td[1], &key->symkey,
-                                     ivec, ivec_length);
-                if (rc == 0) {
-                    rc = SWTPM_SymmetricKeyData_Decrypt(decrypt_data,
-                                                        decrypt_length,
-                                                        td[1].u.const_ptr,
-                                                        td[1].tlv.length,
-                                                        &key->symkey,
-                                                        ivec, ivec_length);
-                }
-            break;
-            default:
+    if (key->symkey.userKeyLength == 0)
+        return rc;
+
+    switch (key->data_encmode) {
+    case ENCRYPTION_MODE_UNKNOWN:
+        rc = TPM_BAD_MODE;
+        break;
+    case ENCRYPTION_MODE_AES_CBC:
+        switch (hdrversion) {
+        case 1:
+            rc = SWTPM_SymmetricKeyData_Decrypt(&tmp_data,
+                                                &tmp_length,
+                                                data, length,
+                                                &key->symkey,
+                                                NULL, 0);
+            if (rc == 0) {
+                rc = SWTPM_CheckHash(tmp_data, tmp_length,
+                                     decrypt_data, decrypt_length);
+            }
+        break;
+        case 2:
+            keylen = (hdrflags & flag_256bitkey)
+                      ? SWTPM_AES256_BLOCK_SIZE : SWTPM_AES128_BLOCK_SIZE;
+            if (keylen != key->symkey.userKeyLength) {
+                logprintf(STDERR_FILENO,
+                          "Wrong decryption key. Need %zu bit key.\n",
+                          keylen * 8);
+                rc = TPM_BAD_KEY_PROPERTY;
+                break;
+            }
+
+            if (!tlv_data_find_tag(data, length, TAG_HMAC, &td[0]) ||
+                !tlv_data_find_tag(data, length, tag_encrypted_data,
+                                   &td[1])) {
+                logprintf(STDERR_FILENO,
+                          "Could not find HMAC or encrypted data (tag %u) "
+                          "in byte stream.\n", tag_encrypted_data);
                 rc = TPM_FAIL;
+                break;
+            }
+            /* get the IV, if there is one */
+            SWTPM_GetIvec(data, length, &ivec, &ivec_length, tag_ivec);
+
+            rc = SWTPM_CheckHMAC(&td[0], &td[1], &key->symkey,
+                                 ivec, ivec_length);
+            if (rc == 0) {
+                rc = SWTPM_SymmetricKeyData_Decrypt(decrypt_data,
+                                                    decrypt_length,
+                                                    td[1].u.const_ptr,
+                                                    td[1].tlv.length,
+                                                    &key->symkey,
+                                                    ivec, ivec_length);
             }
-            free(tmp_data);
+        break;
         }
+        free(tmp_data);
     }
 
     return rc;