[PATCH] tty: Improve error handling and reporting

[Juergen Hoetzel]

From: Juergen Hoetzel <[hidden email]>

* tty/pinentry-tty.c (tty_cmd_handler): Set specific_err,
specific_err_loc and return early if opening the ttyname fails.

Signed-off-by: Juergen Hoetzel <[hidden email]>
---
 tty/pinentry-tty.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tty/pinentry-tty.c b/tty/pinentry-tty.c
index 403dd60..fdffa0d 100644
--- a/tty/pinentry-tty.c
+++ b/tty/pinentry-tty.c
@@ -545,16 +545,20 @@ tty_cmd_handler (pinentry_t pinentry)
     {
       ttyfi = fopen (pinentry->ttyname, "r");
       if (!ttyfi)
-        rc = -1;
+        {
+          pinentry->specific_err = gpg_error_from_syserror ();
+          pinentry->specific_err_loc = "open_tty_for_read";
+          return -1;
+        }
       else
         {
           ttyfo = fopen (pinentry->ttyname, "w");
           if (!ttyfo)
             {
-              int err = errno;
+              pinentry->specific_err = gpg_error_from_syserror ();
+              pinentry->specific_err_loc = "open_tty_for_write";
               fclose (ttyfi);
-              errno = err;
-              rc = -1;
+              return -1;
             }
         }
     }
--
2.26.2


_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

[Juergen Hoetzel]

Hi Daniel,

Am Freitag, den 08.05.2020, 16:51 -0400 schrieb Daniel Kahn Gillmor:
> Hi Juergen--
>
> Thanks for the proposed patch.  Can you give an example where this
> change in failure behavior is concretely useful?
>

I confused TTYNAME and TTYTYPE:

GPG_TTY=xterm

and tried to decrypt a file. The error message i got was also confusing
(end of file):

juergen@lemmy:~ → gpg -d ~/.password-store/Backup/test.gpg
...
gpg: public key decryption failed: End of file
gpg: decryption failed: No secret key


using this patch:

juergen@lemmy:~ → gpg -d ~/.password-store/Backup/test.gpg
...
gpg: public key decryption failed: No such file or directory
gpg: decryption failed: No secret key


Regards,

Jürgen

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

[GnuPG - Dev mailing list]

Hi Jürgen--

sorry for the delay in responding!

On Sat 2020-05-09 10:28:43 +0200, Jürgen Hötzel wrote:

> Am Freitag, den 08.05.2020, 16:51 -0400 schrieb Daniel Kahn Gillmor:
>> Hi Juergen--
>>
>> Thanks for the proposed patch.  Can you give an example where this
>> change in failure behavior is concretely useful?
>>
>
> I confused TTYNAME and TTYTYPE:
>
> GPG_TTY=xterm
>
> and tried to decrypt a file. The error message i got was also confusing
> (end of file):
>
> juergen@lemmy:~ → gpg -d ~/.password-store/Backup/test.gpg
> ...
> gpg: public key decryption failed: End of file
> gpg: decryption failed: No secret key
>
>
> using this patch:
>
> juergen@lemmy:~ → gpg -d ~/.password-store/Backup/test.gpg
> ...
> gpg: public key decryption failed: No such file or directory
> gpg: decryption failed: No secret key

I'm not sure i understand the advantage here -- both of these error
messages seem pretty opaque to me.  How would the user know that it was
the identification of the tty that is the problem?

I'm also a bit concerned about this patch's subtle changes to the
control flow: why "return 1" instead of falling through to the end of
the tty_cmd_handler() function?  Seems like just adjusting the error
reporting without changing the control flow would be a more
narrowly-targeted change (though i haven't tried to do it, or to work
out the behavior of such a patch).

GnuPG devs: any thoughts about whether the advantages this proposes are
worth the control flow risks?  Or are there better ways than this patch
to provide clearer error reporting so that the user can fix problems
more easily?

Regards,

        --dkg

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel