secvar/backend: Bugfixes in edk2 driver

This patch fixes following bugs. Additionally, it improves logs.

* Failure in adding/deleting PK as part of failure of processing any
subsequential update in the queue didn't reset the global variable
setup_mode to the original value. This patch adds the fix to always
set the value of setup_mode as per final contents in variable_bank
before existing process().

* Deletion of HWKH as part of deleting PK was only updating the value of
the variable to be zero. However, this didn't deallocate the variable from
the bank and was getting exposed via sysfs.

* The mismatch in verification of hw-key-hash, was also clearing staging
bank, which isn't initialized in this case. Fix the cleanup tag to only
clear update_bank.

* Fixes a memory leak in validate_esl_list().

* Convert signature verification error code from mbedtls into
opal error code as OPAL_PERMISSION.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

3 files changed, 37 insertions, 14 deletions

diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index 0129023..dfaec13 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -297,12 +297,14 @@ int validate_esl_list(const char *key, const char *esl, const size_t size)
 
 		if (key_equals(key, "dbx")) {
 			if (!validate_hash(list->SignatureType, dsize)) {
+				prlog(PR_ERR, "No valid hash is found\n");
 				rc = OPAL_PARAMETER;
 				break;
 			}
 		} else {
 		       if (!uuid_equals(&list->SignatureType, &EFI_CERT_X509_GUID)
 			   || !validate_cert(data, dsize)) {
+				prlog(PR_ERR, "No valid cert is found\n");
 				rc = OPAL_PARAMETER;
 				break;
 		       }
@@ -327,6 +329,8 @@ int validate_esl_list(const char *key, const char *esl, const size_t size)
 		}
 	}
 
+	free(data);
+
 	prlog(PR_INFO, "Total ESLs are %d\n", rc);
 	return rc;
 }
@@ -513,7 +517,7 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
 
 		/* This should not happen, unless something corrupted in PNOR */
 		if(rc) {
-			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
+			prlog(PR_ERR, "X509 certificate parsing failed %04x\n", rc);
 			rc = OPAL_INTERNAL_ERROR;
 			break;
 		}
@@ -542,13 +546,15 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
 			prlog(PR_INFO, "Signature Verification passed\n");
 			mbedtls_x509_crt_free(&x509);
 			break;
+		} else {
+			errbuf = zalloc(MBEDTLS_ERR_BUFFER_SIZE);
+			mbedtls_strerror(rc, errbuf, MBEDTLS_ERR_BUFFER_SIZE);
+			prlog(PR_ERR, "Signature Verification failed %02x %s\n",
+					rc, errbuf);
+			free(errbuf);
+			rc = OPAL_PERMISSION;
 		}
 
-		errbuf = zalloc(MBEDTLS_ERR_BUFFER_SIZE);
-		mbedtls_strerror(rc, errbuf, MBEDTLS_ERR_BUFFER_SIZE);
-		prlog(PR_INFO, "Signature Verification failed %02x %s\n",
-				rc, errbuf);
-		free(errbuf);
 
 		/* Look for the next ESL */
 		offset = offset + eslsize;
@@ -690,7 +696,7 @@ int process_update(const struct secvar *update, char **newesl,
 	rc = check_timestamp(update->key, timestamp, last_timestamp);
 	/* Failure implies probably an older command being resubmitted */
 	if (rc != OPAL_SUCCESS) {
-		prlog(PR_INFO, "Timestamp verification failed for key %s\n", update->key);
+		prlog(PR_ERR, "Timestamp verification failed for key %s\n", update->key);
 		goto out;
 	}
 
@@ -750,8 +756,10 @@ int process_update(const struct secvar *update, char **newesl,
 				      avar);
 
 		/* Break if signature verification is successful */
-		if (rc == OPAL_SUCCESS)
+		if (rc == OPAL_SUCCESS) {
+			prlog(PR_INFO, "Key %s successfully verified by authority %s\n", update->key, key_authority[i]);
 			break;
+		}
 	}
 
 out:
diff --git a/libstb/secvar/backend/edk2-compat-reset.c b/libstb/secvar/backend/edk2-compat-reset.c
index cc3c6d0..305ea08 100644
--- a/libstb/secvar/backend/edk2-compat-reset.c
+++ b/libstb/secvar/backend/edk2-compat-reset.c
@@ -77,14 +77,15 @@ int add_hw_key_hash(struct list_head *bank)
 int delete_hw_key_hash(struct list_head *bank)
 {
 	struct secvar *var;
-	int rc;
 
 	var = find_secvar("HWKH", 5, bank);
 	if (!var)
 		return OPAL_SUCCESS;
 
-	rc = update_variable_in_bank(var, NULL, 0, bank);
-	return rc;
+	list_del(&var->link);
+	dealloc_secvar(var);
+
+	return OPAL_SUCCESS;
 }
 
 int verify_hw_key_hash(void)
diff --git a/libstb/secvar/backend/edk2-compat.c b/libstb/secvar/backend/edk2-compat.c
index 52631c0..9e61fbc 100644
--- a/libstb/secvar/backend/edk2-compat.c
+++ b/libstb/secvar/backend/edk2-compat.c
@@ -112,7 +112,7 @@ static int edk2_compat_process(struct list_head *variable_bank,
 	if (!setup_mode) {
 		rc = verify_hw_key_hash();
 		if (rc != OPAL_SUCCESS) {
-			prlog(PR_ERR, "Hardware key hash verification mismatch\n");
+			prlog(PR_ERR, "Hardware key hash verification mismatch. Keystore and update queue is reset.\n");
 			rc = reset_keystore(variable_bank);
 			if (rc)
 				goto cleanup;
@@ -217,13 +217,27 @@ static int edk2_compat_process(struct list_head *variable_bank,
 		copy_bank_list(variable_bank, &staging_bank);
 	}
 
+	free(newesl);
+	clear_bank_list(&staging_bank);
+
+	/* Set the global variable setup_mode as per final contents in variable_bank */
+	var = find_secvar("PK", 3, variable_bank);
+	if (!var) {
+		/* This should not happen */
+		rc = OPAL_INTERNAL_ERROR;
+		goto cleanup;
+	}
+
+	if (var->data_size == 0)
+		setup_mode = true;
+	else
+		setup_mode = false;
+
 cleanup:
 	/*
 	 * For any failure in processing update queue, we clear the update bank
 	 * and return failure
 	 */
-	free(newesl);
-	clear_bank_list(&staging_bank);
 	clear_bank_list(update_bank);
 
 	return rc;