Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esys: remove trailing zeros in auth value. #2665

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Makefile-test.am
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ ESYS_TESTS_INTEGRATION_MANDATORY = \
test/integration/esys-certify-creation.int \
test/integration/esys-certifyX509.int \
test/integration/esys-certify.int \
test/integration/esys-check-auth-with-trailing-zero.int \
test/integration/esys-clear-control.int \
test/integration/esys-clockset.int \
test/integration/esys-clockset-audit.int \
Expand Down Expand Up @@ -1272,6 +1273,13 @@ test_integration_esys_change_eps_int_SOURCES = \
test/integration/esys-change-eps.int.c \
test/integration/main-esys.c test/integration/test-esys.h

test_integration_esys_check_auth_with_trailing_zero_int_CFLAGS = $(TESTS_CFLAGS)
test_integration_esys_check_auth_with_trailing_zero_int_LDADD = $(TESTS_LDADD)
test_integration_esys_check_auth_with_trailing_zero_int_LDFLAGS = $(TESTS_LDFLAGS)
test_integration_esys_check_auth_with_trailing_zero_int_SOURCES = \
test/integration/esys-check-auth-with-trailing-zero.int.c \
test/integration/main-esys.c test/integration/test-esys.h

test_integration_esys_clear_int_CFLAGS = $(TESTS_CFLAGS)
test_integration_esys_clear_int_LDADD = $(TESTS_LDADD)
test_integration_esys_clear_int_LDFLAGS = $(TESTS_LDFLAGS)
Expand Down
6 changes: 3 additions & 3 deletions src/tss2-esys/api/Esys_Create.c
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ Esys_Create_Async(

store_input_parameters (esysContext, inSensitive);
if (inPublic) {
r = iesys_hash_long_auth_values(
r = iesys_adapt_auth_value(
&esysContext->crypto_backend,
&esysContext->in.Create.inSensitive->sensitive.userAuth,
inPublic->publicArea.nameAlg);
&esysContext->in.Create.inSensitive->sensitive.userAuth,
inPublic->publicArea.nameAlg);
return_state_if_error(r, _ESYS_STATE_INIT, "Adapt auth value.");
}

Expand Down
2 changes: 1 addition & 1 deletion src/tss2-esys/api/Esys_CreateLoaded.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ Esys_CreateLoaded_Async(
&publicArea);
return_if_error(r, "Unmarshalling inPublic failed");

r = iesys_hash_long_auth_values(
r = iesys_adapt_auth_value(
&esysContext->crypto_backend,
&esysContext->in.CreateLoaded.inSensitive->sensitive.userAuth,
publicArea.nameAlg);
Expand Down
2 changes: 1 addition & 1 deletion src/tss2-esys/api/Esys_CreatePrimary.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Esys_CreatePrimary_Async(
return_state_if_error(r, _ESYS_STATE_INIT, "Check session usage");
store_input_parameters (esysContext, inSensitive);
if (inPublic) {
r = iesys_hash_long_auth_values(
r = iesys_adapt_auth_value(
&esysContext->crypto_backend,
&esysContext->in.CreatePrimary.inSensitive->sensitive.userAuth,
inPublic->publicArea.nameAlg);
Expand Down
10 changes: 7 additions & 3 deletions src/tss2-esys/api/Esys_HierarchyChangeAuth.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ static void store_input_parameters (
const TPM2B_AUTH *newAuth)
{
esysContext->in.HierarchyChangeAuth.authHandle = authHandle;
if (newAuth == NULL)
if (newAuth == NULL) {
memset(&esysContext->in.HierarchyChangeAuth.newAuth, 0,
sizeof(esysContext->in.HierarchyChangeAuth.newAuth));
else
} else {
esysContext->in.HierarchyChangeAuth.newAuth = *newAuth;
iesys_strip_trailing_zeros(&esysContext->in.HierarchyChangeAuth.newAuth);
}
}

/** One-Call function for TPM2_HierarchyChangeAuth
Expand Down Expand Up @@ -175,7 +177,9 @@ Esys_HierarchyChangeAuth_Async(
/* Check input parameters */
r = check_session_feasibility(shandle1, shandle2, shandle3, 1);
return_state_if_error(r, _ESYS_STATE_INIT, "Check session usage");

store_input_parameters(esysContext, authHandle, newAuth);
iesys_strip_trailing_zeros(&esysContext->in.HierarchyChangeAuth.newAuth);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to not do this in store_input_parameters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good hint. The spec states:

This command does not change the authorization of the TPM-resident object on which it operates.
Therefore, the old authValue (of the TPM-resident object) is used when generating the response HMAC
key if required.

So it makes no sense to store the auth value in the esys object. I will remove this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @williamcroberts wants the "iesys_strip_trailing_zeros" command to be implemented in the "store_input_parameters" function. This makes also sense to me.
@JuergenReppSIT: Could you do this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@williamcroberts @cplappert Sorry my comment was related to Esys_ObjectChangeAuth. I will do it in store_input_parameters.


/* Retrieve the metadata objects for provided handles */
r = esys_GetResourceObject(esysContext, authHandle, &authHandleNode);
Expand All @@ -186,7 +190,7 @@ Esys_HierarchyChangeAuth_Async(
(authHandleNode == NULL)
? TPM2_RH_NULL
: authHandleNode->rsrc.handle,
newAuth);
&esysContext->in.HierarchyChangeAuth.newAuth);
return_state_if_error(r, _ESYS_STATE_INIT, "SAPI Prepare returned error.");

/* Calculate the cpHash Values */
Expand Down
6 changes: 6 additions & 0 deletions src/tss2-esys/api/Esys_NV_ChangeAuth.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ Esys_NV_ChangeAuth_Async(
esysContext, nvIndex, newAuth);
TSS2L_SYS_AUTH_COMMAND auths;
RSRC_NODE_T *nvIndexNode;
TPM2B_AUTH *authCopy;
TPMI_ALG_HASH hashAlg;

/* Check context, sequence correctness and set state to error for now */
if (esysContext == NULL) {
Expand All @@ -174,10 +176,14 @@ Esys_NV_ChangeAuth_Async(
r = check_session_feasibility(shandle1, shandle2, shandle3, 1);
return_state_if_error(r, _ESYS_STATE_INIT, "Check session usage");
store_input_parameters(esysContext, nvIndex, newAuth);
authCopy = &esysContext->in.HierarchyChangeAuth.newAuth;

/* Retrieve the metadata objects for provided handles */
r = esys_GetResourceObject(esysContext, nvIndex, &nvIndexNode);
return_state_if_error(r, _ESYS_STATE_INIT, "nvIndex unknown.");
hashAlg = nvIndexNode->rsrc.misc.rsrc_nv_pub.nvPublic.nameAlg;
r = iesys_adapt_auth_value(&esysContext->crypto_backend, authCopy, hashAlg);
return_state_if_error(r, _ESYS_STATE_INIT, "Adapt auth value");

/* Initial invocation of SAPI to prepare the command buffer with parameters */
r = Tss2_Sys_NV_ChangeAuth_Prepare(esysContext->sys,
Expand Down
6 changes: 3 additions & 3 deletions src/tss2-esys/api/Esys_NV_DefineSpace.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,9 @@ Esys_NV_DefineSpace_Async(
store_input_parameters(esysContext, auth, publicInfo);

if (publicInfo) {
r = iesys_hash_long_auth_values(&esysContext->crypto_backend,
&esysContext->in.NV.authData,
publicInfo->nvPublic.nameAlg);
r = iesys_adapt_auth_value(&esysContext->crypto_backend,
&esysContext->in.NV.authData,
publicInfo->nvPublic.nameAlg);
return_state_if_error(r, _ESYS_STATE_INIT, "Adapt auth value.");
}

Expand Down
18 changes: 17 additions & 1 deletion src/tss2-esys/api/Esys_ObjectChangeAuth.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ Esys_ObjectChangeAuth_Async(
TSS2L_SYS_AUTH_COMMAND auths;
RSRC_NODE_T *objectHandleNode;
RSRC_NODE_T *parentHandleNode;
TPM2B_AUTH authCopy =
{ .size = 0,
.buffer = {}
};
TPMI_ALG_HASH hashAlg = 0;

/* Check context, sequence correctness and set state to error for now */
if (esysContext == NULL) {
Expand All @@ -169,6 +174,17 @@ Esys_ObjectChangeAuth_Async(
r = esys_GetResourceObject(esysContext, parentHandle, &parentHandleNode);
return_state_if_error(r, _ESYS_STATE_INIT, "parentHandle unknown.");

if (objectHandleNode->rsrc.rsrcType == IESYSC_KEY_RSRC) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about NV objects?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec states:
NOTE 3 If an NV Index is to have a new authorization, it is done with TPM2_NV_ChangeAuth().

hashAlg = objectHandleNode->rsrc.misc.rsrc_key_pub.publicArea.nameAlg;
}

if (newAuth) {
authCopy = *newAuth;
};

r = iesys_adapt_auth_value(&esysContext->crypto_backend, &authCopy, hashAlg);
return_state_if_error(r, _ESYS_STATE_INIT, "Adapt auth value");

/* Initial invocation of SAPI to prepare the command buffer with parameters */
r = Tss2_Sys_ObjectChangeAuth_Prepare(esysContext->sys,
(objectHandleNode == NULL)
Expand All @@ -177,7 +193,7 @@ Esys_ObjectChangeAuth_Async(
(parentHandleNode == NULL)
? TPM2_RH_NULL
: parentHandleNode->rsrc.handle,
newAuth);
&authCopy);
return_state_if_error(r, _ESYS_STATE_INIT, "SAPI Prepare returned error.");

/* Calculate the cpHash Values */
Expand Down
64 changes: 45 additions & 19 deletions src/tss2-esys/esys_iutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -1653,11 +1653,28 @@ iesys_tpm_error(TSS2_RC r)
(r & TSS2_RC_LAYER_MASK) == TSS2_RESMGR_RC_LAYER));
}

/** Remove trailing spaces includes auth value.
*
* Trailing zeros will be removed.
*
* @param[in,out] auth_value The auth value to be adapted.
*/
void iesys_strip_trailing_zeros(TPM2B_DIGEST *digest)
{
/* Remove trailing zeroes */
if (digest) {
while (digest->size > 0 &&
digest->buffer[digest->size - 1] == 0) {
digest->size--;
}
}
}

/** Replace auth value with Hash for long auth values.
/** Adapt auth value.
*
* if the size of auth value exceeds hash_size the auth value
* will be replaced with the hash of the auth value.
* Trailing zeros will be removed.
*
* @param[in,out] auth_value The auth value to be adapted.
* @param[in] hash_alg The hash alg used for adaption.
Expand All @@ -1668,37 +1685,46 @@ iesys_tpm_error(TSS2_RC r)
* computation.
*/
TSS2_RC
iesys_hash_long_auth_values(
iesys_adapt_auth_value(
ESYS_CRYPTO_CALLBACKS *crypto_cb,
TPM2B_AUTH *auth_value,
TPMI_ALG_HASH hash_alg)
{
TSS2_RC r;
TSS2_RC r = TSS2_RC_SUCCESS;
ESYS_CRYPTO_CONTEXT_BLOB *cryptoContext;
TPM2B_AUTH hash2b;
size_t hash_size;

r = iesys_crypto_hash_get_digest_size(hash_alg, &hash_size);
return_if_error(r, "Get digest size.");
/* Remove trailing zeroes */
iesys_strip_trailing_zeros(auth_value);

if (hash_alg) {
r = iesys_crypto_hash_get_digest_size(hash_alg, &hash_size);
return_if_error(r, "Get digest size.");

if (auth_value && auth_value->size > hash_size) {
/* The auth value has to be adapted. */
r = iesys_crypto_hash_start(crypto_cb,
&cryptoContext, hash_alg);
return_if_error(r, "crypto hash start");
if (auth_value && auth_value->size > hash_size) {
/* The auth value has to be adapted. */
r = iesys_crypto_hash_start(crypto_cb,
&cryptoContext, hash_alg);
return_if_error(r, "crypto hash start");

r = iesys_crypto_hash_update(crypto_cb,
cryptoContext, &auth_value->buffer[0],
auth_value->size);
goto_if_error(r, "crypto hash update", error_cleanup);
r = iesys_crypto_hash_update(crypto_cb,
cryptoContext, &auth_value->buffer[0],
auth_value->size);
goto_if_error(r, "crypto hash update", error_cleanup);

r = iesys_crypto_hash_finish(crypto_cb,
&cryptoContext, &hash2b.buffer[0], &hash_size);
goto_if_error(r, "crypto hash finish", error_cleanup);
r = iesys_crypto_hash_finish(crypto_cb,
&cryptoContext, &hash2b.buffer[0], &hash_size);
goto_if_error(r, "crypto hash finish", error_cleanup);

memcpy(&auth_value->buffer[0], &hash2b.buffer[0], hash_size);
auth_value->size = hash_size;
memcpy(&auth_value->buffer[0], &hash2b.buffer[0], hash_size);
auth_value->size = hash_size;

/* Remove trailing zeroes */
iesys_strip_trailing_zeros(auth_value);
}
}

return r;

error_cleanup:
Expand Down
5 changes: 4 additions & 1 deletion src/tss2-esys/esys_iutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,14 @@ TSS2_RC iesys_get_name(
bool iesys_tpm_error(
TSS2_RC r);

TSS2_RC iesys_hash_long_auth_values(
TSS2_RC iesys_adapt_auth_value(
ESYS_CRYPTO_CALLBACKS *crypto_cb,
TPM2B_AUTH *auth_value,
TPMI_ALG_HASH hash_alg);

void iesys_strip_trailing_zeros(
TPM2B_AUTH *auth_value);

#ifdef __cplusplus
} /* extern "C" */
#endif
Expand Down
10 changes: 8 additions & 2 deletions src/tss2-esys/esys_tr.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,14 +479,20 @@ Esys_TR_SetAuth(ESYS_CONTEXT * esys_context, ESYS_TR esys_handle,
name_alg = esys_object->rsrc.misc.rsrc_key_pub.publicArea.nameAlg;
} else if (esys_object->rsrc.rsrcType == IESYSC_NV_RSRC) {
name_alg = esys_object->rsrc.misc.rsrc_nv_pub.nvPublic.nameAlg;
} else {
name_alg = TPM2_ALG_NULL;
}
esys_object->auth = *authValue;
/* Adapt auth value to hash for large auth values. */

/* Adapt auth value. */
if (name_alg != TPM2_ALG_NULL) {
r = iesys_hash_long_auth_values(&esys_context->crypto_backend,
r = iesys_adapt_auth_value(&esys_context->crypto_backend,
&esys_object->auth, name_alg);
return_if_error(r, "Hashing overlength authValue failed.");
} else {
iesys_strip_trailing_zeros(&esys_object->auth);
}

}
return TSS2_RC_SUCCESS;
}
Expand Down
Loading
Loading