Discussion:
Bug#944340: email-print-mime-structure should have --use-gpg-agent={true|false} argument
(too old to reply)
Daniel Kahn Gillmor
2019-11-08 07:10:02 UTC
Permalink
Package: mailscripts
Severity: wishlist
Version: 0.12-1
Control: tags -1 + patch

Hi Sean and other mailscripts people--

email-print-mime-structure has --pgpkey for decrypting directly with
OpenPGP secret keys that are lying around in the filesystem.

It's possible that the user wants to try to decrypt with secret keys
that are managed by GnuPG already.

To do that, i propose adding a --use-gpg-agent={true|false} option,
defaulting to false. If true, it will attempt to decrypt any encrypted
parts using the local GnuPG secret keys.

--dkg
Daniel Kahn Gillmor
2019-11-08 07:20:01 UTC
Permalink
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 <***@fifthhorseman.net>
---
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 <message.eml")
parser.add_argument('--pgpkey', metavar='KEYFILE', action='append',
help='OpenPGP Transferable Secret Key for decrypting')
+ parser.add_argument('--use-gpg-agent', metavar='true|false', type=bool,
+ default=False,
+ help='Ask local GnuPG installation for decryption')
args:Namespace = parser.parse_args()
msg:Union[Message, str, int, Any] = email.message_from_file(sys.stdin)

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
@@ -29,6 +29,16 @@ standard input, this key will be tried for decryption. May be used
multiple times if you want to try decrypting with more than one secret
key.

+OpenPGP secret keys listed in B<--pgpkey=> are used ephemerally, and
+do not interact with any local GnuPG keyring.
+
+=item B<--use-gpg-agent=>I<true>|I<false>
+
+If I<true>, and B<email-print-mime-structure> 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<false>)
+
=item B<--help>, B<-h>

Show usage instructions.
@@ -49,10 +59,13 @@ Show usage instructions.

=head1 LIMITATIONS

-B<email-print-mime-structure> 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<email-print-mime-structure> only decrypts
+encrypted e-mails using raw, non-password-protected OpenPGP secret
+keys.
+
+If B<email-print-mime-structure> 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<email-print-mime-structure>'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
Daniel Kahn Gillmor
2019-11-08 14:10:01 UTC
Permalink
Post by Daniel Kahn Gillmor
+ 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).
Sean Whitton
2019-11-09 16:00:02 UTC
Permalink
Hello,
Post by Daniel Kahn Gillmor
Post by Daniel Kahn Gillmor
+ 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!)
Post by Daniel Kahn Gillmor
--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
Daniel Kahn Gillmor
2019-11-08 07:20:01 UTC
Permalink
This change has no functional change, it just makes it clear that
there is a distinct condition for even trying to decrypt.

It paves the way for adding in a decryption mechanism that tries to
use GnuPG.

Signed-off-by: Daniel Kahn Gillmor <***@fifthhorseman.net>
---
email-print-mime-structure | 48 +++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/email-print-mime-structure b/email-print-mime-structure
index 644efb1..6507436 100755
--- a/email-print-mime-structure
+++ b/email-print-mime-structure
@@ -70,33 +70,39 @@ 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

- if self.args.pgpkey and \
+ if try_decrypt and \
(parent is not None) and \
(parent.get_content_type().lower() == 'multipart/encrypted') and \
(str(parent.get_param('protocol')).lower() == 'application/pgp-encrypted') and \
(num == 2):
- if pgpy is None:
- logging.warning(f'Python module pgpy is not available, not decrypting (try "apt install python3-pgpy")')
- else:
- cryptopayload:Optional[Message] = None
- keyname:str
- for keyname in self.args.pgpkey:
- try:
- key:pgpy.PGPKey
- key, _ = pgpy.PGPKey.from_file(keyname)
- msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(z.get_payload())
- msg = key.decrypt(msg)
- cryptopayload = email.message_from_bytes(msg.message)
- break
- except:
- pass
- if cryptopayload is None:
- logging.warning(f'Unable to decrypt')
+ cryptopayload:Optional[Message] = None
+ ciphertext:Union[List[Message],str,bytes,None] = z.get_payload()
+ if not isinstance(ciphertext, str):
+ logging.warning('encrypted part was not a leaf mime part somehow')
+ return
+ if self.args.pgpkey:
+ if pgpy is None:
+ logging.warning(f'Python module pgpy is not available, not attempting to decrypt with --pgpkey arguments {self.args.pgpkey} (try "apt install python3-pgpy")')
else:
- newprefix = prefix[:-3] + ' '
- print(f'{newprefix}↧ (decrypts to)')
- self.print_tree(cryptopayload, newprefix + '└', z, 0)
+ keyname:str
+ for keyname in self.args.pgpkey:
+ try:
+ key:pgpy.PGPKey
+ key, _ = pgpy.PGPKey.from_file(keyname)
+ msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(ciphertext)
+ msg = key.decrypt(msg)
+ cryptopayload = email.message_from_bytes(msg.message)
+ break
+ except:
+ pass
+ if cryptopayload is None:
+ logging.warning(f'Unable to decrypt')
+ else:
+ newprefix = prefix[:-3] + ' '
+ print(f'{newprefix}↧ (decrypts to)')
+ self.print_tree(cryptopayload, newprefix + '└', z, 0)

def print_tree(self, z:Message, prefix:str, parent:Optional[Message], num:int) -> None:
if (z.is_multipart()):
--
2.24.0.rc1
Sean Whitton
2019-11-09 15:50:01 UTC
Permalink
Hello,
Post by Daniel Kahn Gillmor
This change has no functional change, it just makes it clear that
there is a distinct condition for even trying to decrypt.
Er, are you sure about this? There is a new sanity check and warning
message that might be output...
Post by Daniel Kahn Gillmor
+ logging.warning(f'Python module pgpy is not available, not attempting to decrypt with --pgpkey arguments {self.args.pgpkey} (try "apt install python3-pgpy")')
I'm not really a python programmer, so please just say if what you're
doing here is idiomatic, but it does strike me as inelegant to have to
indent all the remaining code into an 'else' block just to check for the
presence of a module.

How about factoring that code out into a function, which only gets
called if it was possible to load the module?

I think I'd find that easier to read, myself, but you're the one likely
to be maintaining this script, so I'll defer to you.
--
Sean Whitton
Daniel Kahn Gillmor
2019-11-09 22:00:01 UTC
Permalink
RFC 3156 documents PGP/MIME structural assumptions

Signed-off-by: Daniel Kahn Gillmor <***@fifthhorseman.net>
---
email-print-mime-structure.1.pod | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/email-print-mime-structure.1.pod b/email-print-mime-structure.1.pod
index 69b1cdc..e4634e6 100644
--- a/email-print-mime-structure.1.pod
+++ b/email-print-mime-structure.1.pod
@@ -82,7 +82,8 @@ environment.

=head1 SEE ALSO

-https://tools.ietf.org/html/rfc2045, https://tools.ietf.org/html/rfc2049
+https://tools.ietf.org/html/rfc2045, https://tools.ietf.org/html/rfc2049,
+https://tools.ietf.org/html/rfc3156

=head1 AUTHOR
--
2.24.0
Daniel Kahn Gillmor
2019-11-09 22:00:02 UTC
Permalink
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.

Note that 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).

Signed-off-by: Daniel Kahn Gillmor <***@fifthhorseman.net>
---
debian/control | 2 ++
email-print-mime-structure | 24 ++++++++++++++++++++++--
email-print-mime-structure.1.pod | 24 +++++++++++++++++++-----
3 files changed, 43 insertions(+), 7 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 d780883..5497597 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 = self.args.pgpkey or self.args.use_gpg_agent

if try_decrypt and \
(parent is not None) and \
@@ -84,6 +86,8 @@ class MimePrinter(object):
return
if self.args.pgpkey:
cryptopayload = self.pgpy_decrypt(self.args.pgpkey, ciphertext)
+ if cryptopayload is None and self.args.use_gpg_agent:
+ cryptopayload = self.gpg_decrypt(ciphertext)
if cryptopayload is None:
logging.warning(f'Unable to decrypt')
return
@@ -107,7 +111,20 @@ class MimePrinter(object):
except:
pass
return None
-
+
+ def gpg_decrypt(self, ciphertext:str) -> Optional[Message]:
+ inp:int
+ outp:int
+ inp, outp = os.pipe()
+ with open(outp, 'w') as outf:
+ outf.write(ciphertext)
+ out:subprocess.CompletedProcess[bytes] = subprocess.run(['gpg', '--batch', '--decrypt'],
+ stdin=inp,
+ capture_output=True)
+ if out.returncode == 0:
+ return email.message_from_bytes(out.stdout)
+ return None
+
def print_tree(self, z:Message, prefix:str, parent:Optional[Message], num:int) -> None:
if (z.is_multipart()):
self.print_part(z, prefix+'┬╴', parent, num)
@@ -132,6 +149,9 @@ def main() -> None:
epilog="Example: email-print-mime-structure <message.eml")
parser.add_argument('--pgpkey', metavar='KEYFILE', action='append',
help='OpenPGP Transferable Secret Key for decrypting')
+ parser.add_argument('--use-gpg-agent', metavar='true|false', type=bool,
+ default=False,
+ help='Ask local GnuPG installation for decryption')
args:Namespace = parser.parse_args()
msg:Union[Message, str, int, Any] = email.message_from_file(sys.stdin)

diff --git a/email-print-mime-structure.1.pod b/email-print-mime-structure.1.pod
index b846d87..69b1cdc 100644
--- a/email-print-mime-structure.1.pod
+++ b/email-print-mime-structure.1.pod
@@ -29,6 +29,25 @@ standard input, this key will be tried for decryption. May be used
multiple times if you want to try decrypting with more than one secret
key.

+OpenPGP secret keys listed in B<--pgpkey=> are used ephemerally, and
+do not interact with any local GnuPG keyring.
+
+=item B<--use-gpg-agent=>I<true>|I<false>
+
+If I<true>, and B<email-print-mime-structure> 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<false>)
+
+If both B<--pgpkey=>I<KEYFILE> and B<--use-gpg-agent=true> are
+supplied, I<KEYFILE> arguments will be tried before falling back to
+GnuPG.
+
+If B<email-print-mime-structure> has been asked to decrypt parts with
+either B<--pgpkey=>I<KEYFILE> or with B<--use-gpg-agent=true>, and it
+is unable to decrypt an encrypted part, it will emit a warning to
+stderr.
+
=item B<--help>, B<-h>

Show usage instructions.
@@ -49,11 +68,6 @@ Show usage instructions.

=head1 LIMITATIONS

-B<email-print-mime-structure> 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.
-
B<email-print-mime-structure>'s output is not stable, and is not
intended to be interpreted by machines, so please do not depend on it
in scripts!
--
2.24.0
Daniel Kahn Gillmor
2019-11-09 22:00:02 UTC
Permalink
This has no functional changes, it's just a reorganization for easier
readability. Thanks to Sean Whitton for the suggestion.

Signed-off-by: Daniel Kahn Gillmor <***@fifthhorseman.net>
---
email-print-mime-structure | 44 +++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/email-print-mime-structure b/email-print-mime-structure
index 2cbf6ed..7b8374d 100755
--- a/email-print-mime-structure
+++ b/email-print-mime-structure
@@ -81,27 +81,31 @@ class MimePrinter(object):
if not isinstance(ciphertext, str):
logging.warning('encrypted part was not a leaf mime part somehow')
return
- if pgpy is None:
- logging.warning(f'Python module pgpy is not available, not decrypting (try "apt install python3-pgpy")')
- else:
- keyname:str
- for keyname in self.args.pgpkey:
- try:
- key:pgpy.PGPKey
- key, _ = pgpy.PGPKey.from_file(keyname)
- msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(ciphertext)
- msg = key.decrypt(msg)
- cryptopayload = email.message_from_bytes(msg.message)
- break
- except:
- pass
- if cryptopayload is None:
- logging.warning(f'Unable to decrypt')
- else:
- newprefix = prefix[:-3] + ' '
- print(f'{newprefix}↧ (decrypts to)')
- self.print_tree(cryptopayload, newprefix + '└', z, 0)
+ cryptopayload = self.pgpy_decrypt(self.args.pgpkey, ciphertext)
+ if cryptopayload is None:
+ logging.warning(f'Unable to decrypt')
+ return
+ newprefix = prefix[:-3] + ' '
+ print(f'{newprefix}↧ (decrypts to)')
+ self.print_tree(cryptopayload, newprefix + '└', z, 0)

+ def pgpy_decrypt(self, keys:List[str], ciphertext:str) -> Optional[Message]:
+ if pgpy is None:
+ logging.warning(f'Python module pgpy is not available, not decrypting (try "apt install python3-pgpy")')
+ return None
+ keyname:str
+ ret:Optional[Message] = None
+ for keyname in keys:
+ try:
+ key:pgpy.PGPKey
+ key, _ = pgpy.PGPKey.from_file(keyname)
+ msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(ciphertext)
+ msg = key.decrypt(msg)
+ return email.message_from_bytes(msg.message)
+ except:
+ pass
+ return None
+
def print_tree(self, z:Message, prefix:str, parent:Optional[Message], num:int) -> None:
if (z.is_multipart()):
self.print_part(z, prefix+'┬╴', parent, num)
--
2.24.0
Sean Whitton
2019-11-10 00:00:02 UTC
Permalink
Hello,
Post by Daniel Kahn Gillmor
+ logging.warning(f'Python module pgpy is not available, not decrypting (try "apt install python3-pgpy")')
+ return None
+ keyname:str
+ ret:Optional[Message] = None
+ key:pgpy.PGPKey
+ key, _ = pgpy.PGPKey.from_file(keyname)
+ msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(ciphertext)
+ msg = key.decrypt(msg)
+ return email.message_from_bytes(msg.message)
+ pass
+ return None
+
I can delete the `ret:Optional[Message] = None` line, right? I don't
see the ret variable being used after its declaration.
--
Sean Whitton
Daniel Kahn Gillmor
2019-11-09 22:00:02 UTC
Permalink
We want to make sure we're decrypting the thing that we expect. This
typecheck should keep us honest.

Signed-off-by: Daniel Kahn Gillmor <***@fifthhorseman.net>
---
email-print-mime-structure | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/email-print-mime-structure b/email-print-mime-structure
index 644efb1..2cbf6ed 100755
--- a/email-print-mime-structure
+++ b/email-print-mime-structure
@@ -76,16 +76,20 @@ class MimePrinter(object):
(parent.get_content_type().lower() == 'multipart/encrypted') and \
(str(parent.get_param('protocol')).lower() == 'application/pgp-encrypted') and \
(num == 2):
+ cryptopayload:Optional[Message] = None
+ ciphertext:Union[List[Message],str,bytes,None] = z.get_payload()
+ if not isinstance(ciphertext, str):
+ logging.warning('encrypted part was not a leaf mime part somehow')
+ return
if pgpy is None:
logging.warning(f'Python module pgpy is not available, not decrypting (try "apt install python3-pgpy")')
else:
- cryptopayload:Optional[Message] = None
keyname:str
for keyname in self.args.pgpkey:
try:
key:pgpy.PGPKey
key, _ = pgpy.PGPKey.from_file(keyname)
- msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(z.get_payload())
+ msg:pgpy.PGPMessage = pgpy.PGPMessage.from_blob(ciphertext)
msg = key.decrypt(msg)
cryptopayload = email.message_from_bytes(msg.message)
break
--
2.24.0
Daniel Kahn Gillmor
2019-11-09 22:00:02 UTC
Permalink
No functional change here: this just prepares for adding other
decryption capabilities.

Signed-off-by: Daniel Kahn Gillmor <***@fifthhorseman.net>
---
email-print-mime-structure | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/email-print-mime-structure b/email-print-mime-structure
index 7b8374d..d780883 100755
--- a/email-print-mime-structure
+++ b/email-print-mime-structure
@@ -70,8 +70,9 @@ 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

- if self.args.pgpkey and \
+ if try_decrypt and \
(parent is not None) and \
(parent.get_content_type().lower() == 'multipart/encrypted') and \
(str(parent.get_param('protocol')).lower() == 'application/pgp-encrypted') and \
@@ -81,7 +82,8 @@ class MimePrinter(object):
if not isinstance(ciphertext, str):
logging.warning('encrypted part was not a leaf mime part somehow')
return
- cryptopayload = self.pgpy_decrypt(self.args.pgpkey, ciphertext)
+ if self.args.pgpkey:
+ cryptopayload = self.pgpy_decrypt(self.args.pgpkey, ciphertext)
if cryptopayload is None:
logging.warning(f'Unable to decrypt')
return
--
2.24.0
Daniel Kahn Gillmor
2019-11-09 22:10:02 UTC
Permalink
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.
Post by Daniel Kahn Gillmor
diff --git a/debian/control b/debian/control
index fc2bccc..4c3b956 100644
--- a/debian/control
+++ b/debian/control
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
Sean Whitton
2019-11-10 08:50:02 UTC
Permalink
Hello dkg,
Post by Daniel Kahn Gillmor
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).
Post by Daniel Kahn Gillmor
Post by Daniel Kahn Gillmor
diff --git a/debian/control b/debian/control
index fc2bccc..4c3b956 100644
--- a/debian/control
+++ b/debian/control
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.
Post by Daniel Kahn Gillmor
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
Loading...