Skip to content

Commit

Permalink
FAPI: Fix the authorization of hierarchy objects.
Browse files Browse the repository at this point in the history
The default policy "policy secret" for the endorsement key is defined for the
endorsement hierarchy. The corresponding object had to be searched in the
keystore during policy execution. This search did not work, due to various
bugs.
* A erroneous cleanup of the EK was executed where the policy was deleted.
* The name needed for the search in the keystore was not stored in the
  hierarchy object.
* The esys handle stored in the hierarchy object was not correct.

To ensure compatibility with existing system keystores the initialization
of the hierarchy objects was fixed as follows:

* The initialization of the hierarchy object was moved to to object
  deserialization.
* Depending on the pathname the esys handle in the hierarchy initialized.
* The name of the hierarchy object was computed.
* The object needed for authorization during policy execution was initialized
  with the hierarchy object.
* The searching of objects in the keystore was fixed.
* Objects to be freed in Fapi_Import were initialzed with NULL.

The tests were adapted:
* The linking of the unit test for FAPI deserialization had to be adapted.
* An integration test where a key in the endorsement hierarchy was used
  for signing was added.
* PCR 16 was not reset int all tests expecting pcr 16 equal 0.

Fixes tpm2-software#2253

Signed-off-by: Juergen Repp <juergen.repp@sit.fraunhofer.de>
  • Loading branch information
JuergenReppSIT committed Jan 3, 2022
1 parent 724ca6a commit 7958fad
Show file tree
Hide file tree
Showing 13 changed files with 257 additions and 72 deletions.
17 changes: 15 additions & 2 deletions Makefile-test.am
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ FAPI_TESTS_INTEGRATION = \
test/integration/fapi-get-random.fint \
test/integration/fapi-platform-certificates.fint \
test/integration/fapi-key-create-sign.fint \
test/integration/fapi-key-create-he-sign.fint \
test/integration/fapi-key-create-primary-sign.fint \
test/integration/fapi-key-create2-sign.fint \
test/integration/fapi-key-create-null-key-sign.fint \
Expand Down Expand Up @@ -663,14 +664,19 @@ if FAPI

test_unit_fapi_json_CFLAGS = $(CMOCKA_CFLAGS) $(TESTS_CFLAGS)
test_unit_fapi_json_LDADD = $(CMOCKA_LIBS) $(TESTS_LDADD)
test_unit_fapi_json_LDFLAGS = $(TESTS_LDFLAGS) -ljson-c
test_unit_fapi_json_LDFLAGS = $(TESTS_LDFLAGS) $(CURL_LIBS) -ljson-c
test_unit_fapi_json_SOURCES = test/unit/fapi-json.c \
src/tss2-fapi/ifapi_json_deserialize.c \
src/tss2-fapi/ifapi_json_serialize.c \
src/tss2-fapi/ifapi_policy_json_deserialize.c \
src/tss2-fapi/ifapi_policy_json_serialize.c \
src/tss2-fapi/tpm_json_deserialize.c \
src/tss2-fapi/tpm_json_serialize.c
src/tss2-fapi/tpm_json_serialize.c \
src/tss2-fapi/ifapi_helpers.c \
src/tss2-fapi/fapi_crypto.c \
src/tss2-fapi/ifapi_eventlog.c \
src/tss2-fapi/ifapi_keystore.c \
src/tss2-fapi/ifapi_io.c

test_unit_fapi_helpers_CFLAGS = $(CMOCKA_CFLAGS) $(TESTS_CFLAGS)
test_unit_fapi_helpers_LDADD = $(CMOCKA_LIBS) $(TESTS_LDADD)
Expand Down Expand Up @@ -1706,6 +1712,13 @@ test_integration_fapi_key_create_sign_fint_SOURCES = \
test/integration/fapi-key-create-sign.int.c \
test/integration/main-fapi.c test/integration/test-fapi.h

test_integration_fapi_key_create_he_sign_fint_CFLAGS = $(TESTS_CFLAGS)
test_integration_fapi_key_create_he_sign_fint_LDADD = $(TESTS_LDADD)
test_integration_fapi_key_create_he_sign_fint_LDFLAGS = $(TESTS_LDFLAGS)
test_integration_fapi_key_create_he_sign_fint_SOURCES = \
test/integration/fapi-key-create-he-sign.int.c \
test/integration/main-fapi.c test/integration/test-fapi.h

test_integration_fapi_key_create_primary_sign_fint_CFLAGS = $(TESTS_CFLAGS)
test_integration_fapi_key_create_primary_sign_fint_LDADD = $(TESTS_LDADD)
test_integration_fapi_key_create_primary_sign_fint_LDFLAGS = $(TESTS_LDFLAGS)
Expand Down
2 changes: 2 additions & 0 deletions src/tss2-fapi/api/Fapi_Import.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ Fapi_Import_Async(
IFAPI_OBJECT *object = &command->object;
IFAPI_EXT_PUB_KEY * extPubKey = &object->misc.ext_pub_key;
IFAPI_DUPLICATE * keyTree = &object->misc.key_tree;
command->private = NULL;
command->parent_path = NULL;

if (context->state != _FAPI_STATE_INIT) {
return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
Expand Down
36 changes: 0 additions & 36 deletions src/tss2-fapi/fapi_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,10 +799,8 @@ ifapi_init_primary_finish(FAPI_CONTEXT *context, TSS2_KEY_TYPE ktype, IFAPI_OBJE
pkey->signing_scheme = context->profiles.default_profile.ecc_signing_scheme;
context->createPrimary.pkey_object.handle = primaryHandle;
SAFE_FREE(pkey->serialization.buffer);
ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
return TSS2_RC_SUCCESS;


statecasedefault(context->primary_state);
}

Expand Down Expand Up @@ -2999,8 +2997,6 @@ ifapi_initialize_object(
{
TSS2_RC r = TSS2_RC_SUCCESS;
ESYS_TR handle;
const char *path;
size_t pos = 0, pos2;

switch (object->objectType) {
case IFAPI_NV_OBJ:
Expand All @@ -3027,38 +3023,6 @@ ifapi_initialize_object(
object->handle = handle;
break;

case IFAPI_HIERARCHY_OBJ:
path = object->rel_path;
if (path) {
/* Determine esys handle from pathname. */
if (strncmp("/", &path[0], 1) == 0)
pos += 1;
/* Skip profile if it does exist in path */
if (strncmp("P_", &path[pos], 2) == 0) {
char * start = strchr(&path[pos], IFAPI_FILE_DELIM_CHAR);
if (start) {
pos2 = (int)(start - &path[pos]);
pos = pos2 + 2;
} else {
return_error(TSS2_FAPI_RC_GENERAL_FAILURE, "Invalid path.");
}
}

if (strcmp(&path[pos], "HS") == 0) {
object->handle = ESYS_TR_RH_OWNER;
} else if (strcmp(&path[pos], "HE") == 0) {
object->handle = ESYS_TR_RH_ENDORSEMENT;
} else if (strcmp(&path[pos], "LOCKOUT") == 0) {
object->handle = ESYS_TR_RH_LOCKOUT;
} else if (strcmp(&path[pos], "HN") == 0) {
object->handle = ESYS_TR_RH_NULL;
} else {
return_error(TSS2_FAPI_RC_GENERAL_FAILURE, "Invalid path.");
}
}
object->authorization_state = AUTH_INIT;
break;

default:
object->authorization_state = AUTH_INIT;
break;
Expand Down
92 changes: 91 additions & 1 deletion src/tss2-fapi/ifapi_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -922,12 +922,51 @@ append_object_to_list(void *object, NODE_OBJECT_T **object_list)
return TSS2_RC_SUCCESS;
}

/** Compute the name of a hierarchy object.
*
* The TPM handle will be computed from the esys handle and the name
* will be computed from the TPM handle.
*
* @param[in,out] hierarchy The hierarchy object.
*/
static void
set_name_hierarchy_object(IFAPI_OBJECT *object)
{
TPM2_HANDLE handle = 0;
size_t offset = 0;
switch (object->handle) {
case ESYS_TR_RH_NULL:
handle = TPM2_RH_NULL;
break;
case ESYS_TR_RH_OWNER:
handle = TPM2_RH_OWNER;
break;
case ESYS_TR_RH_ENDORSEMENT:
handle = TPM2_RH_ENDORSEMENT;
break;
case ESYS_TR_RH_LOCKOUT:
handle = TPM2_RH_LOCKOUT;
break;
case ESYS_TR_RH_PLATFORM:
handle = TPM2_RH_PLATFORM;
break;
case ESYS_TR_RH_PLATFORM_NV:
handle = TPM2_RH_PLATFORM_NV;
break;
}
Tss2_MU_TPM2_HANDLE_Marshal(handle,
&object->misc.hierarchy.name.name[0], sizeof(TPM2_HANDLE),
&offset);
object->misc.hierarchy.name.size = offset;
}

/** Initialize the internal representation of a FAPI hierarchy object.
*
* The object will be cleared and the type of the general fapi object will be
* set to hierarchy.
*
* @param[out] hierarchy The caller allocated hierarchy object.
* @param[in,out] hierarchy The caller allocated hierarchy object. The name of the
* object will be computed.
* @param[in] esys_handle The ESAPI handle of the hierarchy which will be added to
* to the object.
*/
Expand All @@ -940,6 +979,54 @@ ifapi_init_hierarchy_object(
hierarchy->system = TPM2_YES;
hierarchy->objectType = IFAPI_HIERARCHY_OBJ;
hierarchy->handle = esys_handle;
hierarchy->misc.hierarchy.esysHandle = esys_handle;
set_name_hierarchy_object(hierarchy);
}

/** Initialize a hierarchy object read from a file.
*
* The esys handles will be set depending on the object path and the
* object name will be computed.
*
* @param[in,out] hierarchy The caller allocated hierarchy object.
* @retval TSS2_RC_SUCCESS if the hierarchy could be initialized.
* @retval TSS2_FAPI_RC_GENERAL_FAILURE For an invalid hierarchy path.
*/
TSS2_RC
ifapi_set_name_hierarchy_object(IFAPI_OBJECT *object)
{
const char *path = object->rel_path;
size_t pos = 0, pos2;
if (path) {
/* Determine esys handle from pathname. */
if (strncmp("/", &path[0], 1) == 0)
pos += 1;
/* Skip profile if it does exist in path */
if (strncmp("P_", &path[pos], 2) == 0) {
char * start = strchr(&path[pos], IFAPI_FILE_DELIM_CHAR);
if (start) {
pos2 = (int)(start - &path[pos]);
pos = pos2 + 2;
} else {
return_error(TSS2_FAPI_RC_GENERAL_FAILURE, "Invalid path.");
}
}
if (strcmp(&path[pos], "HS") == 0) {
object->handle = ESYS_TR_RH_OWNER;
object->misc.hierarchy.esysHandle = ESYS_TR_RH_OWNER;
} else if (strcmp(&path[pos], "HE") == 0) {
object->handle = ESYS_TR_RH_ENDORSEMENT;
object->misc.hierarchy.esysHandle = ESYS_TR_RH_ENDORSEMENT;
} else if (strcmp(&path[pos], "LOCKOUT") == 0) {
object->handle = ESYS_TR_RH_LOCKOUT;
object->misc.hierarchy.esysHandle = ESYS_TR_RH_LOCKOUT;
} else if (strcmp(&path[pos], "HN") == 0) {
object->handle = ESYS_TR_RH_NULL;
object->misc.hierarchy.esysHandle = ESYS_TR_RH_NULL;
}
}
set_name_hierarchy_object(object);
return TSS2_RC_SUCCESS;
}

/** Create a directory and all sub directories.
Expand Down Expand Up @@ -1516,6 +1603,9 @@ ifapi_object_cmp_name(IFAPI_OBJECT *object, void *name, bool *equal)
TPM2B_NAME nv_name;

switch (object->objectType) {
case IFAPI_HIERARCHY_OBJ:
obj_name = &object->misc.hierarchy.name;
break;
case IFAPI_KEY_OBJ:
obj_name = &object->misc.key.name;
break;
Expand Down
4 changes: 4 additions & 0 deletions src/tss2-fapi/ifapi_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ ifapi_init_hierarchy_object(
IFAPI_OBJECT *hierarchy,
ESYS_TR esys_handle);

TSS2_RC
ifapi_set_name_hierarchy_object(
IFAPI_OBJECT *hierarchy);

char *
get_description(IFAPI_OBJECT *object);

Expand Down
5 changes: 3 additions & 2 deletions src/tss2-fapi/ifapi_json_deserialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,6 @@ ifapi_json_IFAPI_OBJECT_deserialize(json_object *jso, IFAPI_OBJECT *out)
return TSS2_FAPI_RC_BAD_VALUE;
}

out->rel_path = NULL;

r = ifapi_json_IFAPI_OBJECT_TYPE_CONSTANT_deserialize(jso2, &out->objectType);
return_if_error(r, "Bad value for field \"objectType\".");

Expand All @@ -690,6 +688,9 @@ ifapi_json_IFAPI_OBJECT_deserialize(json_object *jso, IFAPI_OBJECT *out)
r = ifapi_json_IFAPI_HIERARCHY_deserialize(jso, &out->misc.hierarchy);
return_if_error(r, "Bad value for hierarchy.");

r = ifapi_set_name_hierarchy_object(out);
return_if_error(r, "Bad hierarchy.");

break;
case IFAPI_KEY_OBJ:
r = ifapi_json_IFAPI_KEY_deserialize(jso, &out->misc.key);
Expand Down
2 changes: 1 addition & 1 deletion src/tss2-fapi/ifapi_keystore.c
Original file line number Diff line number Diff line change
Expand Up @@ -629,10 +629,10 @@ ifapi_keystore_load_finish(
goto_if_null2(jso, "Keystore is corrupted (Json error).", r, TSS2_FAPI_RC_GENERAL_FAILURE,
error_cleanup);

object->rel_path = keystore->rel_path;
r = ifapi_json_IFAPI_OBJECT_deserialize(jso, object);
goto_if_error(r, "Deserialize object.", error_cleanup);

object->rel_path = keystore->rel_path;
SAFE_FREE(buffer);
if (jso)
json_object_put(jso);
Expand Down
1 change: 1 addition & 0 deletions src/tss2-fapi/ifapi_keystore.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ typedef struct {
TPM2B_DIGEST authPolicy;
ESYS_TR esysHandle;
bool authorized; /**< Switch whether hiearchy is authorized. */
TPM2B_NAME name; /**< Name of the hierarchy */
} IFAPI_HIERARCHY;

/** Type for representing a FAPI NV object
Expand Down
1 change: 1 addition & 0 deletions src/tss2-fapi/ifapi_policy_callbacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ ifapi_policyeval_cbauth(
break;
} else if (cb_ctx->object.objectType == IFAPI_HIERARCHY_OBJ) {
cb_ctx->cb_state = POL_CB_AUTHORIZE_OBJECT;
cb_ctx->auth_object_ptr = &cb_ctx->object;
next_case = true;
break;
} else {
Expand Down
3 changes: 3 additions & 0 deletions test/integration/fapi-export-policy.int.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ test_fapi_export_policy(FAPI_CONTEXT *context)
r = Fapi_Provision(context, NULL, NULL, NULL);
goto_if_error(r, "Error Fapi_Provision", error);

r = pcr_reset(context, 16);
goto_if_error(r, "Error pcr_reset", error);

for (i = 0; i < sizeof(policies) / sizeof(policies[0]); i++) {
fprintf(stderr, "\nTest policy: %s\n", policies[i].path);
json_policy = read_policy(context, policies[i].path);
Expand Down
Loading

0 comments on commit 7958fad

Please sign in to comment.