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

PCR policy authmodel #833

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
4 changes: 2 additions & 2 deletions .ci/docker.run
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fi
# avoid error checking or cause make distcheck to fail. So run a
# pure check with gcc before adding those flags.
if [[ "$CC" != clang* ]]; then
./configure --enable-esapi-session-manage-flags --disable-fapi --enable-unit --enable-integration
./configure --enable-esapi-session-manage-flags --without-fapi --enable-unit --enable-integration
make distcheck TESTS=
make distclean
fi
Expand All @@ -84,7 +84,7 @@ if [[ "$CC" != clang* ]]; then
# rebuild after running scan-build.
fi

../configure --enable-unit --enable-integration --enable-esapi-session-manage-flags --enable-fapi $config_flags
../configure --enable-unit --enable-integration --enable-esapi-session-manage-flags --with-fapi $config_flags
make -j$(nproc)
make -j check

Expand Down
6 changes: 4 additions & 2 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ ACLOCAL_AMFLAGS = -I m4 --install
AM_CFLAGS = $(INCLUDE_DIRS) $(EXTRA_CFLAGS) $(CODE_COVERAGE_CFLAGS) \
$(TSS2_ESYS_CFLAGS) $(TSS2_MU_CFLAGS) $(TSS2_TCTILDR_CFLAGS) \
$(TSS2_RC_CFLAGS) $(SQLITE3_CFLAGS) $(PTHREAD_CFLAGS) \
$(CRYPTO_CFLAGS) $(YAML_CFLAGS) $(TSS2_FAPI_CFLAGS)
$(CRYPTO_CFLAGS) $(YAML_CFLAGS) $(TSS2_FAPI_CFLAGS) \
$(TSS2_POLICY_CFLAGS)

AM_LDFLAGS = $(EXTRA_LDFLAGS) $(CODE_COVERAGE_LIBS) $(TSS2_ESYS_LIBS) \
$(TSS2_MU_LIBS) $(TSS2_TCTILDR_LIBS) $(TSS2_RC_LIBS) \
$(SQLITE3_LIBS) $(PTHREAD_LIBS) $(CRYPTO_LIBS) $(YAML_LIBS) $(TSS2_FAPI_LIBS)
$(SQLITE3_LIBS) $(PTHREAD_LIBS) $(CRYPTO_LIBS) $(YAML_LIBS) \
$(TSS2_FAPI_LIBS) $(TSS2_POLICY_LIBS)

check-programs: $(check_PROGRAMS)

Expand Down
23 changes: 23 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,29 @@ PKG_CHECK_MODULES([TSS2_MU], [tss2-mu])
PKG_CHECK_MODULES([TSS2_TCTILDR], [tss2-tctildr])
PKG_CHECK_MODULES([TSS2_RC], [tss2-rc])

AC_ARG_WITH(
[policy],
[AS_HELP_STRING([--with-policy],
[enable or disable policy support. Default is "auto" to autodetect])],
[enable_policy=$withval],
[enable_policy=auto])

AC_DEFUN([do_policy_configure], [
# tss2-policy was added in tpm2-tss v4.0
AS_IF([test "x$enable_policy" = "xauto"],
[ PKG_CHECK_MODULES([TSS2_POLICY], [tss2-policy], [have_policy=1], [have_policy=0]) ],
[ PKG_CHECK_MODULES([TSS2_POLICY], [tss2-policy], [have_policy=1]) ]
)
])

AS_IF([test "x$enable_policy" != "xno"],
[do_policy_configure])

AS_IF([test "$have_policy" = "1"], [
AC_DEFINE([HAVE_POLICY], [1], [Should respect policies and libpolicy is found.])
AC_SUBST(TSS2_POLICY_DEP, [tss2-policy])
])

# Macro that checks for existence of a python module
AC_DEFUN([AC_PYTHON_MODULE],
[AC_MSG_CHECKING([for module $2 in python])
Expand Down
2 changes: 1 addition & 1 deletion docs/BUILDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The project depends on:
1. [gcc](https://www.gnu.org/software/gcc/)
2. [clang](https://clang.llvm.org/)
4. [SQLite3](https://www.sqlite.org/)
5. [tpm2-tss](https://github.com/tpm2-software/tpm2-tss): **Requires >= 2.0, recommended >= 2.3.0**
5. [tpm2-tss](https://github.com/tpm2-software/tpm2-tss): **Requires >= 2.0, recommended >= 4.0.1**
6. A Resource Manager, one of:
- [tpm2-abrmd](https://github.com/tpm2-software/tpm2-abrmd): **Requires version >= 2.1.0**
- Linux kernel version >= 4.12 for `/dev/tpmrm[0-9]+` nodes.
Expand Down
2 changes: 1 addition & 1 deletion lib/tpm2-pkcs11.pc.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Name: tpm2-pkcs11
Description: TPM2 PKCS#11 library
URL: https://github.com/tpm2-software/tpm2-pkcs11
Version: @VERSION@
Requires.private: tss2-esys tss2-mu sqlite3 libcrypto
Requires.private: @TSS2_POLICY_DEP@ tss2-esys tss2-mu sqlite3 libcrypto
Cflags: @PTHREAD_CFLAGS@
Libs: -L${p11_module_path} -ltpm2_pkcs11
Libs.private: @PTHREAD_LIBS@
1 change: 1 addition & 0 deletions src/lib/attrs.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ static attr_handler2 attr_handlers[] = {
ADD_ATTR_HANDLER(CKA_TPM2_PUB_BLOB, TYPE_BYTE_HEX_STR),
ADD_ATTR_HANDLER(CKA_TPM2_PRIV_BLOB, TYPE_BYTE_HEX_STR),
ADD_ATTR_HANDLER(CKA_TPM2_ENC_BLOB, TYPE_BYTE_HEX_STR),
ADD_ATTR_HANDLER(CKA_TPM2_POLICY_JSON, TYPE_BYTE_HEX_STR),
};

static attr_handler2 default_handler = { .memtype = 0, .name="UNKNOWN" };
Expand Down
1 change: 1 addition & 0 deletions src/lib/attrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#define CKA_TPM2_PUB_BLOB (CKA_VENDOR_DEFINED|CKA_VENDOR_TPM2_DEFINED|0x2UL)
#define CKA_TPM2_PRIV_BLOB (CKA_VENDOR_DEFINED|CKA_VENDOR_TPM2_DEFINED|0x3UL)
#define CKA_TPM2_ENC_BLOB (CKA_VENDOR_DEFINED|CKA_VENDOR_TPM2_DEFINED|0x4UL)
#define CKA_TPM2_POLICY_JSON (CKA_VENDOR_DEFINED|CKA_VENDOR_TPM2_DEFINED|0x5UL)

/* Invalid values for error detection */
#define CK_OBJECT_CLASS_BAD (~(CK_OBJECT_CLASS)0)
Expand Down
50 changes: 49 additions & 1 deletion src/lib/sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
#include <openssl/ec.h>
#include <openssl/ecdsa.h>
#include <openssl/evp.h>
#include <openssl/rsa.h>
#include <openssl/rsa.h>

#ifdef HAVE_POLICY
#include <tss2/tss2_policy.h>
#include <tss2/tss2_rc.h>
#endif

#include "attrs.h"
#include "backend.h"
Expand Down Expand Up @@ -149,6 +154,42 @@ static CK_RV update_pss_sig_state(token *tok, tobject *tobj) {
return rv;
}

static CK_RV policy_is_satisfied(tpm_ctx *tctx, tobject *tobj, uint32_t handle) {

CK_ATTRIBUTE_PTR policy_attr = attr_get_attribute_by_type(tobj->attrs, CKA_TPM2_POLICY_JSON);
if (!policy_attr) {
/* nonexistent policy is always satisfied */
return CKR_OK;
}

#ifndef HAVE_POLICY
UNUSED(tctx);
UNUSED(handle);
/* the policy is considered to be satisfied, but will warn about it */
LOGW("Found a policy, but support for enforcing it wasn't compiled in");
return CKR_OK;
#else
Copy link
Member

Choose a reason for hiding this comment

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

I would create 2 versions of this function and ifdef between the two and avoid even looking for the policy on non-policy enabled builds.

TSS2_RC rc;

TSS2_POLICY_CTX *policy_ctx = NULL;
rc = Tss2_PolicyInit(policy_attr->pValue, TPM2_ALG_SHA256, &policy_ctx);
if (rc != TSS2_RC_SUCCESS) {
LOGE("Tss2_PolicyInit: %s:", Tss2_RC_Decode(rc));
return CKR_GENERAL_ERROR;
}

CK_RV rv = tpm2_execute_policy(tctx, policy_ctx, handle);
if (rv != CKR_OK) {
LOGE("Could not execute policy.");
Tss2_PolicyFinalize(&policy_ctx);
return CKR_GENERAL_ERROR;
}

Tss2_PolicyFinalize(&policy_ctx);
return CKR_OK;
#endif
}

static CK_RV common_init(operation op, session_ctx *ctx, CK_MECHANISM_PTR mechanism, CK_OBJECT_HANDLE key) {

check_pointer(mechanism);
Expand Down Expand Up @@ -211,6 +252,13 @@ static CK_RV common_init(operation op, session_ctx *ctx, CK_MECHANISM_PTR mechan
return rv;
}

if (op == operation_sign) {
rv = policy_is_satisfied(tok->tctx, tobj, tok->pobject.handle);
if (rv != CKR_OK) {
return rv;
}
}

tpm_op_data *tpm_opdata = NULL;
if (op == operation_sign || is_hmac) {

Expand Down
108 changes: 108 additions & 0 deletions src/lib/tpm.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
#include <tss2/tss2_mu.h>
#include <tss2/tss2_rc.h>
#include <tss2/tss2_tctildr.h>
#ifdef HAVE_POLICY
#include <tss2/tss2_policy.h>
#endif

#include "attrs.h"
#include "checks.h"
Expand Down Expand Up @@ -4008,6 +4011,111 @@ CK_RV tpm2_getmechanisms(tpm_ctx *ctx, CK_MECHANISM_TYPE *mechanism_list, CK_ULO
return rv;
}

#ifdef HAVE_POLICY
static TSS2_RC tpm2_policy_get_pcr(TSS2_POLICY_PCR_SELECTION *selection,
TPML_PCR_SELECTION *out_selection,
TPML_DIGEST *out_digest,
void *userdata)
{

TPML_PCR_SELECTION in_pcr_selection = {0};
if (selection->type == TSS2_POLICY_PCR_SELECTOR_PCR_SELECTION) {
in_pcr_selection = selection->selections.pcr_selection;
} else {
in_pcr_selection.count = 1;

TPMS_PCR_SELECTION *pcr_bank = &in_pcr_selection.pcrSelections[0];
TPMS_PCR_SELECT *pcr_select = &selection->selections.pcr_select;

pcr_bank->hash = TPM2_ALG_SHA256;
pcr_bank->sizeofSelect = pcr_select->sizeofSelect;
memcpy(pcr_bank->pcrSelect, pcr_select->pcrSelect, pcr_bank->sizeofSelect);
}

ESYS_CONTEXT *esys_ctx = userdata;

UINT32 pcr_update_counter;
TPML_PCR_SELECTION *pcr_selection = NULL;
TPML_DIGEST *pcr_values = NULL;

TSS2_RC rc = Esys_PCR_Read(esys_ctx,
ESYS_TR_NONE,
ESYS_TR_NONE,
ESYS_TR_NONE,
&in_pcr_selection,
&pcr_update_counter,
&pcr_selection,
&pcr_values);
if (rc != TSS2_RC_SUCCESS) {
LOGE("Esys_PCR_Read: %s:", Tss2_RC_Decode(rc));
free(pcr_selection);
free(pcr_values);
Copy link
Member

Choose a reason for hiding this comment

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

These should be ESYS_Free calls.

return rc;
}

*out_selection = *pcr_selection;
*out_digest = *pcr_values;
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I would have the failure Esys_PCR_Read goto an out label that Esys_Free's all the allocated pointers and here where you transfer ownership to out_selection and out_digest set the pcr_selection and pcr_values to NULL and still pass them to Esys_Free as NULL is ok to pass and then return an rv variable that gets tripped to success before the out label and is initialized to CKR_GENERAL_ERROR or whatever makes sense.


free(pcr_selection);
free(pcr_values);
return TSS2_RC_SUCCESS;
}

CK_RV tpm2_execute_policy(tpm_ctx *ctx, TSS2_POLICY_CTX *policy_ctx, uint32_t handle)
{

check_pointer(ctx);
check_pointer(policy_ctx);
check_num(handle);

TPMT_SYM_DEF symmetric = {
.algorithm = TPM2_ALG_AES,
.keyBits = { .aes = 128 },
.mode = { .aes = TPM2_ALG_CFB }
};

TSS2_POLICY_CALC_CALLBACKS calc_callbacks = {0};
calc_callbacks.cbpcr = &tpm2_policy_get_pcr;
calc_callbacks.cbpcr_userdata = ctx->esys_ctx;

TSS2_RC rc;

rc = Tss2_PolicySetCalcCallbacks(policy_ctx, &calc_callbacks);
if (rc != TSS2_RC_SUCCESS) {
LOGE("Tss2_PolicySetCalcCallbacks: %s:", Tss2_RC_Decode(rc));
return CKR_GENERAL_ERROR;
}

/* XXX should we cache the session or running multiple policies is unlikely? */
Copy link
Member

Choose a reason for hiding this comment

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

This session handle needs to be used to authorize the object for Esys_Sign. You're essentially still on the password model (see my other comment in the review summary). I would modify the tpm_ctx struct to have two session handles:

  • auth_session: This is the session that will be either the HMAC session OR the policy session from execute policy.
    • For HMAC session use, it can just live the lifetime of the token.
    • For POLICY session use, it needs to be created for each command.
      • You can use the attribute continue session and use policy reset command, but thats a performance thing that can be done later.
  • enc_session: This is important, In the current code the HMAC session handle is also the encryption handle for protecting the traffic to and from the TPM, we need this to also be created for the lifetime of the token and passed to each command as the first unused session handle, which in most commands in session2, sometimes its session3.

ESYS_TR policy_session = ESYS_TR_NONE;
rc = Esys_StartAuthSession(ctx->esys_ctx,
handle,
handle,
ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE,
NULL,
TPM2_SE_POLICY, &symmetric, TPM2_ALG_SHA256,
&policy_session);
if (rc != TSS2_RC_SUCCESS) {
LOGE("Esys_StartAuthSession: %s", Tss2_RC_Decode(rc));
return CKR_GENERAL_ERROR;
}

TSS2_RC result = Tss2_PolicyExecute(policy_ctx, ctx->esys_ctx, policy_session);
if (result != TSS2_RC_SUCCESS) {
LOGE("Tss2_PolicyExecute: %s:", Tss2_RC_Decode(result));
/* continue and stop the session */
}

rc = Esys_FlushContext(ctx->esys_ctx, policy_session);
if (rc != TSS2_RC_SUCCESS) {
LOGE("Esys_FlushContext: %s", Tss2_RC_Decode(rc));
return CKR_GENERAL_ERROR;
}

return result;
}
#endif

void tpm_init(void) {
/* pass nothing to do */
}
Expand Down
8 changes: 8 additions & 0 deletions src/lib/tpm.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
#include <stdint.h>

#include <tss2/tss2_esys.h>
#ifdef HAVE_POLICY
#include <tss2/tss2_policy.h>
#endif

#include "attrs.h"
#include "debug.h"
Expand Down Expand Up @@ -189,6 +192,11 @@ CK_RV tpm2_generate_key(

CK_RV tpm2_getmechanisms(tpm_ctx *ctx, CK_MECHANISM_TYPE *mechanism_list, CK_ULONG_PTR count);

/* the function can't be defined without the library because of TSS2_POLICY_CTX type */
#ifdef HAVE_POLICY
CK_RV tpm2_execute_policy(tpm_ctx *ctx, TSS2_POLICY_CTX *policy_ctx, uint32_t handle);
#endif

CK_RV tpm_get_existing_primary(tpm_ctx *tpm, uint32_t *primary_handle, twist *primary_blob);

CK_RV tpm_create_persistent_primary(tpm_ctx *tpm, uint32_t *primary_handle, twist *primary_blob);
Expand Down
Loading