guest/hash: Fix get_hash_data

There are several things wrong with get_hash_data:
	1. it doesn't work, all returned hashes are all 0x00
	2. memory leak of hash allocated by crypto_md_generate_hash
	3. memory leak of der data buffer from is_x509certificate
	4. useless casting of hash_ids to x509_hash_ids

Issues 1-3 can be seen below
$ bin/secvarctl-dbg -m guest generate f:e -i ./test/testdata/guest/x509certs/dbx.crt -o dbx.esl -n dbx
  error: invalid crypto alg key 8
  error: invalid crypto alg key 11
  error: invalid crypto alg key 8
  RESULT: SUCCESS

  =================================================================
  ==88342==ERROR: LeakSanitizer: detected memory leaks

  Direct leak of 813 byte(s) in 1 object(s) allocated from:
    #0 0x7fa153bdebb8 in __interceptor_malloc (/lib64/libasan.so.5+0xefbb8)
    #1 0x7fa1537a42da in PEM_read_bio_ex (/lib64/libcrypto.so.1.1+0x19f2da)

  Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7fa153bdebb8 in __interceptor_malloc (/lib64/libasan.so.5+0xefbb8)
    #1 0x463e27 in crypto_md_generate_hash src/crypto_openssl.c:768
    open-power#2 0x445b4a in get_hash_data backends/guest/common/generate.c:477
    open-power#3 0x43234a in generate_esl backends/guest/guest_svc_generate.c:73
    open-power#4 0x43505e in generate_data backends/guest/guest_svc_generate.c:365
    open-power#5 0x439382 in guest_generate_command backends/guest/guest_svc_generate.c:690
    open-power#6 0x4069b4 in main /home/nchild/IBM/secvarctl-stuff/erichte-guest-devel/secvarctl/secvarctl.c:249
    open-power#7 0x7fa152558d84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)

  SUMMARY: AddressSanitizer: 845 byte(s) leaked in 2 allocation(s).

$ hexdump dbx.esl
  0000000 1626 c1c4 504c 4092 a9ac f941 9336 2843
  0000010 004c 0000 0000 0000 0030 0000 0000 0000
  0000020 0000 0000 0000 0000 0000 0000 0000 0000
  *
  0000040 0000 0000 0000 0000 0000 0000
  000004c

Fixing issue 1 and 2:
  get_hash_data() acts as a middle function during hash generation.
  First generate_esl() allocates an empty hash buffer and hands it to
  get_hash_data(). get_hash_data() then hands the address to that buffer
  to crypto_md_generate_hash(). crypto_md_generate_hash() allocates its
  own buffer and overwrites our pointer to our buffer. get_hash_data()
  has no way of updating generate_esl() of the new address. This results
  in a successful hash generation but a memoryleaks and an empty hash
  in the esl:
  To avoid this, simply copy the returned buffer into the existing buffer.

Fixing issue 3:
  is_x509certificate will allocate and return data if it finds the given
  buffer contains an x509.
  To fix this, simply free the der data after generating the hash.

Fixing issue 4:
  This is only a semi-issue, but previously, if the buffer was a
  certificate then we would cast our hash_function parameter into an
  x509 hash function ID. Later on this value get passed to
  get_crypto_alg_id() to get the crypto library specific MD identifier.
  get_crypto_alg_id() does not keep track of x509 hash function ID (and
  tbh the difference between them is not clear) and defaults to sha256.
  The point of converting the given hash id to x509 hash id only to then
  convert that value into crypto library specific ID is unnecessary.
  To fix just stick with the hash function passed to the function.

Lastly, add a test case for all this.

Signed-off-by: Nick Child <nick.child@ibm.com>

backends/guest/common/generate.c

@@ -449,7 +449,7 @@ int is_x509certificate(const uint8_t *buffer, const size_t buffer_size, uint8_t
  * @param buffer_size , length of buffer
  * @param hash_funct, index of hash function information to use for ESL GUID,
  *                   also helps in prevalation, if inform is '[c]ert' then this doesn't matter
- * @param hash_data, the generated hash data
+ * @param hash_data, the generated hash data, buffer should be allocated before calling
  * @param hash_data_size, the length of hash data
  * @param esl_guid, signature type of ESL
  * @return SUCCESS or err number
@@ -458,27 +458,23 @@ int get_hash_data(const uint8_t *buffer, const size_t buffer_size, enum signatur
 		  uint8_t *hash_data, size_t *hash_data_size)
 {
 	int rc = SUCCESS;
-	size_t data_size = 0;
-	uint8_t *data = NULL;
-	enum signature_type x509_hash_func;
+	size_t crt_size = 0;
+	uint8_t *crt_der = NULL, *out_data = NULL;
 
-	rc = is_x509certificate(buffer, buffer_size, &data, &data_size);
+	rc = is_x509certificate(buffer, buffer_size, &crt_der, &crt_size);
 	if (rc == SUCCESS) {
-		rc = get_x509_hash_function(get_crypto_alg_name(hash_funct), &x509_hash_func);
-		if (rc)
-			return rc;
-
-		hash_funct = x509_hash_func;
-	} else {
-		data_size = buffer_size;
-		data = (uint8_t *)buffer;
-	}
-
-	rc = crypto_md_generate_hash(data, data_size, get_crypto_alg_id(hash_funct), &hash_data,
-				     hash_data_size);
+		rc = crypto_md_generate_hash(crt_der, crt_size, get_crypto_alg_id(hash_funct),
+					     &out_data, hash_data_size);
+		free(crt_der);
+	} else
+		rc = crypto_md_generate_hash(buffer, buffer_size, get_crypto_alg_id(hash_funct),
+					     &out_data, hash_data_size);
 
 	if (rc != CRYPTO_SUCCESS)
 		return HASH_FAIL;
 
+	memcpy(hash_data, out_data, *hash_data_size);
+	free(out_data);
+
 	return SUCCESS;
 }

backends/guest/include/common/generate.h

@@ -139,7 +139,7 @@ int is_x509certificate(const uint8_t *buffer, const size_t buffer_size, uint8_t
  * @param buffer_size , length of buffer
  * @param hash_funct, index of hash function information to use for ESL GUID,
  *                   also helps in prevalation, if inform is '[c]ert' then this doesn't matter
- * @param hash_data, the generated hash data
+ * @param hash_data, the generated hash data, should already be allocated to hold hash
  * @param hash_data_size, the length of hash data
  * @param esl_guid, signature type of ESL
  * @return SUCCESS or err number

test/guest_tests.py

@@ -187,6 +187,11 @@ def test_generate_esl_files(self):
             cmd = generate_esl(var_name, format_type, cert_file, esl_file)
             self.assertCmdTrue(cmd)
 
+            # dbx ESl can be generated with a hash OR a cert, validate both
+            if var_name == "dbx":
+                cmd = generate_esl(var_name, file_to_esl, cert_file, esl_file)
+                self.assertCmdTrue(cmd)
+
     def test_generate_auth_files(self):
         for var_by_PK in variable_by_PK:
             auth_file = gen_dir + var_by_PK[0] + ".auth"