Web lists-archives.com

[PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null




In 29ff1f8f74 (t: lib-gpg: flush gpg agent on startup, 2017-07-20), a
call to gpgconf was added to kill the gpg-agent.  The intention was to
ignore all output from the call, but the order of the redirection needs
to be switched to ensure that both stdout and stderr are redirected to
/dev/null.  Without this, gpgconf from gnupg-2.0 releases would output
'gpgconf: invalid option "--kill"' each time it was called.

Signed-off-by: Todd Zullinger <tmz@xxxxxxxxx>
---

I noticed that gpgconf produced error output for a number of tests on
CentOS/RHEL.  As an example:

    *** t5534-push-signed.sh ***
    gpgconf: invalid option "--kill"

Looking at the code in lib-gpg.sh, it appeared the intention was to ignore this
output.  Reading through the review of the patch confirmed that feeling[1].  The
current code gets caught by the subtleties of output redirection.  (Who hasn't
been burned at some point by the difference between '2>&1 >/dev/null' and
'>/dev/null 2>&1' ? ;)

[1] https://public-inbox.org/git/xmqq379qlvzi.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/

Of course, beyond getting stderr to /dev/null, there is the fact that on
versions of gnupg < 2.1, gpgconf --kill is not available.  I noticed this with
gnupg-2.0.14 on CentOS 6.  It also occurs on CentOS 7, which provides
gnupg-2.0.22.

I don't know if there's much value in trying to better handle older gnupg-2.0
systems.  Using gpgconf --reload might be sufficient to work on gnupg-2.0 and
newer systems.  That might solve the issues with gpg-agent caching stale file
handles that motivated the initial patch.  If not, finding what works well with
both gnupg-2.0 and newer seems mildly painful.  This method works for me on
2.0, 2.1, and 2.2:

    pid=$(echo GETINFO pid | gpg-connect-agent | awk '/^D / {print $2}')

And people say crypto tools aren't intuitive.  Pfff. :/

A fairly gross way to use that in lib-gpg.sh might be:

    diff --git c/t/lib-gpg.sh w/t/lib-gpg.sh
    index 43679a4c64..c91d9b334f 100755
    --- c/t/lib-gpg.sh
    +++ w/t/lib-gpg.sh
    @@ -1,5 +1,10 @@
     #!/bin/sh

    +gpg_killagent() {
    +	pid=$(echo GETINFO pid | gpg-connect-agent | awk '/^D / {print $2}')
    +	test -n "$pid" && kill "$pid"
    +}
    +
     gpg_version=$(gpg --version 2>&1)
     if test $? != 127
     then
    @@ -31,7 +36,7 @@ then
     		chmod 0700 ./gpghome &&
     		GNUPGHOME="$(pwd)/gpghome" &&
     		export GNUPGHOME &&
    -		(gpgconf --kill gpg-agent 2>&1 >/dev/null || : ) &&
    +		(gpg_killagent >/dev/null 2>&1 || : ) &&
     		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
     			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
     		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \

I have my doubts that doing something like the above is worthwhile.  It's
probably good enough to simply fix up the gpgconf stderr redirection and call
it a day.

I haven't seen any gpg failures in the test runs I've done, so I haven't had a
need to re-run those tests.  (I also have not run the test suite with the ugly
gpg_killagent() diff above.  I have run it with the change to fix stderr
redirection and confirmed it succeeds without the gpgconf error messages.)

Lastly, I also noticed that git-rebase.sh uses the same 2>&1 >/dev/null.  I
suspect it's similarly not intentional:

    $ git grep -h -C4 '2>&1 >/dev/null' -- git-rebase.sh
    apply_autostash () {
    	if test -f "$state_dir/autostash"
    	then
    		stash_sha1=$(cat "$state_dir/autostash")
    		if git stash apply $stash_sha1 2>&1 >/dev/null
    		then
    			echo "$(gettext 'Applied autostash.')" >&2
    		else
    			git stash store -m "autostash" -q $stash_sha1 ||

I'll send a separate patch to adjust that code as well.

 t/lib-gpg.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 43679a4c64..a5d3b2cbaa 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,7 +31,7 @@ then
 		chmod 0700 ./gpghome &&
 		GNUPGHOME="$(pwd)/gpghome" &&
 		export GNUPGHOME &&
-		(gpgconf --kill gpg-agent 2>&1 >/dev/null || : ) &&
+		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
 		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
 			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
 		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.15.0