From: David Woodhouse <dwmw2@infradead.org>
Date: Sat, 8 May 2021 07:41:12 +0000 (+0100)
Subject: GnuTLS: Move TPMv1 context to certinfo
X-Git-Tag: v8.20~208
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=b9b663ff14ee16423e2b30f214dc2ffc2f33fcb5;p=users%2Fdwmw2%2Fopenconnect.git

GnuTLS: Move TPMv1 context to certinfo

If we want to support multiple certificates, the TPM context needs to
be managed per cert, not per vpninfo.

Strictly, some of this *could* be global state while some of it is
per-key. But that's an optimisation for the future.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---

diff --git a/gnutls.c b/gnutls.c
index eacf792e..52ef49ff 100644
--- a/gnutls.c
+++ b/gnutls.c
@@ -2447,7 +2447,7 @@ void openconnect_close_https(struct openconnect_info *vpninfo, int final)
 		gnutls_certificate_free_credentials(vpninfo->https_cred);
 		vpninfo->https_cred = NULL;
 #ifdef HAVE_TROUSERS
-		release_tpm1_ctx(vpninfo);
+		release_tpm1_ctx(vpninfo, &vpninfo->certinfo[0]);
 #endif
 #ifdef HAVE_TSS2
 		release_tpm2_ctx(vpninfo);
diff --git a/gnutls.h b/gnutls.h
index 5705bb7b..4aa5c547 100644
--- a/gnutls.h
+++ b/gnutls.h
@@ -26,7 +26,7 @@
 
 int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 		  gnutls_datum_t *fdata, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig);
-void release_tpm1_ctx(struct openconnect_info *info);
+void release_tpm1_ctx(struct openconnect_info *info, struct cert_info *certinfo);
 
 int load_tpm2_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 		  gnutls_datum_t *fdata, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig);
diff --git a/gnutls_tpm.c b/gnutls_tpm.c
index cb527f18..2a9ffb1c 100644
--- a/gnutls_tpm.c
+++ b/gnutls_tpm.c
@@ -43,10 +43,11 @@ struct oc_tpm1_ctx {
 };
 
 /* Signing function for TPM privkeys, set with gnutls_privkey_import_ext() */
-static int tpm_sign_fn(gnutls_privkey_t key, void *_vpninfo,
+static int tpm_sign_fn(gnutls_privkey_t key, void *_certinfo,
 		       const gnutls_datum_t *data, gnutls_datum_t *sig)
 {
-	struct openconnect_info *vpninfo = _vpninfo;
+	struct cert_info *certinfo = _certinfo;
+	struct openconnect_info *vpninfo = certinfo->vpninfo;
 	TSS_HHASH hash;
 	int err;
 
@@ -54,7 +55,7 @@ static int tpm_sign_fn(gnutls_privkey_t key, void *_vpninfo,
 		     _("TPM sign function called for %d bytes.\n"),
 		     data->size);
 
-	err = Tspi_Context_CreateObject(vpninfo->tpm1->tpm_context, TSS_OBJECT_TYPE_HASH,
+	err = Tspi_Context_CreateObject(certinfo->tpm1->tpm_context, TSS_OBJECT_TYPE_HASH,
 					TSS_HASH_OTHER, &hash);
 	if (err) {
 		vpn_progress(vpninfo, PRG_ERR,
@@ -67,13 +68,13 @@ static int tpm_sign_fn(gnutls_privkey_t key, void *_vpninfo,
 		vpn_progress(vpninfo, PRG_ERR,
 			     _("Failed to set value in TPM hash object: %s\n"),
 			     Trspi_Error_String(err));
-		Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, hash);
+		Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, hash);
 		return GNUTLS_E_PK_SIGN_FAILED;
 	}
-	err = Tspi_Hash_Sign(hash, vpninfo->tpm1->tpm_key, &sig->size, &sig->data);
-	Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, hash);
+	err = Tspi_Hash_Sign(hash, certinfo->tpm1->tpm_key, &sig->size, &sig->data);
+	Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, hash);
 	if (err) {
-		if (vpninfo->tpm1->tpm_key_policy || err != TPM_E_AUTHFAIL)
+		if (certinfo->tpm1->tpm_key_policy || err != TPM_E_AUTHFAIL)
 			vpn_progress(vpninfo, PRG_ERR,
 				     _("TPM hash signature failed: %s\n"),
 				     Trspi_Error_String(err));
@@ -101,7 +102,7 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 			     gnutls_strerror(err));
 		return -EINVAL;
 	}
-	vpninfo->tpm1 = calloc(1, sizeof(*vpninfo->tpm1));
+	certinfo->tpm1 = calloc(1, sizeof(*certinfo->tpm1));
 	/* Ick. We have to parse the ASN1 OCTET_STRING for ourselves. */
 	if (asn1.size < 2 || asn1.data[0] != 0x04 /* OCTET_STRING */) {
 		vpn_progress(vpninfo, PRG_ERR,
@@ -133,29 +134,29 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 		goto out_blob;
 	}
 
-	err = Tspi_Context_Create(&vpninfo->tpm1->tpm_context);
+	err = Tspi_Context_Create(&certinfo->tpm1->tpm_context);
 	if (err) {
 		vpn_progress(vpninfo, PRG_ERR,
 			     _("Failed to create TPM context: %s\n"),
 			     Trspi_Error_String(err));
 		goto out_blob;
 	}
-	err = Tspi_Context_Connect(vpninfo->tpm1->tpm_context, NULL);
+	err = Tspi_Context_Connect(certinfo->tpm1->tpm_context, NULL);
 	if (err) {
 		vpn_progress(vpninfo, PRG_ERR,
 			     _("Failed to connect TPM context: %s\n"),
 			     Trspi_Error_String(err));
 		goto out_context;
 	}
-	err = Tspi_Context_LoadKeyByUUID(vpninfo->tpm1->tpm_context, TSS_PS_TYPE_SYSTEM,
-					 SRK_UUID, &vpninfo->tpm1->srk);
+	err = Tspi_Context_LoadKeyByUUID(certinfo->tpm1->tpm_context, TSS_PS_TYPE_SYSTEM,
+					 SRK_UUID, &certinfo->tpm1->srk);
 	if (err) {
 		vpn_progress(vpninfo, PRG_ERR,
 			     _("Failed to load TPM SRK key: %s\n"),
 			     Trspi_Error_String(err));
 		goto out_context;
 	}
-	err = Tspi_GetPolicyObject(vpninfo->tpm1->srk, TSS_POLICY_USAGE, &vpninfo->tpm1->srk_policy);
+	err = Tspi_GetPolicyObject(certinfo->tpm1->srk, TSS_POLICY_USAGE, &certinfo->tpm1->srk_policy);
 	if (err) {
 		vpn_progress(vpninfo, PRG_ERR,
 			     _("Failed to load TPM SRK policy object: %s\n"),
@@ -170,11 +171,11 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 
 		/* We don't seem to get the error here... */
 		if (pass)
-			err = Tspi_Policy_SetSecret(vpninfo->tpm1->srk_policy,
+			err = Tspi_Policy_SetSecret(certinfo->tpm1->srk_policy,
 						    TSS_SECRET_MODE_PLAIN,
 						    strlen(pass), (BYTE *)pass);
 		else /* Well-known NULL key */
-			err = Tspi_Policy_SetSecret(vpninfo->tpm1->srk_policy,
+			err = Tspi_Policy_SetSecret(certinfo->tpm1->srk_policy,
 						    TSS_SECRET_MODE_SHA1,
 						    sizeof(nullpass), (BYTE *)nullpass);
 		if (err) {
@@ -187,9 +188,9 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 		free_pass(&pass);
 
 		/* ... we get it here instead. */
-		err = Tspi_Context_LoadKeyByBlob(vpninfo->tpm1->tpm_context, vpninfo->tpm1->srk,
+		err = Tspi_Context_LoadKeyByBlob(certinfo->tpm1->tpm_context, certinfo->tpm1->srk,
 						 tss_len, asn1.data + ofs,
-						 &vpninfo->tpm1->tpm_key);
+						 &certinfo->tpm1->tpm_key);
 		if (!err)
 			break;
 
@@ -211,24 +212,24 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 	/* This would be nicer if there was a destructor callback. I could
 	   allocate a data structure with the TPM handles and the vpninfo
 	   pointer, and destroy that properly when the key is destroyed. */
-	gnutls_privkey_import_ext(*pkey, GNUTLS_PK_RSA, vpninfo, tpm_sign_fn, NULL, 0);
+	gnutls_privkey_import_ext(*pkey, GNUTLS_PK_RSA, certinfo, tpm_sign_fn, NULL, 0);
 
  retry_sign:
 	err = gnutls_privkey_sign_data(*pkey, GNUTLS_DIG_SHA1, 0, fdata, pkey_sig);
 	if (err == GNUTLS_E_INSUFFICIENT_CREDENTIALS) {
-		if (!vpninfo->tpm1->tpm_key_policy) {
-			err = Tspi_Context_CreateObject(vpninfo->tpm1->tpm_context,
+		if (!certinfo->tpm1->tpm_key_policy) {
+			err = Tspi_Context_CreateObject(certinfo->tpm1->tpm_context,
 							TSS_OBJECT_TYPE_POLICY,
 							TSS_POLICY_USAGE,
-							&vpninfo->tpm1->tpm_key_policy);
+							&certinfo->tpm1->tpm_key_policy);
 			if (err) {
 				vpn_progress(vpninfo, PRG_ERR,
 					     _("Failed to create key policy object: %s\n"),
 					     Trspi_Error_String(err));
 				goto out_key;
 			}
-			err = Tspi_Policy_AssignToObject(vpninfo->tpm1->tpm_key_policy,
-							 vpninfo->tpm1->tpm_key);
+			err = Tspi_Policy_AssignToObject(certinfo->tpm1->tpm_key_policy,
+							 certinfo->tpm1->tpm_key);
 			if (err) {
 				vpn_progress(vpninfo, PRG_ERR,
 					     _("Failed to assign policy to key: %s\n"),
@@ -241,7 +242,7 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 		if (err)
 			goto out_key_policy;
 
-		err = Tspi_Policy_SetSecret(vpninfo->tpm1->tpm_key_policy,
+		err = Tspi_Policy_SetSecret(certinfo->tpm1->tpm_key_policy,
 					    TSS_SECRET_MODE_PLAIN,
 					    strlen(pass), (void *)pass);
 		free_pass(&pass);
@@ -258,53 +259,53 @@ int load_tpm1_key(struct openconnect_info *vpninfo, struct cert_info *certinfo,
 	free(asn1.data);
 	return 0;
  out_key_policy:
-	Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->tpm_key_policy);
-	vpninfo->tpm1->tpm_key_policy = 0;
+	Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->tpm_key_policy);
+	certinfo->tpm1->tpm_key_policy = 0;
  out_key:
-	Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->tpm_key);
-	vpninfo->tpm1->tpm_key = 0;
+	Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->tpm_key);
+	certinfo->tpm1->tpm_key = 0;
  out_srkpol:
-	Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->srk_policy);
-	vpninfo->tpm1->srk_policy = 0;
+	Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->srk_policy);
+	certinfo->tpm1->srk_policy = 0;
  out_srk:
-	Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->srk);
-	vpninfo->tpm1->srk = 0;
+	Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->srk);
+	certinfo->tpm1->srk = 0;
  out_context:
-	Tspi_Context_Close(vpninfo->tpm1->tpm_context);
-	vpninfo->tpm1->tpm_context = 0;
+	Tspi_Context_Close(certinfo->tpm1->tpm_context);
+	certinfo->tpm1->tpm_context = 0;
  out_blob:
 	free(asn1.data);
-	free(vpninfo->tpm1);
-	vpninfo->tpm1 = NULL;
+	free(certinfo->tpm1);
+	certinfo->tpm1 = NULL;
 	return -EIO;
 }
 
-void release_tpm1_ctx(struct openconnect_info *vpninfo)
+void release_tpm1_ctx(struct openconnect_info *vpninfo, struct cert_info *certinfo)
 {
-	if (!vpninfo->tpm1)
+	if (!certinfo->tpm1)
 		return;
 
-	if (vpninfo->tpm1->tpm_key_policy) {
-		Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->tpm_key_policy);
-		vpninfo->tpm1->tpm_key = 0;
+	if (certinfo->tpm1->tpm_key_policy) {
+		Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->tpm_key_policy);
+		certinfo->tpm1->tpm_key = 0;
 	}
-	if (vpninfo->tpm1->tpm_key) {
-		Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->tpm_key);
-		vpninfo->tpm1->tpm_key = 0;
+	if (certinfo->tpm1->tpm_key) {
+		Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->tpm_key);
+		certinfo->tpm1->tpm_key = 0;
 	}
-	if (vpninfo->tpm1->srk_policy) {
-		Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->srk_policy);
-		vpninfo->tpm1->srk_policy = 0;
+	if (certinfo->tpm1->srk_policy) {
+		Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->srk_policy);
+		certinfo->tpm1->srk_policy = 0;
 	}
-	if (vpninfo->tpm1->srk) {
-		Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->srk);
-		vpninfo->tpm1->srk = 0;
+	if (certinfo->tpm1->srk) {
+		Tspi_Context_CloseObject(certinfo->tpm1->tpm_context, certinfo->tpm1->srk);
+		certinfo->tpm1->srk = 0;
 	}
-	if (vpninfo->tpm1->tpm_context) {
-		Tspi_Context_Close(vpninfo->tpm1->tpm_context);
-		vpninfo->tpm1->tpm_context = 0;
+	if (certinfo->tpm1->tpm_context) {
+		Tspi_Context_Close(certinfo->tpm1->tpm_context);
+		certinfo->tpm1->tpm_context = 0;
 	}
-	free(vpninfo->tpm1);
-	vpninfo->tpm1 = NULL;
+	free(certinfo->tpm1);
+	certinfo->tpm1 = NULL;
 };
 #endif /* HAVE_TROUSERS */
diff --git a/openconnect-internal.h b/openconnect-internal.h
index 79ceeb18..96990a92 100644
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -416,6 +416,9 @@ struct cert_info {
 	char *cert;
 	char *key;
 	char *password;
+#if defined(OPENCONNECT_GNUTLS) && defined(HAVE_TROUSERS)
+	struct oc_tpm1_ctx *tpm1;
+#endif
 };
 
 struct openconnect_info {
@@ -583,9 +586,6 @@ struct openconnect_info {
 	gnutls_certificate_credentials_t https_cred;
 	gnutls_psk_client_credentials_t psk_cred;
 	char local_cert_md5[MD5_SIZE * 2 + 1]; /* For CSD */
-#ifdef HAVE_TROUSERS
-	struct oc_tpm1_ctx *tpm1;
-#endif
 #ifdef HAVE_TSS2
 	struct oc_tpm2_ctx *tpm2;
 #endif