bug#23619: Some epg tests fail

bug#23619: Some epg tests fail

[Glenn Morris]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Package: emacs
Version: 25.1.50
Severity: minor

Two epg tests fail for me on RHEL 7.2 with current master (and have done
since they stopped being skipped).

gpg --version
gpg (GnuPG) 2.0.22
libgcrypt 1.5.3

Test log attached, though obviously more info will be needed.


[-- Attachment #2: epg-tests.log.gz --]
[-- Type: application/octet-stream, Size: 585 bytes --]

bug#23619: Some epg tests fail

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

tags 23619 patch
quit

Glenn Morris <rgm@gnu.org> writes:

> Two epg tests fail for me on RHEL 7.2 with current master (and have done
> since they stopped being skipped).
>
> gpg --version
> gpg (GnuPG) 2.0.22
> libgcrypt 1.5.3

> 2 unexpected results:
>    FAILED  epg-decrypt-1
>    FAILED  epg-roundtrip-1

I have

    gpg --version
    gpg (GnuPG) 2.1.18
    libgcrypt 1.7.6

These tests popup a dialog box to enter a passphrase before failing.
The following patch which sets pinentry mode to 'loopback' fixes it for
me (I have not tested with other gpg versions though):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1507 bytes --]

From 1414b5b5f7c52215c0b7b5aaac2b657b69802ee4 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 19 Feb 2017 10:34:50 -0500
Subject: [PATCH v1] Fix passphrase callback with gpg 2.x (Bug#23619)

* lisp/epg.el (epg-context-set-passphrase-callback): Set CONTEXT's
pinentry mode to `loopback'.
---
 lisp/epg.el | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lisp/epg.el b/lisp/epg.el
index 587271b000..6b4accf1fe 100644
--- a/lisp/epg.el
+++ b/lisp/epg.el
@@ -246,16 +246,15 @@ epg-context-set-passphrase-callback
 The function gets three arguments: the context, the key-id in
 question, and the callback data (if any).
 
-The callback may not be called if you use GnuPG 2.x, which relies
-on the external program called `gpg-agent' for passphrase query.
-If you really want to intercept passphrase query, consider
-installing GnuPG 1.x _along with_ GnuPG 2.x, which does passphrase
-query by itself and Emacs can intercept them."
+Also set CONTEXT's pinentry mode to `loopback' (when using GnuPG
+2.x, the callback would not be called otherwise)."
   ;; (declare (obsolete setf "25.1"))
   (setf (epg-context-passphrase-callback context)
         (if (functionp passphrase-callback)
 	    (list passphrase-callback)
-	  passphrase-callback)))
+	  passphrase-callback))
+  (when passphrase-callback
+    (setf (epg-context-pinentry-mode context) 'loopback)))
 
 (defun epg-context-set-progress-callback (context
 					  progress-callback)
-- 
2.11.1

bug#23619: Some epg tests fail

[Daiki Ueno]

npostavs@users.sourceforge.net writes:

> tags 23619 patch
> quit
>
> Glenn Morris <rgm@gnu.org> writes:
>
>> Two epg tests fail for me on RHEL 7.2 with current master (and have done
>> since they stopped being skipped).
>>
>> gpg --version
>> gpg (GnuPG) 2.0.22
>> libgcrypt 1.5.3
>
>> 2 unexpected results:
>>    FAILED  epg-decrypt-1
>>    FAILED  epg-roundtrip-1
>
> I have
>
>     gpg --version
>     gpg (GnuPG) 2.1.18
>     libgcrypt 1.7.6
>
> These tests popup a dialog box to enter a passphrase before failing.
> The following patch which sets pinentry mode to 'loopback' fixes it for
> me (I have not tested with other gpg versions though):

Thank you.  That sounds like a good idea, but the version actually
matters: gpg 1.x and 2.0.x refuse the --pinentry-mode option and exit
abnormally...

Maybe you could add a version check around the call (or in epg--start)?
epg-config.el provides a few functions to do that.

Regards,
-- 
Daiki Ueno

bug#23619: Some epg tests fail

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

Daiki Ueno <ueno@gnu.org> writes:

> npostavs@users.sourceforge.net writes:
>
>>
>> These tests popup a dialog box to enter a passphrase before failing.
>> The following patch which sets pinentry mode to 'loopback' fixes it for
>> me (I have not tested with other gpg versions though):
>
> Thank you.  That sounds like a good idea, but the version actually
> matters: gpg 1.x and 2.0.x refuse the --pinentry-mode option and exit
> abnormally...
>
> Maybe you could add a version check around the call (or in epg--start)?
> epg-config.el provides a few functions to do that.

Does this look right?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2386 bytes --]

From 9cd6f55d3d01fe0781ce81b2e394ed3e8febd6fa Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 19 Feb 2017 10:34:50 -0500
Subject: [PATCH v2] Fix passphrase callback with gpg 2.1+ (Bug#23619)

* lisp/epg.el (epg-context-set-passphrase-callback): Set CONTEXT's
pinentry mode to `loopback'.
(epg--start): Only pass `--pinentry-mode' for gpg 2.1+.
---
 lisp/epg.el | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/epg.el b/lisp/epg.el
index 587271b000..ba7125f232 100644
--- a/lisp/epg.el
+++ b/lisp/epg.el
@@ -246,16 +246,15 @@ epg-context-set-passphrase-callback
 The function gets three arguments: the context, the key-id in
 question, and the callback data (if any).
 
-The callback may not be called if you use GnuPG 2.x, which relies
-on the external program called `gpg-agent' for passphrase query.
-If you really want to intercept passphrase query, consider
-installing GnuPG 1.x _along with_ GnuPG 2.x, which does passphrase
-query by itself and Emacs can intercept them."
+Also set CONTEXT's pinentry mode to `loopback' (when using GnuPG
+2.x, the callback would not be called otherwise)."
   ;; (declare (obsolete setf "25.1"))
   (setf (epg-context-passphrase-callback context)
         (if (functionp passphrase-callback)
 	    (list passphrase-callback)
-	  passphrase-callback)))
+	  passphrase-callback))
+  (when passphrase-callback
+    (setf (epg-context-pinentry-mode context) 'loopback)))
 
 (defun epg-context-set-progress-callback (context
 					  progress-callback)
@@ -578,7 +577,13 @@ epg--start
 		       (if (epg-context-textmode context) '("--textmode"))
 		       (if (epg-context-output-file context)
 			   (list "--output" (epg-context-output-file context)))
-		       (if (epg-context-pinentry-mode context)
+		       (if (and (epg-context-pinentry-mode context)
+                                ;; GPG 1.x and 2.0.x reject `--pinentry-mode'.
+                                (version<= "2.1"
+                                           (alist-get
+                                            'version (epg-find-configuration
+                                                      (epg-context-protocol context))
+                                            "1.0")))
 			   (list "--pinentry-mode"
 				 (symbol-name (epg-context-pinentry-mode
 					       context))))
-- 
2.11.1

bug#23619: Some epg tests fail

[Daiki Ueno]

npostavs@users.sourceforge.net writes:

> Does this look right?

Yes, thank you, please push.

Regards,
-- 
Daiki Ueno

bug#23619: Some epg tests fail

[npostavs]

Although this doesn't solve the problem for gpg 2.0.  I think we could
use '--batch' with '--passphrase' or 'passphrase-fd' for this, although
neither of these works as a callback.

Or is it better just to skip?  We would need to be able to specify
multiple version ranges to do that properly.

bug#23619: Some epg tests fail

[Daiki Ueno]

npostavs@users.sourceforge.net writes:

> Although this doesn't solve the problem for gpg 2.0.

For a proper fix, you might want to check how GPGME and its test suites
do.

> I think we could use '--batch' with '--passphrase' or 'passphrase-fd'
> for this, although neither of these works as a callback.

If that approach makes all interactions with gpg synchronous, it
wouldn't be acceptable.

bug#23619: Some epg tests fail

[npostavs]

Daiki Ueno <ueno@gnu.org> writes:

> npostavs@users.sourceforge.net writes:
>
>> Although this doesn't solve the problem for gpg 2.0.
>
> For a proper fix, you might want to check how GPGME and its test suites
> do.

I don't see anything to handle gpg 2.0, specifically.  They do disable
pinentry for versions below 2.1.  Maybe their tests don't work with 2.0
either?

>> I think we could use '--batch' with '--passphrase' or 'passphrase-fd'
>> for this, although neither of these works as a callback.
>
> If that approach makes all interactions with gpg synchronous, it
> wouldn't be acceptable.

With '--passphrase-fd', interactions remain asynchronous from Emacs'
point of view, but gpg sends no output until receiving the passphrase on
the given fd, so the normal callback doesn't get triggered.  So to make
that work, we would have to send the callback's output immediately after
starting the gpg process.

Using '--passphrase' means giving the passphrase on the command line,
which is not very good in terms of security for normal use, though
acceptable for tests.

bug#23619: Some epg tests fail

[Daiki Ueno]

npostavs@users.sourceforge.net writes:

> Daiki Ueno <ueno@gnu.org> writes:
>
>> npostavs@users.sourceforge.net writes:
>>
>>> Although this doesn't solve the problem for gpg 2.0.
>>
>> For a proper fix, you might want to check how GPGME and its test suites
>> do.
>
> I don't see anything to handle gpg 2.0, specifically.  They do disable
> pinentry for versions below 2.1.  Maybe their tests don't work with 2.0
> either?

It sounds unlikely to me that they don't work with 2.0, because 2.0
support has been there for a decade.

If I understand correctly their tests use their own pinentry program for
2.0 and 2.1:
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gpgme.git;a=blob;f=tests/gpg/pinentry
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gpgme.git;a=blob;f=tests/gpg/Makefile.am;h=8e26a92f0c54ce3c76e63e223ce034dea11d6e71;hb=HEAD#l115

If your goal is to make epg tests work with all gpg versions, this could
be an option worth considering.

bug#23619: Some epg tests fail

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

Daiki Ueno <ueno@gnu.org> writes:

> npostavs@users.sourceforge.net writes:
>
>> Daiki Ueno <ueno@gnu.org> writes:
>>
>>> npostavs@users.sourceforge.net writes:
>>>
>>>> Although this doesn't solve the problem for gpg 2.0.
>>>
>>> For a proper fix, you might want to check how GPGME and its test suites
>>> do.
>>
>> I don't see anything to handle gpg 2.0, specifically.  They do disable
>> pinentry for versions below 2.1.  Maybe their tests don't work with 2.0
>> either?
>
> It sounds unlikely to me that they don't work with 2.0, because 2.0
> support has been there for a decade.
>
> If I understand correctly their tests use their own pinentry program for
> 2.0 and 2.1:
> https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gpgme.git;a=blob;f=tests/gpg/pinentry
> https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gpgme.git;a=blob;f=tests/gpg/Makefile.am;h=8e26a92f0c54ce3c76e63e223ce034dea11d6e71;hb=HEAD#l115
>
> If your goal is to make epg tests work with all gpg versions, this could
> be an option worth considering.

The patch below lets the tests succeed also without the loopback patch.
Should we have both?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 3097 bytes --]

From 822207cea1f6b65a19c665a96bcbe181ab47547c Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Tue, 21 Feb 2017 20:46:15 -0500
Subject: [PATCH v3] Fix epg-tests with dummy-pinentry program (Bug#23619)

* test/data/epg/dummy-pinentry: New file.
* test/lisp/epg-tests.el (with-epg-tests): Add it to gpg-agent.conf
when a passphrase is required.  Add debug declaration.  Set
GPG_AGENT_INFO non-destructively.
---
 test/data/epg/dummy-pinentry | 22 ++++++++++++++++++++++
 test/lisp/epg-tests.el       | 18 ++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)
 create mode 100755 test/data/epg/dummy-pinentry

diff --git a/test/data/epg/dummy-pinentry b/test/data/epg/dummy-pinentry
new file mode 100755
index 0000000000..2228dfb0c6
--- /dev/null
+++ b/test/data/epg/dummy-pinentry
@@ -0,0 +1,22 @@
+#! /bin/bash
+# Dummy pinentry
+#
+# Copyright 2008 g10 Code GmbH
+#
+# This file is free software; as a special exception the author gives
+# unlimited permission to copy and/or distribute it, with or without
+# modifications, as long as this notice is preserved.
+#
+# This file is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY, to the extent permitted by law; without even the
+# implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+# PURPOSE.
+
+echo OK Your orders please
+
+while read cmd; do
+  case $cmd in
+    GETPIN) echo D test0123456789; echo OK;;
+    *) echo OK;;
+  esac
+done
diff --git a/test/lisp/epg-tests.el b/test/lisp/epg-tests.el
index ea2b62c358..c61527207f 100644
--- a/test/lisp/epg-tests.el
+++ b/test/lisp/epg-tests.el
@@ -52,8 +52,9 @@ epg-tests-passphrase-callback
 					require-secret-key)
 			    &rest body)
   "Set up temporary locations and variables for testing."
-  (declare (indent 1))
-  `(let ((epg-tests-home-directory (make-temp-file "epg-tests-homedir" t)))
+  (declare (indent 1) (debug (sexp body)))
+  `(let ((epg-tests-home-directory (make-temp-file "epg-tests-homedir" t))
+         (process-environment (cons "GPG_AGENT_INFO" process-environment)))
      (unwind-protect
 	 (let ((context (epg-make-context 'OpenPGP)))
            (setf (epg-context-program context)
@@ -63,11 +64,16 @@ epg-tests-passphrase-callback
                                   `'require-passphrase))))
 	   (setf (epg-context-home-directory context)
 		 epg-tests-home-directory)
-	   (setenv "GPG_AGENT_INFO")
 	   ,(if require-passphrase
-		`(epg-context-set-passphrase-callback
-		  context
-		  #'epg-tests-passphrase-callback))
+		`(with-temp-file (expand-file-name
+                                  "gpg-agent.conf" epg-tests-home-directory)
+                   (insert "pinentry-program "
+                           (expand-file-name "dummy-pinentry"
+                                             epg-tests-data-directory)
+                           "\n")
+                   (epg-context-set-passphrase-callback
+                    context
+                    #'epg-tests-passphrase-callback)))
 	   ,(if require-public-key
 		`(epg-import-keys-from-file
 		  context
-- 
2.11.1

bug#23619: Some epg tests fail

[Daiki Ueno]

npostavs@users.sourceforge.net writes:

> The patch below lets the tests succeed also without the loopback patch.
> Should we have both?

Thank you.  I guess maybe having only the dummy pinentry patch would be
sufficient.

Regards,
-- 
Daiki Ueno

bug#23619: Some epg tests fail

[npostavs]

tags 23619 fixed
close 23619 26.1
quit

Daiki Ueno <ueno@gnu.org> writes:

> npostavs@users.sourceforge.net writes:
>
>> The patch below lets the tests succeed also without the loopback patch.
>> Should we have both?
>
> Thank you.  I guess maybe having only the dummy pinentry patch would be
> sufficient.

Pushed to master [1: b7250496bd].

1: 2017-02-28 19:57:35 -0500 b7250496bd2d681a7a7f3c958eb3f0047b5c0a22
  Fix epg-tests with dummy-pinentry program (Bug#23619)

bug#23619: Some epg tests fail

[Glenn Morris]

Both epg test continue to fail for me just as in the original report.

bug#23619: Some epg tests fail

[npostavs]

reopen 23619 
quit

Glenn Morris <rgm@gnu.org> writes:

> Both epg test continue to fail for me just as in the original report.

Hmm, so pinentry-program doesn't work for 2.0?  It's documented in the
manual [1].

[1]: https://www.gnupg.org/documentation/manuals/gnupg-2.0/Agent-Options.html#index-pinentry_002dprogram

bug#23619: Some epg tests fail

[Glenn Morris]

npostavs@users.sourceforge.net wrote:

> reopen 23619 

Thanks. Since you add the fixed tag when you close bugs, you'll need to
remember to remove it when you reopen. (Personally I recommend not using
the fixed tag.)

>> Both epg test continue to fail for me just as in the original report.
>
> Hmm, so pinentry-program doesn't work for 2.0?  It's documented in the
> manual [1].

Obviously I need to get a proper informative failure message so we can
see why the tests fail...

bug#23619: Some epg tests fail

[Glenn Morris]

[-- Attachment #1: Type: text/plain, Size: 53 bytes --]


I don't know if the attached backtrace is helpful.


[-- Attachment #2: backtrace.xz --]
[-- Type: application/octet-stream, Size: 3576 bytes --]

bug#23619: Some epg tests fail

[npostavs]

tag 23619 - fixed patch
quit

Glenn Morris <rgm@gnu.org> writes:

>> reopen 23619 
>
> Thanks. Since you add the fixed tag when you close bugs, you'll need to
> remember to remove it when you reopen. (Personally I recommend not using
> the fixed tag.)

The debbugs.el code (from debbugs-gnu-send-control-message) I adapted
adds the fixed tag.  I've now added code to remove it when reopening.

>>
>> Hmm, so pinentry-program doesn't work for 2.0?  It's documented in the
>> manual [1].
>
> Obviously I need to get a proper informative failure message so we can
> see why the tests fail...

> I don't know if the attached backtrace is helpful.

AFAICT, the only relevant data is on the first line:

gpg: keyring `/tmp/epg-tests-homedir30708Tdb/secring.gpg' created
gpg: keyring `/tmp/epg-tests-homedir30708Tdb/pubring.gpg' created
gpg: CAST5 encrypted data
gpg: can't connect to the agent: IPC connect call failed
gpg: problem with the agent: No agent running
gpg: encrypted with 1 passphrase
gpg: decryption failed: No secret key

Does "IPC connect call failed" give you any ideas?

You could try setting `epg-debug' and looking at " *epg-debug*" buffer,
not sure if will have any more info though.