Bug#944340: [PATCH 2/2] email-print-mime-structure: Add --use-gpg-agent for decryption
Hello dkg, On Sat 09 Nov 2019 at 04:57PM -05, Daniel Kahn Gillmor wrote: > Thanks for your thoughtful and helpful feedback. Of course! Series applied to master, except for a few commit message edits (I was worried that we are quoting too much of the gpg manpage). > On Sat 2019-11-09 08:46:34 -0700, Sean Whitton wrote: >>> diff --git a/debian/control b/debian/control >>> index fc2bccc..4c3b956 100644 >>> --- a/debian/control >>> +++ b/debian/control >>> @@ -38,6 +38,8 @@ Depends: >>> Recommends: >>> devscripts, >>> git, >>> + gpg, >>> + gpg-agent, >> >> I think that Recommends: is a bit strong here. It would be perfectly >> reasonable to use the whole mailscripts package without using this >> feature of email-print-mime-structure. So please use Suggests:. > > we have python3-pgpy in Recommends: already, and this is analogous > functionality. If you want to move them both to Suggests, i won't > object too vociferously, but i think it would be a shame. > > Recommends already permits people to avoid installing these dependencies > on constrained systems, and many users will have gpg and gpg-agent > installed already, so this isn't actually much of an additional cost for > many people. The goal of Recommends is to install the things that > people will find typically useful, and i think this piece of > functionality is (or at least should be) typically useful. Well, what I had in mind, in particular, was how gpg-agent will get added to /etc/X11/Xsession.d, and similar places. That's quite a bit different to installing a Python library, to my mind. It seems to me that it is more difficult to distinguish between Recommends and Suggests for a package like mailscripts, because it is reasonable to install mailscripts only to use one of its scripts. That would not be an "unusual installation", to quote Policy. Another example of a package like mailscripts is devscripts, and their README says this: ... the individual dependencies (of scripts) are listed as "Recommends" in the control file; lastly, scripts that are unlikely to be used by many people have their dependencies categorized as "Suggests" in the control file. email-print-mime-structure's decryption capabilities are very cool but highly specialised, when compared to some other things in mailscripts. Finally, moving things Suggests->Recommends is less disruptive to users than Recommends->Suggests, so I'd like to leave these in Suggests for now, and we can promote them later if that looks sensible. >> Also, reading the description of bin:gpg, it seems that you need to have >> bin:gnupg for all secret key operations. > > bin:gnupg is the whole shebang -- much more than > email-print-mime-structure needs, including things like gpg-wks-client, > dirmngr, and gnupg-l10n. gpg-agent provides secret key material access, > and gpg provides the binary frontend, so this really is the right > surface area for the dependencies. (i'm one of the debian maintainers > for the package, and was responsible for this particular split, fwiw) Ah, thanks! -- Sean Whitton signature.asc Description: PGP signature
Bug#944340: [PATCH 2/2] email-print-mime-structure: Add --use-gpg-agent for decryption
Hi Sean-- Thanks for your thoughtful and helpful feedback. I've just sent a revised series (5 patches) that takes into account everything that you said. I've declined to adopt two suggestions (please see my reasons below): On Sat 2019-11-09 08:46:34 -0700, Sean Whitton wrote: >> diff --git a/debian/control b/debian/control >> index fc2bccc..4c3b956 100644 >> --- a/debian/control >> +++ b/debian/control >> @@ -38,6 +38,8 @@ Depends: >> Recommends: >> devscripts, >> git, >> + gpg, >> + gpg-agent, > > I think that Recommends: is a bit strong here. It would be perfectly > reasonable to use the whole mailscripts package without using this > feature of email-print-mime-structure. So please use Suggests:. we have python3-pgpy in Recommends: already, and this is analogous functionality. If you want to move them both to Suggests, i won't object too vociferously, but i think it would be a shame. Recommends already permits people to avoid installing these dependencies on constrained systems, and many users will have gpg and gpg-agent installed already, so this isn't actually much of an additional cost for many people. The goal of Recommends is to install the things that people will find typically useful, and i think this piece of functionality is (or at least should be) typically useful. > Also, reading the description of bin:gpg, it seems that you need to have > bin:gnupg for all secret key operations. bin:gnupg is the whole shebang -- much more than email-print-mime-structure needs, including things like gpg-wks-client, dirmngr, and gnupg-l10n. gpg-agent provides secret key material access, and gpg provides the binary frontend, so this really is the right surface area for the dependencies. (i'm one of the debian maintainers for the package, and was responsible for this particular split, fwiw) --dkg signature.asc Description: PGP signature
Bug#944340: [PATCH 2/2] email-print-mime-structure: Add --use-gpg-agent for decryption
Hello, On Fri 08 Nov 2019 at 09:03AM -05, Daniel Kahn Gillmor wrote: > On Fri 2019-11-08 02:10:48 -0500, Daniel Kahn Gillmor wrote: >> +out:subprocess.CompletedProcess[bytes] = >> subprocess.run(['gpg', '--decrypt'], >> + >> stdin=inp, >> + >> capture_output=True) > > sigh. this line should have the '--batch' option added between 'gpg' > and its command '--decrypt'. I can send you a revised patch, or you can > feel free to fix it up yourself when applying. let me know if you'd > prefer a revised patch. I've sent you other comments so please add this change to your revised series. (Another option would have been to send a "[PATCH 3/2] fixup! ..." patch which would have signalled to me that it should just be folded into the 2/2 patch. But what you did was certainly okay too!) > PS gpg(1) says: > >--batch >--no-batch > Use batch mode. Never ask, do not allow interactive commands. > --no-batch disables this option. Note that even with a filename > given on the command line, gpg might still need to read from > STDIN (in particular if gpg figures that the input is a detached > signature and no data file has been specified). Thus if you do > not want to feed data via STDIN, you should connect STDIN to > g‘/dev/null’. > > It is highly recommended to use this option along with the op‐ > tions --status-fd and --with-colons for any unattended use of > gpg. > > I am deliberately choosing to not use either --status-fd or > --with-colons for email-print-mime-structure. > > I'm not using --with-colons because there is no output from GnuPG that > we expect to be machine-readable -- we're just looking for the cleartext > of whatever ciphertext is in the message part. > > I'm not using --status-fd because there is nothing actionable we can do > with GnuPG status messages, and asking for them would require switching > from subprocess.run to subprocess.Popen to take advantage of the > pass_fds argument, which in turn would make the script only work in a > POSIX environment (i believe, but have not tested, that the script can > currently be used on Windows). This makes sense to me. One possible advantage of --status-fd would be that we could be more confident gpg will never output anything except the cleartext to our script, but for a script designed to be used only interactively, that's not a large advantage. -- Sean Whitton signature.asc Description: PGP signature
Bug#944340: [PATCH 2/2] email-print-mime-structure: Add --use-gpg-agent for decryption
Hello, On Fri 08 Nov 2019 at 02:10AM -05, Daniel Kahn Gillmor wrote: > In some cases, the user may want to try to use their own GnuPG secret > keys to decrypt encrypted parts of the message. > > By default it is disabled so that we aren't accidentally triggering > the use of user secret key material. > > Signed-off-by: Daniel Kahn Gillmor > --- > debian/control | 2 ++ > email-print-mime-structure | 18 +- > email-print-mime-structure.1.pod | 21 + > 3 files changed, 36 insertions(+), 5 deletions(-) > > diff --git a/debian/control b/debian/control > index fc2bccc..4c3b956 100644 > --- a/debian/control > +++ b/debian/control > @@ -38,6 +38,8 @@ Depends: > Recommends: > devscripts, > git, > + gpg, > + gpg-agent, I think that Recommends: is a bit strong here. It would be perfectly reasonable to use the whole mailscripts package without using this feature of email-print-mime-structure. So please use Suggests:. Also, reading the description of bin:gpg, it seems that you need to have bin:gnupg for all secret key operations. > diff --git a/email-print-mime-structure.1.pod > b/email-print-mime-structure.1.pod > index b846d87..cfdeb20 100644 > --- a/email-print-mime-structure.1.pod > +++ b/email-print-mime-structure.1.pod > + > +If I, and B encounters a > +PGP/MIME-encrypted part, it will try to decrypt the part using the > +secret keys found in the local installation of GnuPG. (default: > +I) It looks like it will always try --pgpkey= keys first, before talking to the gpg-agent. This sentence suggests that gpg-agent keys will be tried first. > -B only decrypts encrypted e-mails using > -raw, non-password-protected OpenPGP secret keys (see B<--pgpkey>, > -above). If it is unable to decrypt an encrypted part with the > -supplied keys, it will warn on stderr. > +When using B<--pgpkey>, B only decrypts > +encrypted e-mails using raw, non-password-protected OpenPGP secret > +keys. This isn't really a limitation anymore, so could either be deleted, or incorporated elsewhere in the manpage. > + > +If B has been asked to decrypt parts with > +either B<--pgpkey> or with B<--use-gpg-agent=true>, and it is unable > +to decrypt an encrypted part, it will emit a warning to stderr. Likewise, not really a limitation anymore. -- Sean Whitton signature.asc Description: PGP signature
Bug#944340: [PATCH 2/2] email-print-mime-structure: Add --use-gpg-agent for decryption
On Fri 2019-11-08 02:10:48 -0500, Daniel Kahn Gillmor wrote: > +out:subprocess.CompletedProcess[bytes] = > subprocess.run(['gpg', '--decrypt'], > + > stdin=inp, > + > capture_output=True) sigh. this line should have the '--batch' option added between 'gpg' and its command '--decrypt'. I can send you a revised patch, or you can feel free to fix it up yourself when applying. let me know if you'd prefer a revised patch. PS gpg(1) says: --batch --no-batch Use batch mode. Never ask, do not allow interactive commands. --no-batch disables this option. Note that even with a filename given on the command line, gpg might still need to read from STDIN (in particular if gpg figures that the input is a detached signature and no data file has been specified). Thus if you do not want to feed data via STDIN, you should connect STDIN to g‘/dev/null’. It is highly recommended to use this option along with the op‐ tions --status-fd and --with-colons for any unattended use of gpg. I am deliberately choosing to not use either --status-fd or --with-colons for email-print-mime-structure. I'm not using --with-colons because there is no output from GnuPG that we expect to be machine-readable -- we're just looking for the cleartext of whatever ciphertext is in the message part. I'm not using --status-fd because there is nothing actionable we can do with GnuPG status messages, and asking for them would require switching from subprocess.run to subprocess.Popen to take advantage of the pass_fds argument, which in turn would make the script only work in a POSIX environment (i believe, but have not tested, that the script can currently be used on Windows). signature.asc Description: PGP signature
Bug#944340: [PATCH 2/2] email-print-mime-structure: Add --use-gpg-agent for decryption
In some cases, the user may want to try to use their own GnuPG secret
keys to decrypt encrypted parts of the message.
By default it is disabled so that we aren't accidentally triggering
the use of user secret key material.
Signed-off-by: Daniel Kahn Gillmor
---
debian/control | 2 ++
email-print-mime-structure | 18 +-
email-print-mime-structure.1.pod | 21 +
3 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/debian/control b/debian/control
index fc2bccc..4c3b956 100644
--- a/debian/control
+++ b/debian/control
@@ -38,6 +38,8 @@ Depends:
Recommends:
devscripts,
git,
+ gpg,
+ gpg-agent,
notmuch,
python3-pgpy,
Architecture: all
diff --git a/email-print-mime-structure b/email-print-mime-structure
index 6507436..2270506 100755
--- a/email-print-mime-structure
+++ b/email-print-mime-structure
@@ -29,9 +29,11 @@ Example:
If you want to number the parts, i suggest piping the output through
something like "cat -n"
'''
+import os
import sys
import email
import logging
+import subprocess
from argparse import ArgumentParser, Namespace
from typing import Optional, Union, List, Tuple, Any
@@ -70,7 +72,7 @@ class MimePrinter(object):
nbytes = len(payload)
print(f'{prefix}{z.get_content_type()}{cset}{disposition}{fname}
{nbytes:d} bytes')
-try_decrypt:bool = True if self.args.pgpkey else False
+try_decrypt:bool = True if (self.args.pgpkey or
self.args.use_gpg_agent) else False
if try_decrypt and \
(parent is not None) and \
@@ -97,6 +99,17 @@ class MimePrinter(object):
break
except:
pass
+if cryptopayload is None and self.args.use_gpg_agent:
+inp:int
+outp:int
+inp, outp = os.pipe()
+with open(outp, 'w') as outf:
+outf.write(ciphertext)
+out:subprocess.CompletedProcess[bytes] =
subprocess.run(['gpg', '--decrypt'],
+
stdin=inp,
+
capture_output=True)
+if out.returncode == 0:
+cryptopayload = email.message_from_bytes(out.stdout)
if cryptopayload is None:
logging.warning(f'Unable to decrypt')
else:
@@ -128,6 +141,9 @@ def main() -> None:
epilog="Example:
email-print-mime-structure are used ephemerally, and
+do not interact with any local GnuPG keyring.
+
+=item B<--use-gpg-agent=>I|I
+
+If I, and B encounters a
+PGP/MIME-encrypted part, it will try to decrypt the part using the
+secret keys found in the local installation of GnuPG. (default:
+I)
+
=item B<--help>, B<-h>
Show usage instructions.
@@ -49,10 +59,13 @@ Show usage instructions.
=head1 LIMITATIONS
-B only decrypts encrypted e-mails using
-raw, non-password-protected OpenPGP secret keys (see B<--pgpkey>,
-above). If it is unable to decrypt an encrypted part with the
-supplied keys, it will warn on stderr.
+When using B<--pgpkey>, B only decrypts
+encrypted e-mails using raw, non-password-protected OpenPGP secret
+keys.
+
+If B has been asked to decrypt parts with
+either B<--pgpkey> or with B<--use-gpg-agent=true>, and it is unable
+to decrypt an encrypted part, it will emit a warning to stderr.
B's output is not stable, and is not
intended to be interpreted by machines, so please do not depend on it
--
2.24.0.rc1
