Message ID | 07afb94ed8336d4ca9de7078d7a6c02b1db8a908.1631304462.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 1bfb57f642d34dc4b65be3602bb429abd9f32b58 |
Headers | show |
Series | ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen | expand |
On Fri, Sep 10 2021, Fabian Stelzer via GitGitGadget wrote: > From: Fabian Stelzer <fs@gigacodes.de> > > Test that verify-commit/tag will fail when a gpg key is completely > unknown. To do this we have to generate a key, use it for a signature > and delete it from our keyring aferwards completely. > > Signed-off-by: Fabian Stelzer <fs@gigacodes.de> > --- > t/t7510-signed-commit.sh | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh > index 8df5a74f1db..d65a0171f29 100755 > --- a/t/t7510-signed-commit.sh > +++ b/t/t7510-signed-commit.sh > @@ -71,7 +71,25 @@ test_expect_success GPG 'create signed commits' ' > git tag eleventh-signed $(cat oid) && > echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid && > test_line_count = 1 oid && > - git tag twelfth-signed-alt $(cat oid) > + git tag twelfth-signed-alt $(cat oid) && > + > + cat >keydetails <<-\EOF && > + Key-Type: RSA > + Key-Length: 2048 > + Subkey-Type: RSA > + Subkey-Length: 2048 > + Name-Real: Unknown User > + Name-Email: unknown@git.com > + Expire-Date: 0 > + %no-ask-passphrase > + %no-protection > + EOF > + gpg --batch --gen-key keydetails && > + echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && > + git tag thirteenth-signed && > + DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && > + gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && > + gpg --batch --yes --delete-keys unknown@git.com > ' > > test_expect_success GPG 'verify and show signatures' ' > @@ -110,6 +128,13 @@ test_expect_success GPG 'verify and show signatures' ' > ) > ' > > +test_expect_success GPG 'verify-commit exits failure on unknown signature' ' > + test_must_fail git verify-commit thirteenth-signed 2>actual && > + ! grep "Good signature from" actual && > + ! grep "BAD signature from" actual && > + grep -q -F -e "No public key" -e "public key not found" actual > +' > + > test_expect_success GPG 'verify-commit exits success on untrusted signature' ' > git verify-commit eighth-signed-alt 2>actual && > grep "Good signature from" actual && > @@ -338,6 +363,8 @@ test_expect_success GPG 'show double signature with custom format' ' > ' > > > +# NEEDSWORK: This test relies on the test_tick commit/author dates from the first > +# 'create signed commits' test even though it creates its own > test_expect_success GPG 'verify-commit verifies multiply signed commits' ' > git init multiply-signed && > cd multiply-signed && The t7510-signed-commit.sh script hangs on startup with this change, and with -vx we show: [...] ++ git tag twelfth-signed-alt 17f06d503ee50df92746c17f6cced6feb5940cf5 ++ cat ++ gpg --batch --gen-key keydetails gpg: skipping control `%no-protection' () This is on a CentOS 7.9 box on the GCC Farm: [avar@gcc135 t]$ uname -a ; gpg --version Linux gcc135.osuosl.org 4.18.0-80.7.2.el7.ppc64le #1 SMP Thu Sep 12 15:45:05 UTC 2019 ppc64le ppc64le ppc64le GNU/Linux gpg (GnuPG) 2.0.22 libgcrypt 1.5.3 Copyright (C) 2013 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Home: ~/.gnupg Supported algorithms: Pubkey: RSA, ?, ?, ELG, DSA Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH, CAMELLIA128, CAMELLIA192, CAMELLIA256 Hash: MD5, SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224 Compression: Uncompressed, ZIP, ZLIB, BZIP2
On 22.12.2021 04:18, Ævar Arnfjörð Bjarmason wrote: > >On Fri, Sep 10 2021, Fabian Stelzer via GitGitGadget wrote: > >> From: Fabian Stelzer <fs@gigacodes.de> >> >> Test that verify-commit/tag will fail when a gpg key is completely >> unknown. To do this we have to generate a key, use it for a signature >> and delete it from our keyring aferwards completely. >> >> Signed-off-by: Fabian Stelzer <fs@gigacodes.de> >> + >> + cat >keydetails <<-\EOF && >> + Key-Type: RSA >> + Key-Length: 2048 >> + Subkey-Type: RSA >> + Subkey-Length: 2048 >> + Name-Real: Unknown User >> + Name-Email: unknown@git.com >> + Expire-Date: 0 >> + %no-ask-passphrase >> + %no-protection >> + EOF >> + gpg --batch --gen-key keydetails && >> >The t7510-signed-commit.sh script hangs on startup with this change, and >with -vx we show: > > [...] > ++ git tag twelfth-signed-alt 17f06d503ee50df92746c17f6cced6feb5940cf5 > ++ cat > ++ gpg --batch --gen-key keydetails > gpg: skipping control `%no-protection' () > >This is on a CentOS 7.9 box on the GCC Farm: > > [avar@gcc135 t]$ uname -a ; gpg --version > Linux gcc135.osuosl.org 4.18.0-80.7.2.el7.ppc64le #1 SMP Thu Sep 12 15:45:05 UTC 2019 ppc64le ppc64le ppc64le GNU/Linux > gpg (GnuPG) 2.0.22 > libgcrypt 1.5.3 > Copyright (C) 2013 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. > > Home: ~/.gnupg > Supported algorithms: > Pubkey: RSA, ?, ?, ELG, DSA > Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH, > CAMELLIA128, CAMELLIA192, CAMELLIA256 > Hash: MD5, SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224 > Compression: Uncompressed, ZIP, ZLIB, BZIP2 Hm. I have an identical centos 7.9 installation (same versions/features) and the key is generated without issues. Does the VM maybe have not enough entropy for generating a gpg key? Otherwise we could of course pre-generate the key and commit it. I'm usually not a fan of this since over time it can become unclear how it was generated or if the committed version still matches what would be generated today. But of course I don't want to slow down CI with rsa key generation stuff :/ If missing entropy is the problem, then maybe CI could benefit from something like haveged in general (other tests might want more entropy too).
On 2021-12-22 at 10:13:26, Fabian Stelzer wrote: > Hm. I have an identical centos 7.9 installation (same versions/features) and > the key is generated without issues. Does the VM maybe have not enough > entropy for generating a gpg key? > Otherwise we could of course pre-generate the key and commit it. I'm usually > not a fan of this since over time it can become unclear how it was generated > or if the committed version still matches what would be generated today. > But of course I don't want to slow down CI with rsa key generation stuff :/ > If missing entropy is the problem, then maybe CI could benefit from > something like haveged in general (other tests might want more entropy too). GnuPG is notorious for using /dev/random for generating keys, so yes, this is likely to block in a variety of situations. We don't see this on newer systems because they've replaced the blocking /dev/random with a non-blocking one except for when the CSPRNG hasn't been seeded at least once. The problem isn't lack of entropy, but the fact that there's no reason to use /dev/random since /dev/urandom is suitable for all cryptographic needs once initialized. On modern versions of Linux, one just uses getrandom(2), which deals with the uninitialized case and otherwise doesn't block. However, CentOS 7 is old.
On Wed, Dec 22 2021, Fabian Stelzer wrote: > On 22.12.2021 04:18, Ævar Arnfjörð Bjarmason wrote: >> >>On Fri, Sep 10 2021, Fabian Stelzer via GitGitGadget wrote: >> >>> From: Fabian Stelzer <fs@gigacodes.de> >>> >>> Test that verify-commit/tag will fail when a gpg key is completely >>> unknown. To do this we have to generate a key, use it for a signature >>> and delete it from our keyring aferwards completely. >>> >>> Signed-off-by: Fabian Stelzer <fs@gigacodes.de> >>> + >>> + cat >keydetails <<-\EOF && >>> + Key-Type: RSA >>> + Key-Length: 2048 >>> + Subkey-Type: RSA >>> + Subkey-Length: 2048 >>> + Name-Real: Unknown User >>> + Name-Email: unknown@git.com >>> + Expire-Date: 0 >>> + %no-ask-passphrase >>> + %no-protection >>> + EOF >>> + gpg --batch --gen-key keydetails && >>> >>The t7510-signed-commit.sh script hangs on startup with this change, and >>with -vx we show: >> >> [...] >> ++ git tag twelfth-signed-alt 17f06d503ee50df92746c17f6cced6feb5940cf5 >> ++ cat >> ++ gpg --batch --gen-key keydetails >> gpg: skipping control `%no-protection' () >> >>This is on a CentOS 7.9 box on the GCC Farm: >> >> [avar@gcc135 t]$ uname -a ; gpg --version >> Linux gcc135.osuosl.org 4.18.0-80.7.2.el7.ppc64le #1 SMP Thu Sep 12 15:45:05 UTC 2019 ppc64le ppc64le ppc64le GNU/Linux >> gpg (GnuPG) 2.0.22 >> libgcrypt 1.5.3 >> Copyright (C) 2013 Free Software Foundation, Inc. >> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> >> This is free software: you are free to change and redistribute it. >> There is NO WARRANTY, to the extent permitted by law. >> >> Home: ~/.gnupg >> Supported algorithms: >> Pubkey: RSA, ?, ?, ELG, DSA >> Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH, >> CAMELLIA128, CAMELLIA192, CAMELLIA256 >> Hash: MD5, SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224 >> Compression: Uncompressed, ZIP, ZLIB, BZIP2 > > Hm. I have an identical centos 7.9 installation (same > versions/features) and the key is generated without issues. Does the > VM maybe have not enough entropy for generating a gpg key? > Otherwise we could of course pre-generate the key and commit it. I'm > usually not a fan of this since over time it can become unclear how it > was generated or if the committed version still matches what would be > generated today. > But of course I don't want to slow down CI with rsa key generation stuff :/ > If missing entropy is the problem, then maybe CI could benefit from > something like haveged in general (other tests might want more entropy > too). Late reply. It's not a VM, but yes. I've confirmed that it's due to /dev/random hanging. I don't understand why we need to generate a key at all. It looks like your 1bfb57f642d (ssh signing: test that gpg fails for unknown keys, 2021-09-10) is just trying to test the case where we sign with a key, and then don't have that key anymore. The below POC patch seems to work just as well, and will succeed with: ./t7510-signed-commit.sh --run=1,3 Of course a lot of other tests now fail, because they relied on the discord@example.net key. But that seems easily solved by just moving this test to its own file, or deleting/re-importing the key for just that test or whatever. If we truly need yet another key why are we making it on the fly instead of adding it to t/lib-gpg/keyring.gpg like the others? diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 9882b69ae29..eec2a045cbc 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -73,23 +73,11 @@ test_expect_success GPG 'create signed commits' ' test_line_count = 1 oid && git tag twelfth-signed-alt $(cat oid) && - cat >keydetails <<-\EOF && - Key-Type: RSA - Key-Length: 2048 - Subkey-Type: RSA - Subkey-Length: 2048 - Name-Real: Unknown User - Name-Email: unknown@git.com - Expire-Date: 0 - %no-ask-passphrase - %no-protection - EOF - gpg --batch --gen-key keydetails && - echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && + echo 13 >file && git commit -a -S"discord@example.net" -m thirteenth && git tag thirteenth-signed && - DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && + DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch discord@example.net | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && - gpg --batch --yes --delete-keys unknown@git.com + gpg --batch --yes --delete-keys discord@example.net ' test_expect_success GPG 'verify and show signatures' '
On 26.12.2021 23:53, Ævar Arnfjörð Bjarmason wrote: >> >> Hm. I have an identical centos 7.9 installation (same >> versions/features) and the key is generated without issues. Does the >> VM maybe have not enough entropy for generating a gpg key? >> Otherwise we could of course pre-generate the key and commit it. I'm >> usually not a fan of this since over time it can become unclear how it >> was generated or if the committed version still matches what would be >> generated today. >> But of course I don't want to slow down CI with rsa key generation stuff :/ >> If missing entropy is the problem, then maybe CI could benefit from >> something like haveged in general (other tests might want more entropy >> too). > >Late reply. It's not a VM, but yes. I've confirmed that it's due to >/dev/random hanging. > >I don't understand why we need to generate a key at all. You are right, we don't need to. I initially toyed with the GPG commands to disable/export/reimport a key but without success (I'm not terribly familiar with GPG though). > >It looks like your 1bfb57f642d (ssh signing: test that gpg fails for >unknown keys, 2021-09-10) is just trying to test the case where we sign >with a key, and then don't have that key anymore. > It tests verifying a commit for which the key is not in our keyring at all. All the other tests only use present keys (with varying trust levels) or completely unsigned commits for the failure check. I think we could do the following though and simply point git to an empty keyring to be able to verify this: diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 9882b69ae2..2d38580847 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -71,25 +71,7 @@ test_expect_success GPG 'create signed commits' ' git tag eleventh-signed $(cat oid) && echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid && test_line_count = 1 oid && - git tag twelfth-signed-alt $(cat oid) && - - cat >keydetails <<-\EOF && - Key-Type: RSA - Key-Length: 2048 - Subkey-Type: RSA - Subkey-Length: 2048 - Name-Real: Unknown User - Name-Email: unknown@git.com - Expire-Date: 0 - %no-ask-passphrase - %no-protection - EOF - gpg --batch --gen-key keydetails && - echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && - git tag thirteenth-signed && - DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && - gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && - gpg --batch --yes --delete-keys unknown@git.com + git tag twelfth-signed-alt $(cat oid) ' test_expect_success GPG 'verify and show signatures' ' @@ -129,7 +111,7 @@ test_expect_success GPG 'verify and show signatures' ' ' test_expect_success GPG 'verify-commit exits failure on unknown signature' ' - test_must_fail git verify-commit thirteenth-signed 2>actual && + GNUPGHOME=./empty_home test_must_fail git verify-commit initial 2>actual && ! grep "Good signature from" actual && ! grep "BAD signature from" actual && grep -q -F -e "No public key" -e "public key not found" actual
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 8df5a74f1db..d65a0171f29 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -71,7 +71,25 @@ test_expect_success GPG 'create signed commits' ' git tag eleventh-signed $(cat oid) && echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid && test_line_count = 1 oid && - git tag twelfth-signed-alt $(cat oid) + git tag twelfth-signed-alt $(cat oid) && + + cat >keydetails <<-\EOF && + Key-Type: RSA + Key-Length: 2048 + Subkey-Type: RSA + Subkey-Length: 2048 + Name-Real: Unknown User + Name-Email: unknown@git.com + Expire-Date: 0 + %no-ask-passphrase + %no-protection + EOF + gpg --batch --gen-key keydetails && + echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && + git tag thirteenth-signed && + DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && + gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && + gpg --batch --yes --delete-keys unknown@git.com ' test_expect_success GPG 'verify and show signatures' ' @@ -110,6 +128,13 @@ test_expect_success GPG 'verify and show signatures' ' ) ' +test_expect_success GPG 'verify-commit exits failure on unknown signature' ' + test_must_fail git verify-commit thirteenth-signed 2>actual && + ! grep "Good signature from" actual && + ! grep "BAD signature from" actual && + grep -q -F -e "No public key" -e "public key not found" actual +' + test_expect_success GPG 'verify-commit exits success on untrusted signature' ' git verify-commit eighth-signed-alt 2>actual && grep "Good signature from" actual && @@ -338,6 +363,8 @@ test_expect_success GPG 'show double signature with custom format' ' ' +# NEEDSWORK: This test relies on the test_tick commit/author dates from the first +# 'create signed commits' test even though it creates its own test_expect_success GPG 'verify-commit verifies multiply signed commits' ' git init multiply-signed && cd multiply-signed &&