Mailing List Archive

On Sun, 2023-06-18 at 16:36 +0200, Maxime Ripard wrote:

> [...]
> > > This series fixes some of the issues, but the tests still do not
> > > runproperly with the following error:
> > >
> > > Making check in tpm2dtests
> > > make[2]: Entering directory '/var/home/max/gnupg2/gnupg-
> > > 2.4.2/tests/tpm2dtests'
> > > LC_ALL=C EXEEXT=
> > > PATH="../gpgscm:/var/home/max/.cache/cabal//bin:/var/home/max/.lo
> > > cal/
> > > share/cargo/bin:/var/home/max/.local/bin:/var/home/max/.local/sha
> > > re/f
> > > latpak/exports/bin:/var/lib/flatpak/exports/bin:/usr/local/sbin:/
> > > usr/
> > > local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
> > > abs_top_srcdir="/var/home/max/gnupg2/gnupg-2.4.2"
> > > objdir="/var/home/max/gnupg2/gnupg-2.4.2" TSS2_LOG=all+DEBUG
> > > TSS2_LOGFILE=tss2.log TPMSERVER="" SWTPM="/bin/swtpm"
> > > SWTPM_IOCTL=""
> > > GNUPG_BUILD_ROOT="/var/home/max/gnupg2/gnupg-2.4.2"
> > > GNUPG_IN_TEST_SUITE=fact GPGSCM_PATH="/var/home/max/gnupg2/gnupg-
> > > 2.4.2/tests/gpgscm"
> > > TPM2TOOLS_TCTI="swtpm:host=localhost,port=2321"
> > > /var/home/max/gnupg2/gnupg-2.4.2/tests/gpgscm/gpgscm \
> > >   /var/home/max/gnupg2/gnupg-2.4.2/tests/tpm2dtests/run-
> > > tests.scm 
> >
> > But now I try to run it with make -C tests/tpm2dtests, it fails
> > with:
> >
> > make: Entering directory '/home/jejb/git/gnupg/tests/tpm2dtests'
> > LC_ALL=C EXEEXT=
> > PATH="../gpgscm:/home/jejb/.cargo/bin:/home/jejb/bin:/usr/local/bin
> > :/bi
> > n:/usr/bin:/sbin:/usr/sbin:/usr/local/sbin:/usr/etc:/etc:/home/jejb
> > /and
> > roid/android-sdk-linux_x86/platform-
> > tools:/home/jejb/android/android-
> > sdk-linux_x86/tools" abs_top_srcdir="/home/jejb/git/gnupg"
> > objdir="/home/jejb/git/gnupg"
> > TPMSERVER="/usr/lib/ibmtss/tpm_server"
> > SWTPM="" SWTPM_IOCTL=""
> > GNUPG_BUILD_ROOT="/home/jejb/git/gnupg/tests"
> > GNUPG_IN_TEST_SUITE=fact
> > GPGSCM_PATH="/home/jejb/git/gnupg/tests/gpgscm"
> > /home/jejb/git/gnupg/tests/gpgscm/gpgscm \
> >   /home/jejb/git/gnupg/tests/tpm2dtests/run-tests.scm 
> > /home/jejb/git/gnupg/tests/tpm2dtests/run-tests.scm:30: not enough
> > arguments, missing: (path . args)
>
> The patches in this series should solve this.

Even with the patches, I'm now getting a different error:

jejb@lingrow:~/git/gnupg> make -C tests/tpm2dtests check
make: Entering directory '/home/jejb/git/gnupg/tests/tpm2dtests'
LC_ALL=C EXEEXT=
PATH="../gpgscm:/home/jejb/.cargo/bin:/home/jejb/bin:/usr/local/bin:/bi
n:/usr/bin:/sbin:/usr/sbin:/usr/local/sbin:/usr/etc:/etc:/home/jejb/and
roid/android-sdk-linux_x86/platform-tools:/home/jejb/android/android-
sdk-linux_x86/tools" abs_top_srcdir="/home/jejb/git/gnupg"
objdir="/home/jejb/git/gnupg" TPMSERVER="/usr/lib/ibmtss/tpm_server"
SWTPM="" SWTPM_IOCTL="" GNUPG_BUILD_ROOT="/home/jejb/git/gnupg/tests"
GNUPG_IN_TEST_SUITE=fact
GPGSCM_PATH="/home/jejb/git/gnupg/tests/gpgscm"
/home/jejb/git/gnupg/tests/gpgscm/gpgscm \
/home/jejb/git/gnupg/tests/tpm2dtests/run-tests.scm

0: tests.scm:121: (throw (:stderr result))
1: defs.scm:148: (call-popen `(,(tool-hardcoded 'gpgconf) ,@(if *win32*
(list '--build-prefix (getenv "objdir")) '()) ,@args) input)
2: defs.scm:146: (gpg-conf' "" args)
3: #<CLOSURE>
4: defs.scm:189: (apply gpg-conf '(--list-components))
FAIL: tests/openpgp/setup.scm
Setup failed.
make: *** [Makefile:632: xcheck] Error 1
make: Leaving directory '/home/jejb/git/gnupg/tests/tpm2dtests'

I know this means something is missing from the setup, but I can't
figure out what.

James

On Mon, 2023-06-19 at 18:22 +0200, Maxime Ripard wrote:
> On Mon, Jun 19, 2023 at 11:49:15AM -0400, James Bottomley wrote:
> > On Sun, 2023-06-18 at 16:36 +0200, Maxime Ripard wrote:
> >
> > > [...]
> > > > > This series fixes some of the issues, but the tests still do
> > > > > not
> > > > > runproperly with the following error:
> > > > >
> > > > > Making check in tpm2dtests
> > > > > make[2]: Entering directory '/var/home/max/gnupg2/gnupg-
> > > > > 2.4.2/tests/tpm2dtests'
> > > > > LC_ALL=C EXEEXT=
> > > > > PATH="../gpgscm:/var/home/max/.cache/cabal//bin:/var/home/max
> > > > > /.lo
> > > > > cal/
> > > > > share/cargo/bin:/var/home/max/.local/bin:/var/home/max/.local
> > > > > /sha
> > > > > re/f
> > > > > latpak/exports/bin:/var/lib/flatpak/exports/bin:/usr/local/sb
> > > > > in:/
> > > > > usr/
> > > > > local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
> > > > > abs_top_srcdir="/var/home/max/gnupg2/gnupg-2.4.2"
> > > > > objdir="/var/home/max/gnupg2/gnupg-2.4.2" TSS2_LOG=all+DEBUG
> > > > > TSS2_LOGFILE=tss2.log TPMSERVER="" SWTPM="/bin/swtpm"
> > > > > SWTPM_IOCTL=""
> > > > > GNUPG_BUILD_ROOT="/var/home/max/gnupg2/gnupg-2.4.2"
> > > > > GNUPG_IN_TEST_SUITE=fact
> > > > > GPGSCM_PATH="/var/home/max/gnupg2/gnupg-
> > > > > 2.4.2/tests/gpgscm"
> > > > > TPM2TOOLS_TCTI="swtpm:host=localhost,port=2321"
> > > > > /var/home/max/gnupg2/gnupg-2.4.2/tests/gpgscm/gpgscm \
> > > > >   /var/home/max/gnupg2/gnupg-2.4.2/tests/tpm2dtests/run-
> > > > > tests.scm 
> > > >
> > > > But now I try to run it with make -C tests/tpm2dtests, it fails
> > > > with:
> > > >
> > > > make: Entering directory
> > > > '/home/jejb/git/gnupg/tests/tpm2dtests'
> > > > LC_ALL=C EXEEXT=
> > > > PATH="../gpgscm:/home/jejb/.cargo/bin:/home/jejb/bin:/usr/local
> > > > /bin
> > > > :/bi
> > > > n:/usr/bin:/sbin:/usr/sbin:/usr/local/sbin:/usr/etc:/etc:/home/
> > > > jejb
> > > > /and
> > > > roid/android-sdk-linux_x86/platform-
> > > > tools:/home/jejb/android/android-
> > > > sdk-linux_x86/tools" abs_top_srcdir="/home/jejb/git/gnupg"
> > > > objdir="/home/jejb/git/gnupg"
> > > > TPMSERVER="/usr/lib/ibmtss/tpm_server"
> > > > SWTPM="" SWTPM_IOCTL=""
> > > > GNUPG_BUILD_ROOT="/home/jejb/git/gnupg/tests"
> > > > GNUPG_IN_TEST_SUITE=fact
> > > > GPGSCM_PATH="/home/jejb/git/gnupg/tests/gpgscm"
> > > > /home/jejb/git/gnupg/tests/gpgscm/gpgscm \
> > > >   /home/jejb/git/gnupg/tests/tpm2dtests/run-tests.scm 
> > > > /home/jejb/git/gnupg/tests/tpm2dtests/run-tests.scm:30: not
> > > > enough
> > > > arguments, missing: (path . args)
> > >
> > > The patches in this series should solve this.
> >
> > Even with the patches, I'm now getting a different error:
> >
> > jejb@lingrow:~/git/gnupg> make -C tests/tpm2dtests check
> > make: Entering directory '/home/jejb/git/gnupg/tests/tpm2dtests'
> > LC_ALL=C EXEEXT=
> > PATH="../gpgscm:/home/jejb/.cargo/bin:/home/jejb/bin:/usr/local/bin
> > :/bi
> > n:/usr/bin:/sbin:/usr/sbin:/usr/local/sbin:/usr/etc:/etc:/home/jejb
> > /and
> > roid/android-sdk-linux_x86/platform-
> > tools:/home/jejb/android/android-
> > sdk-linux_x86/tools" abs_top_srcdir="/home/jejb/git/gnupg"
> > objdir="/home/jejb/git/gnupg"
> > TPMSERVER="/usr/lib/ibmtss/tpm_server"
> > SWTPM="" SWTPM_IOCTL=""
> > GNUPG_BUILD_ROOT="/home/jejb/git/gnupg/tests"
> > GNUPG_IN_TEST_SUITE=fact
> > GPGSCM_PATH="/home/jejb/git/gnupg/tests/gpgscm"
> > /home/jejb/git/gnupg/tests/gpgscm/gpgscm \
> >   /home/jejb/git/gnupg/tests/tpm2dtests/run-tests.scm 
> >
> > 0: tests.scm:121: (throw (:stderr result))
> > 1: defs.scm:148: (call-popen `(,(tool-hardcoded 'gpgconf) ,@(if
> > *win32*
> > (list '--build-prefix (getenv "objdir")) '()) ,@args) input)
> > 2: defs.scm:146: (gpg-conf' "" args)
> > 3: #<CLOSURE>
> > 4: defs.scm:189: (apply gpg-conf '(--list-components))
> > FAIL: tests/openpgp/setup.scm
> > Setup failed.
> > make: *** [Makefile:632: xcheck] Error 1
> > make: Leaving directory '/home/jejb/git/gnupg/tests/tpm2dtests'
> >
> > I know this means something is missing from the setup, but I can't
> > figure out what.
>
> I got it to build and run on a Fedora 38 system using:
>
> ./autogen.sh
> ./configure --sysconfdir=/etc --enable-maintainer-mode
> make
> make -C tests/tpm2dtests check

Oh, right, you alter a Makefile.am so I have to re-run autoreconf. I
think I'm getting a different failure now, but it's actually because
keytotpm really isn't working in gpg current (so the tests are
correctly failing). The problem is this commit: 2783b786a ("agent: Do
not overwrite a key file by a shadow key file.") because the KEYTOTPM
agent command relies on overwriting the real key with a shadowed TPM
key. This is my hack to fix it and now I have all the TPM tests
passing (still using the ibmswtpm2 because the other one isn't building
on opensuse). I think the hack is actually the best way because the
corresponding KEYTOCARD would delete the key as well before rescanning
the card.

James

---

diff --git a/agent/divert-tpm2.c b/agent/divert-tpm2.c
index b2f884f93..2a4d0a352 100644
--- a/agent/divert-tpm2.c
+++ b/agent/divert-tpm2.c
@@ -40,11 +40,18 @@ agent_write_tpm2_shadow_key (ctrl_t ctrl, const unsigned char *grip,
gcry_sexp_sprint (s_pkey, GCRYSEXP_FMT_CANON, pkbuf, len);
gcry_sexp_release (s_pkey);

+ err = agent_delete_key (ctrl, NULL, grip, 1, 0);
+ if (err)
+ {
+ log_error ("failed to delete unshadowed key: %s\n", gpg_strerror (err));
+ /* try to overwrite anyway */
+ }
+
err = agent_shadow_key_type (pkbuf, shadow_info, "tpm2-v1", &shdkey);
xfree (pkbuf);
if (err)
{
- log_error ("shadowing the key failed: %s\n", gpg_strerror (err));
+ log_error ("shadowing the tpm key failed: %s\n", gpg_strerror (err));
return err;
}

On Mon, 19 Jun 2023 22:32, James Bottomley said:
> on opensuse). I think the hack is actually the best way because the
> corresponding KEYTOCARD would delete the key as well before rescanning
> the card.

I agree for the TPM. For keytocard with smartcards it is common not to
do a "save" and thus keep the original on-disk key. This allows to copy
a key to several cards as a backup.


Shalom-Salam,

Werner

--
The pioneers of a warless world are the youth that
refuse military service. - A. Einstein

On Tue, 2023-06-20 at 08:13 +0200, Werner Koch via Gnupg-devel wrote:
> On Mon, 19 Jun 2023 22:32, James Bottomley said:
> > on opensuse).  I think the hack is actually the best way because
> > the corresponding KEYTOCARD would delete the key as well before
> > rescanning the card.
>
> I agree for the TPM.  For keytocard with smartcards it is common not
> to do a "save" and thus keep the original on-disk key.  This allows
> to copy a key to several cards as a backup.

OK, I'll try to package this up as a patch. I'm still a bit worried
about the error handling: if agent_shadow_key_type() fails and the user
does a quit with save, we'll have deleted the private key. Is there
some way to force an abort without saving?

James

On Tue, 2023-06-20 at 09:10 -0400, James Bottomley via Gnupg-devel
wrote:
> On Tue, 2023-06-20 at 08:13 +0200, Werner Koch via Gnupg-devel wrote:
> > On Mon, 19 Jun 2023 22:32, James Bottomley said:
> > > on opensuse).  I think the hack is actually the best way because
> > > the corresponding KEYTOCARD would delete the key as well before
> > > rescanning the card.
> >
> > I agree for the TPM.  For keytocard with smartcards it is common
> > not
> > to do a "save" and thus keep the original on-disk key.  This allows
> > to copy a key to several cards as a backup.
>
> OK, I'll try to package this up as a patch.  I'm still a bit worried
> about the error handling: if agent_shadow_key_type() fails and the
> user does a quit with save, we'll have deleted the private key.  Is
> there some way to force an abort without saving?

OK, this is what I came up with. The error handling tries to rewrite
the secret key if something goes wrong with the shadow write, which at
least attempts to put stuff back, but I'd still prefer some ability not
to delete it in the first place.

James

-----8>8>8><8<8<8----
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Subject: [PATCH] agent: fix tpm2d key to tpm handling

commit: 2783b786a ("agent: Do not overwrite a key file by a shadow key
file.") broke keytotpm because you can no longer overwrite a
non-shadowed secret key, now you must first delete it. Fix KEYTOTPM
by deleting the key before writing it.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
agent/divert-tpm2.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/agent/divert-tpm2.c b/agent/divert-tpm2.c
index b2f884f93..64a66f5a5 100644
--- a/agent/divert-tpm2.c
+++ b/agent/divert-tpm2.c
@@ -28,7 +28,7 @@ static gpg_error_t
agent_write_tpm2_shadow_key (ctrl_t ctrl, const unsigned char *grip,
unsigned char *shadow_info)
{
- gpg_error_t err;
+ gpg_error_t err, err1;
unsigned char *shdkey;
unsigned char *pkbuf;
size_t len;
@@ -38,14 +38,20 @@ agent_write_tpm2_shadow_key (ctrl_t ctrl, const unsigned char *grip,
len = gcry_sexp_sprint(s_pkey, GCRYSEXP_FMT_CANON, NULL, 0);
pkbuf = xtrymalloc (len);
gcry_sexp_sprint (s_pkey, GCRYSEXP_FMT_CANON, pkbuf, len);
- gcry_sexp_release (s_pkey);

err = agent_shadow_key_type (pkbuf, shadow_info, "tpm2-v1", &shdkey);
xfree (pkbuf);
if (err)
{
- log_error ("shadowing the key failed: %s\n", gpg_strerror (err));
- return err;
+ log_error ("shadowing the tpm key failed: %s\n", gpg_strerror (err));
+ goto out_free;
+ }
+
+ err = agent_delete_key (ctrl, NULL, grip, 1, 0);
+ if (err)
+ {
+ log_error ("failed to delete unshadowed key: %s\n", gpg_strerror (err));
+ goto out_free;
}

len = gcry_sexp_canon_len (shdkey, 0, NULL, NULL);
@@ -53,9 +59,22 @@ agent_write_tpm2_shadow_key (ctrl_t ctrl, const unsigned char *grip,
NULL, NULL, NULL, 0);
xfree (shdkey);
if (err)
- log_error ("error writing key: %s\n", gpg_strerror (err));
+ {
+ log_error ("error writing key: %s\n", gpg_strerror (err));
+
+ err1 = agent_write_private_key (grip, shdkey, len, 1 /*force*/,
+ NULL, NULL, NULL, 0);
+ if (err1)
+ {
+ log_error ("error trying to restore private key: %s\n",
+ gpg_strerror (err1));
+ }
+ }
+ out_free:
+ gcry_sexp_release (s_pkey);

return err;
+
}

int
--
2.35.3