pinentry fails for tpm protected key

[James Bottomley]

On Mon, 2021-12-27 at 14:46 -0700, Joshua Rubin via Gnupg-devel wrote:
> > I think you're going to have to be a lot more specific.  I use gpg-
> > agent with pinentry and tpm keys and it works fine for me on
> > openSUSE
> > and gpg-2.3.4.
> 
> I'm using gpg-2.3.4 and pinentry 1.2.0. I _only_ have this issue when
> I select that I want to save the passphrase in the keyring, it works
> otherwise (which I think is what it sounds like you are doing). Happy
> to provide any other useful details, just not sure what you might
> need.

Based on this, my best guess is that whatever is on the other end of
libsecret doesn't like binary key grips.  There's no harm in converting
them all to ASCII, does this fix your problem?

James

---

>From 7af7213246a7cf085cdf42d1f79abf0d6333ed30 Mon Sep 17 00:00:00 2001
From: James Bottomley <James.Bottomley at HansenPartnership.com>
Date: Wed, 29 Dec 2021 11:58:16 -0500
Subject: [PATCH] agent: always use hexgrip when storing key password

The current code uses the binary ctrl->keygrip, but all the passphrase
storage engines expect this to be a string, so convert the binary
keygrip to a hex one before passing it in as the keyid.  This fixes a
crash seen in some libsecret implementations where a non-ascii keyid
isn't well handled.

Signed-off-by: James Bottomley <James.Bottomley at HansenPartnership.com>
---
 agent/call-tpm2d.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/agent/call-tpm2d.c b/agent/call-tpm2d.c
index 6fae5d85a..1048c7d63 100644
--- a/agent/call-tpm2d.c
+++ b/agent/call-tpm2d.c
@@ -141,14 +141,17 @@ agent_tpm2d_writekey (ctrl_t ctrl, unsigned char **shadow_info,
 static gpg_error_t
 pin_cb (ctrl_t ctrl, const char *prompt, char **passphrase)
 {
-  *passphrase = agent_get_cache (ctrl, ctrl->keygrip, CACHE_MODE_USER);
+  char hexgrip[2*KEYGRIP_LEN + 1];
+
+  bin2hex (ctrl->keygrip, KEYGRIP_LEN, hexgrip);
+  *passphrase = agent_get_cache (ctrl, hexgrip, CACHE_MODE_USER);
   if (*passphrase)
     return 0;
   return agent_get_passphrase(ctrl, passphrase,
 			      _("Please enter your passphrase, so that the "
 				"secret key can be unlocked for this session"),
 			      prompt, NULL, 0,
-			      ctrl->keygrip, CACHE_MODE_USER, NULL);
+			      hexgrip, CACHE_MODE_USER, NULL);
 }
 
 int
@@ -160,6 +163,7 @@ agent_tpm2d_pksign (ctrl_t ctrl, const unsigned char *digest,
   char line[ASSUAN_LINELENGTH];
   membuf_t data;
   struct inq_parm_s inqparm;
+  char hexgrip[2*KEYGRIP_LEN + 1];
 
   rc = start_tpm2d (ctrl);
   if (rc)
@@ -183,7 +187,10 @@ agent_tpm2d_pksign (ctrl_t ctrl, const unsigned char *digest,
 			inq_extra, &inqparm,
 			NULL, NULL);
   if (!rc)
-    agent_put_cache (ctrl, ctrl->keygrip, CACHE_MODE_USER, inqparm.pin, 0);
+    {
+      bin2hex (ctrl->keygrip, KEYGRIP_LEN, hexgrip);
+      agent_put_cache (ctrl, hexgrip, CACHE_MODE_USER, inqparm.pin, 0);
+    }
 
   xfree (inqparm.pin);
 
@@ -208,6 +215,7 @@ agent_tpm2d_pkdecrypt (ctrl_t ctrl, const unsigned char *cipher,
   char line[ASSUAN_LINELENGTH];
   membuf_t data;
   struct inq_parm_s inqparm;
+  char hexgrip[2*KEYGRIP_LEN + 1];
 
   rc = start_tpm2d (ctrl);
   if (rc)
@@ -231,7 +239,10 @@ agent_tpm2d_pkdecrypt (ctrl_t ctrl, const unsigned char *cipher,
 			inq_extra, &inqparm,
 			NULL, NULL);
   if (!rc)
-    agent_put_cache (ctrl, ctrl->keygrip, CACHE_MODE_USER, inqparm.pin, 0);
+    {
+      bin2hex (ctrl->keygrip, KEYGRIP_LEN, hexgrip);
+      agent_put_cache (ctrl, hexgrip, CACHE_MODE_USER, inqparm.pin, 0);
+    }
 
   xfree (inqparm.pin);
 
-- 
2.26.2