[Skiboot] [PATCH v2 10/12] secvar/test: add edk2-compat driver test and test data

Stefan Berger stefanb at linux.ibm.com
Fri Jan 24 00:35:30 AEDT 2020


On 1/19/20 9:36 PM, Eric Richter wrote:
> This patch contains a set of tests to exercise the edk2 driver using actual
> properly (and in some cases, improperly) signed binary data.
>
> Due to the excessive size of the binary data included in the header files,
> this test was split into its own patch.
>
> Co-developed-by: Nayna Jain <nayna at linux.ibm.com>
> Signed-off-by: Nayna Jain <nayna at linux.ibm.com>
> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
> ---
>   libstb/secvar/test/Makefile.check            |   6 +-
>   libstb/secvar/test/data/KEK.h                | 170 +++++
>   libstb/secvar/test/data/PK1.h                | 170 +++++
>   libstb/secvar/test/data/edk2_test_data.h     | 764 +++++++++++++++++++
>   libstb/secvar/test/data/multipleDB.h         | 246 ++++++
>   libstb/secvar/test/data/multipleKEK.h        | 236 ++++++
>   libstb/secvar/test/data/multiplePK.h         | 236 ++++++
>   libstb/secvar/test/data/noPK.h               | 102 +++
>   libstb/secvar/test/secvar-test-edk2-compat.c | 394 ++++++++++
>   libstb/secvar/test/secvar_common_test.c      |   2 +
>   10 files changed, 2324 insertions(+), 2 deletions(-)
>   create mode 100644 libstb/secvar/test/data/KEK.h
>   create mode 100644 libstb/secvar/test/data/PK1.h
>   create mode 100644 libstb/secvar/test/data/edk2_test_data.h
>   create mode 100644 libstb/secvar/test/data/multipleDB.h
>   create mode 100644 libstb/secvar/test/data/multipleKEK.h
>   create mode 100644 libstb/secvar/test/data/multiplePK.h
>   create mode 100644 libstb/secvar/test/data/noPK.h
>   create mode 100644 libstb/secvar/test/secvar-test-edk2-compat.c
>
> diff --git a/libstb/secvar/test/Makefile.check b/libstb/secvar/test/Makefile.check
> index b704a071..8d1b98d6 100644
> --- a/libstb/secvar/test/Makefile.check
> +++ b/libstb/secvar/test/Makefile.check
> @@ -29,13 +29,15 @@ $(SECVAR_TEST:%=%-check) : %-check: %
>   	$(call QTEST, RUN-TEST ,$(VALGRIND) $<, $<)
>   	@$(RM) -f secboot.img
>   
> +HOSTMBEDFLAGS += -lmbedcrypto -lmbedx509
> +
>   $(SECVAR_TEST) : core/test/stubs.o
>   
>   $(SECVAR_TEST) : % : %.c
> -	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) -O0 -g -I include -I . -I libfdt -o $@ $< core/test/stubs.o, $<)
> +	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) $(HOSTMBEDFLAGS) -O0 -g -I include -I . -I libfdt -o $@ $< core/test/stubs.o, $<)
>   
>   $(SECVAR_TEST:%=%-gcov): %-gcov : %.c %
> -	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) $(HOSTGCOVCFLAGS) -I include -I . -I libfdt -lgcov -o $@ $< core/test/stubs.o, $<)
> +	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) $(HOSTGCOVCFLAGS) $(HOSTMBEDFLAGS) -I include -I . -I libfdt -lgcov -o $@ $< core/test/stubs.o, $<)
>   
>   -include $(wildcard libstb/secvar/test/*.d)
>   
> diff --git a/libstb/secvar/test/data/KEK.h b/libstb/secvar/test/data/KEK.h
> new file mode 100644
> index 00000000..23dc3774
> --- /dev/null
> +++ b/libstb/secvar/test/data/KEK.h
> @@ -0,0 +1,170 @@
> +/* Good KEK */
> +unsigned char KEK_auth[] = {
> +	0xe3, 0x07, 0x0b, 0x16, 0x0e, 0x05, 0x2b, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x91, 0x04, 0x00, 0x00, 0x00, 0x02, 0xf1, 0x0e,
> +	0x9d, 0xd2, 0xaf, 0x4a, 0xdf, 0x68, 0xee, 0x49, 0x8a, 0xa9, 0x34, 0x7d,
> +	0x37, 0x56, 0x65, 0xa7, 0x30, 0x82, 0x04, 0x75, 0x06, 0x09, 0x2a, 0x86,
[...]
> +	0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02, 0xa0, 0x82, 0x04, 0x66, 0x30,
> +	0x82, 0x04, 0x62, 0x02, 0x01, 0x01, 0x31, 0x0f, 0x30, 0x0d, 0x06, 0x09,
> +	0xfc, 0x77, 0x06, 0x2e, 0x3d, 0x4f, 0xa1, 0x14, 0x04, 0x5d, 0xae, 0x38,
> +	0x28, 0xf9, 0x3d, 0x82, 0x5f, 0xc6, 0xd0, 0x31, 0x21, 0x88, 0xda, 0x7f,
> +	0x78, 0xe3, 0xb7, 0xed, 0x52, 0x37, 0xf4, 0x29, 0x08, 0x88, 0x50, 0x54,
> +	0x56, 0x67, 0xc0, 0xe1, 0xf4, 0xe7, 0xcf,
> +};
> +unsigned int KEK_auth_len = 1987;


sizeof() should work here as well. And may be appropriate to load these 
from files. There are so many of these include files. KEK_auth = 
load_file("KEK_auth.bin", &KEK_auth_len);



>
> diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
> new file mode 100644
> index 00000000..9ad92ee9
> --- /dev/null
> +++ b/libstb/secvar/test/secvar-test-edk2-compat.c
> @@ -0,0 +1,394 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#define TSS_NV_Read NULL
> +#define TSS_NV_Write NULL
> +#define TSS_NV_Define_Space NULL
> +
> +#include "secvar_common_test.c"
> +#include "../backend/edk2-compat.c"
> +#include "../secvar_util.c"
> +#include "../secvar_tpmnv.c"
> +#define MBEDTLS_PKCS7_USE_C
> +#include "../../crypto/pkcs7/pkcs7.c"
> +#include <platform.h>
> +#include "./data/edk2_test_data.h"
> +#include "./data/PK1.h"
> +#include "./data/noPK.h"
> +#include "./data/KEK.h"
> +#include "./data/multipleKEK.h"
> +#include "./data/multipleDB.h"
> +#include "./data/multiplePK.h"
> +
> +const char *secvar_test_name = "edk2-compat";
> +
> +struct platform platform;
> +
> +// Change to TSS-intercepting wrappers
> +#define ARBITRARY_TPMNV_SIZE 2048
> +char *secboot_buffer;


static?


> +static int secboot_read(void *dst, uint32_t src, uint32_t len)
> +{
> +	(void) src; // Don't need to use offset here

I see other parts in skiboot use '__attribute__((unused))', so you 
should use this also.


> +	memcpy(dst, secboot_buffer, len);
> +	return 0;
> +}
> +
> +static int secboot_write(uint32_t dst, void *src, uint32_t len)
> +{
> +	(void) dst;
> +	memcpy(secboot_buffer, src, len);
> +	return 0;
> +}
> +
> +int secvar_set_secure_mode(void) { return 0; };
> +
> +int run_test()
> +{
> +	int rc = -1;
> +	struct secvar_node *tmp;
> +	int keksize;
> +	int dbsize;
> +	struct secvar_node *ts;
> +	ts = alloc_secvar(sizeof(struct secvar) + 64);
> +        memcpy(ts->var->key, "TS", 3);
> +        ts->var->key_len = 3;
> +        memset(ts->var->data, 0, 64);
some whitespace errors here. You should have a wrapper for this 
ever-repeating initialization of a secvar_node: secvar_new().
> +	ts->var->data_size = 64;
> +
> +	// Check pre-process creates the empty variables
> +	ASSERT(0 == list_length(&variable_bank));
> +	rc = edk2_compat_pre_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	tmp = find_secvar("TS", 3, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(64 == tmp->var->data_size);
> +	ASSERT(!(memcmp(tmp->var->data, ts->var->data, 64)));
> +	
> +
> +	// Add PK to update and .process()
> +	printf("Add PK");
> +	tmp = alloc_secvar(PK1_auth_len);
> +	memcpy(tmp->var->key, "PK", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, PK1_auth, PK1_auth_len);
> +	tmp->var->data_size = PK1_auth_len;

I think the suggested secvar_new() would be go a long way in this file.


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("PK", 3, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +	ASSERT(PK_auth_len > tmp->var->data_size); // esl should be smaller without auth
> +	ASSERT(!setup_mode);
> +
> +	// Add db, should fail with no KEK
> +	printf("Add db");
> +	dbsize = sizeof(DB_auth);
> +	tmp = alloc_secvar(dbsize);
> +	memcpy(tmp->var->key, "db", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, DB_auth, dbsize);
> +	tmp->var->data_size = dbsize;

Also use here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS != rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("db", 3, &variable_bank);
> +	ASSERT(NULL != tmp);
> +
> +	printf("Add KEK");
> +
> +	// Add valid KEK, .process(), succeeds
> +
> +	tmp = alloc_secvar(KEK_auth_len);
> +	memcpy(tmp->var->key, "KEK", 4);
> +	tmp->var->key_len = 4;
> +	memcpy(tmp->var->data, KEK_auth, KEK_auth_len);
> +	tmp->var->data_size = KEK_auth_len;


and here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("KEK", 4, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Add valid KEK, .process(), timestamp check fails
> +
> +	tmp = alloc_secvar(ValidKEK_auth_len);
> +	memcpy(tmp->var->key, "KEK", 4);
> +	tmp->var->key_len = 4;
> +	memcpy(tmp->var->data, ValidKEK_auth, ValidKEK_auth_len);
> +	tmp->var->data_size = ValidKEK_auth_len;


and here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_PERMISSION == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("KEK", 4, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Add db, .process(), should succeed
> +	printf("Add db again\n");
> +	dbsize = sizeof(DB_auth);
> +	tmp = alloc_secvar(dbsize);
> +	memcpy(tmp->var->key, "db", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, DB_auth, dbsize);
> +	tmp->var->data_size = dbsize;


and here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("db", 3, &variable_bank);
> +	printf("tmp is %s\n", tmp->var->key);

should'nt is say 'key is'


> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Add db, .process(), should fail because of timestamp
> +	printf("Add db again\n");
> +	dbsize = sizeof(DB_auth);
> +	tmp = alloc_secvar(dbsize);
> +	memcpy(tmp->var->key, "db", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, DB_auth, dbsize);
> +	tmp->var->data_size = dbsize;


and here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_PERMISSION == rc);
> +
> +	// Add invalid KEK, .process(), should fail
> +	printf("Add invalid KEK\n");
> +	keksize = sizeof(InvalidKEK_auth);
> +	tmp = alloc_secvar(keksize);
> +	memcpy(tmp->var->key, "KEK", 4);
> +	tmp->var->key_len = 4;
> +	memcpy(tmp->var->data, InvalidKEK_auth, keksize);
> +	tmp->var->data_size = keksize;


and here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS != rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("KEK", 4, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Add ill formatted KEK, .process(), should fail
> +	printf("Add invalid KEK\n");
> +	keksize = sizeof(IllformatKEK_auth);
> +	tmp = alloc_secvar(keksize);
> +	memcpy(tmp->var->key, "KEK", 4);
> +	tmp->var->key_len = 4;
> +	memcpy(tmp->var->data, IllformatKEK_auth, keksize);
> +	tmp->var->data_size = keksize;


and here


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS != rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("KEK", 4, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Add multiple KEK ESLs, one of them should sign the db
> +	printf("Add multiple KEK\n");
> +	tmp = alloc_secvar(multipleKEK_auth_len);
> +	memcpy(tmp->var->key, "KEK", 4);
> +	tmp->var->key_len = 4;
> +	memcpy(tmp->var->data, multipleKEK_auth, multipleKEK_auth_len);
> +	tmp->var->data_size = multipleKEK_auth_len;


and here ... and so on


> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("KEK", 4, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Add multiple DB ESLs signed with second key of the KEK
> +	printf("Add multiple db\n");
> +	tmp = alloc_secvar(multipleDB_auth_len);
> +	memcpy(tmp->var->key, "db", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, multipleDB_auth, multipleDB_auth_len);
> +	tmp->var->data_size = multipleDB_auth_len;
> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("db", 3, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 != tmp->var->data_size);
> +
> +	// Delete PK.
> +	printf("Delete PK\n");
> +	tmp = alloc_secvar(noPK_auth_len);
> +	memcpy(tmp->var->key, "PK", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, noPK_auth, noPK_auth_len);
> +	tmp->var->data_size = noPK_auth_len;
> +	ASSERT(0 == edk2_compat_validate(tmp->var));
> +	list_add_tail(&update_bank, &tmp->link);
> +	ASSERT(1 == list_length(&update_bank));
> +
> +	rc = edk2_compat_process();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(5 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	tmp = find_secvar("PK", 3, &variable_bank);
> +	ASSERT(NULL != tmp);
> +	ASSERT(0 == tmp->var->data_size);
> +	ASSERT(setup_mode);
> +
> +	// Add multiple PK.
> +	printf("Multiple PK\n");
> +	tmp = alloc_secvar(multiplePK_auth_len);
> +	memcpy(tmp->var->key, "PK", 3);
> +	tmp->var->key_len = 3;
> +	memcpy(tmp->var->data, multiplePK_auth, multiplePK_auth_len);
> +	tmp->var->data_size = multiplePK_auth_len;
> +	ASSERT(0 != edk2_compat_validate(tmp->var));
> +
> +	return 0;
> +}
> +
> +static int run_edk2_tpmnv_test(void)
> +{
> +	int size, rc;
> +	char *tmp;
> +
> +	size = secvar_tpmnv_size(TPMNV_ID_EDK2_PK);
> +	ASSERT(size > 0);
> +	ASSERT(size < 1024);
> +	tmp = malloc(size);
> +	rc = secvar_tpmnv_read(TPMNV_ID_EDK2_PK, tmp, size, 0);
> +	ASSERT(OPAL_SUCCESS == rc);
> +	// memcmp here?
> +
> +	free(tmp);
> +	tmp = NULL; // Going to reuse this pointer later...
no need to init to NULL.
> +
> +	clear_bank_list(&variable_bank);
> +	ASSERT(0 == list_length(&variable_bank));
> +
> +	rc = edk2_p9_load_pk();
> +	ASSERT(OPAL_SUCCESS == rc);
> +	ASSERT(1 == list_length(&variable_bank));
> +
> +	// Now lets double check that write is working...
> +	memset(secboot_buffer, 0, ARBITRARY_TPMNV_SIZE);
> +
> +	rc = edk2_p9_write_pk();
> +	ASSERT(OPAL_SUCCESS == rc);
> +
> +	for (tmp = secboot_buffer;
> +		tmp <= secboot_buffer + ARBITRARY_TPMNV_SIZE;
> +		tmp++)
> +		if (*tmp != '\0')
> +			return 0; // Something was written
> +
> +	// Buffer was still empty
> +	return 1;
> +}
> +
> +int main(void)
> +{
> +	int rc;
> +
> +	tpm_fake_nv = 1;
> +	tpm_fake_nv_offset = 0;
> +
> +	list_head_init(&variable_bank);
> +	list_head_init(&update_bank);
> +
> +	secvar_storage.max_var_size = 4096;
> +
> +	// Run as a generic platform using whatever storage
> +	proc_gen = 0;
> +	rc = run_test();
> +	if (rc)
> +		goto out_bank;
> +
> +	clear_bank_list(&variable_bank);
> +	clear_bank_list(&update_bank);
> +	ASSERT(0 == list_length(&variable_bank));
> +	ASSERT(0 == list_length(&update_bank));
> +	printf("PASSED FIRST TEST\n");
> +
> +	// Run as "p9" and use the TPM for pk
> +	// TODO: Change to TSS stubs when this matters
> +	platform.secboot_read = secboot_read;
> +	platform.secboot_write = secboot_write;
> +	secboot_buffer = zalloc(ARBITRARY_TPMNV_SIZE);
> +
> +	proc_gen = proc_gen_p9;
> +	rc = run_test();
> +	if (rc)
> +		goto out;
> +
> +	printf("Run TPMNV tests\n");
> +	// Check that PK was actually written to "TPM" and load it
> +	rc = run_edk2_tpmnv_test();
> +
> +out:
> +	free(secboot_buffer);
> +out_bank:
> +	clear_bank_list(&variable_bank);
> +	clear_bank_list(&update_bank);
> +
> +	return rc;
> +}
> diff --git a/libstb/secvar/test/secvar_common_test.c b/libstb/secvar/test/secvar_common_test.c
> index fbc23145..13371bd6 100644
> --- a/libstb/secvar/test/secvar_common_test.c
> +++ b/libstb/secvar/test/secvar_common_test.c
> @@ -4,6 +4,8 @@
>   #define SECBOOT_FILE "secboot.img"
>   #define SECBOOT_SIZE 128000
>   
> +#define HAVE_LITTLE_ENDIAN 1
> +


Should this be here at all? I see the compiler passing in 
HAVE_BIG_ENDIAN like this: '[...] -DBITS_PER_LONG=64 -ffreestanding 
-DHAVE_BIG_ENDIAN -fno-strict-aliasing [...]'. You should probably 
compile your tests with HAVE_BIG_ENDIAN as well so you know that not 
only the tests are working but it's also working when it's running as 
part of skiboot.


>   #include <stdio.h>
>   #include <stdint.h>
>   #include <stdlib.h>




More information about the Skiboot mailing list