PGP support is again working (pyme API has changed significantly)...

...and we now have a regression test. We now take care that
bounce-messages for incoming encrypted mails or mails where the policy
dictates that outgoing traffic should be encrypted is actually
pgp-encrypted. Note that the new pgp encrypt option for outgoing mails
works only for bounces for now.

Author: Ralf Schlatterbeck

CHANGES.txt

@@ -29,6 +29,12 @@ Fixed:
   is addressed to [email protected] this would (wrongly) match. (Ralf)
 - issue2550729: Fix password history display for anydbm backend, thanks
   to Ralf Hemmecke for reporting. (Ralf)
+- PGP support is again working (pyme API has changed significantly) and
+  we now have a regression test. We now take care that bounce-messages
+  for incoming encrypted mails or mails where the policy dictates that
+  outgoing traffic should be encrypted is actually pgp-encrypted. Note
+  that the new pgp encrypt option for outgoing mails works only for
+  bounces for now. (Ralf)
 
 2011-07-15 1.4.19 (r4638)
 

roundup/configuration.py

@@ -799,14 +799,36 @@ def str2value(self, value):
     ), "Roundup Mail Gateway options"),
     ("pgp", (
         (BooleanOption, "enable", "no",
-            "Enable PGP processing. Requires pyme."),
+            "Enable PGP processing. Requires pyme. If you're planning\n"
+            "to send encrypted PGP mail to the tracker, you should also\n"
+            "enable the encrypt-option below, otherwise mail received\n"
+            "encrypted might be sent unencrypted to another user."),
         (NullableOption, "roles", "",
             "If specified, a comma-separated list of roles to perform\n"
             "PGP processing on. If not specified, it happens for all\n"
-            "users."),
+            "users. Note that received PGP messages (signed and/or\n"
+            "encrypted) will be processed with PGP even if the user\n"
+            "doesn't have one of the PGP roles, you can use this to make\n"
+            "PGP processing completely optional by defining a role here\n"
+            "and not assigning any users to that role."),
         (NullableOption, "homedir", "",
             "Location of PGP directory. Defaults to $HOME/.gnupg if\n"
             "not specified."),
+        (BooleanOption, "encrypt", "no",
+            "Enable PGP encryption. All outgoing mails are encrypted.\n"
+            "This requires that keys for all users (with one of the gpg\n"
+            "roles above or all users if empty) are available. Note that\n"
+            "it makes sense to educate users to also send mails encrypted\n"
+            "to the tracker, to enforce this, set 'require_incoming'\n"
+            "option below (but see the note)."),
+        (Option, "require_incoming", "signed",
+            "Require that pgp messages received by roundup are either\n"
+            "'signed', 'encrypted' or 'both'. If encryption is required\n"
+            "we do not return the message (in clear) to the user but just\n"
+            "send an informational message that the message was rejected.\n"
+            "Note that this still presents known-plaintext to an attacker\n"
+            "when the users sends the mail a second time with encryption\n"
+            "turned on."),
     ), "OpenPGP mail processing options"),
     ("nosy", (
         (RunDetectorOption, "messages_to_author", "no",

roundup/mailer.py

@@ -12,9 +12,16 @@
 from email.Utils import formatdate, formataddr, specialsre, escapesre
 from email.Message import Message
 from email.Header import Header
+from email.MIMEBase import MIMEBase
 from email.MIMEText import MIMEText
 from email.MIMEMultipart import MIMEMultipart
 
+try:
+    import pyme, pyme.core
+except ImportError:
+    pyme = None
+
+
 class MessageSendError(RuntimeError):
     pass
 
@@ -64,17 +71,14 @@ def __init__(self, config):
             os.environ['TZ'] = get_timezone(self.config.TIMEZONE).tzname(None)
             time.tzset()
 
-    def get_standard_message(self, to, subject, author=None, multipart=False):
-        '''Form a standard email message from Roundup.
-
+    def set_message_attributes(self, message, to, subject, author=None):
+        ''' Add attributes to a standard output message
         "to"      - recipients list
         "subject" - Subject
         "author"  - (name, address) tuple or None for admin email
 
         Subject and author are encoded using the EMAIL_CHARSET from the
         config (default UTF-8).
-
-        Returns a Message object.
         '''
         # encode header values if they need to be
         charset = getattr(self.config, 'EMAIL_CHARSET', 'utf-8')
@@ -85,13 +89,6 @@ def get_standard_message(self, to, subject, author=None, multipart=False):
         else:
             name = unicode(author[0], 'utf-8')
         author = nice_sender_header(name, author[1], charset)
-
-        if multipart:
-            message = MIMEMultipart()
-        else:
-            message = MIMEText("")
-            message.set_charset(charset)
-
         try:
             message['Subject'] = subject.encode('ascii')
         except UnicodeError:
@@ -114,6 +111,17 @@ def get_standard_message(self, to, subject, author=None, multipart=False):
         # finally, an aid to debugging problems
         message['X-Roundup-Version'] = __version__
 
+    def get_standard_message(self, multipart=False):
+        '''Form a standard email message from Roundup.
+        Returns a Message object.
+        '''
+        charset = getattr(self.config, 'EMAIL_CHARSET', 'utf-8')
+        if multipart:
+            message = MIMEMultipart()
+        else:
+            message = MIMEText("")
+            message.set_charset(charset)
+
         return message
 
     def standard_message(self, to, subject, content, author=None):
@@ -127,13 +135,14 @@ def standard_message(self, to, subject, content, author=None):
 
         All strings are assumed to be UTF-8 encoded.
         """
-        message = self.get_standard_message(to, subject, author)
+        message = self.get_standard_message()
+        self.set_message_attributes(message, to, subject, author)
         message.set_payload(content)
         encode_quopri(message)
         self.smtp_send(to, message.as_string())
 
     def bounce_message(self, bounced_message, to, error,
-                       subject='Failed issue tracker submission'):
+                       subject='Failed issue tracker submission', crypt=False):
         """Bounce a message, attaching the failed submission.
 
         Arguments:
@@ -143,18 +152,29 @@ def bounce_message(self, bounced_message, to, error,
           ERROR_MESSAGES_TO setting.
         - error: the reason of failure as a string.
         - subject: the subject as a string.
+        - crypt: require encryption with pgp for user -- applies only to
+          mail sent back to the user, not the dispatcher oder admin.
 
         """
+        crypt_to = None
+        if crypt:
+            crypt_to = to
+            to = None
         # see whether we should send to the dispatcher or not
         dispatcher_email = getattr(self.config, "DISPATCHER_EMAIL",
             getattr(self.config, "ADMIN_EMAIL"))
         error_messages_to = getattr(self.config, "ERROR_MESSAGES_TO", "user")
         if error_messages_to == "dispatcher":
             to = [dispatcher_email]
+            crypt = False
+            crypt_to = None
         elif error_messages_to == "both":
-            to.append(dispatcher_email)
+            if crypt:
+                to = [dispatcher_email]
+            else:
+                to.append(dispatcher_email)
 
-        message = self.get_standard_message(to, subject, multipart=True)
+        message = self.get_standard_message(multipart=True)
 
         # add the error text
         part = MIMEText('\n'.join(error))
@@ -175,15 +195,54 @@ def bounce_message(self, bounced_message, to, error,
         part = MIMEText(''.join(body))
         message.attach(part)
 
-        # send
-        try:
-            self.smtp_send(to, message.as_string())
-        except MessageSendError:
-            # squash mail sending errors when bouncing mail
-            # TODO this *could* be better, as we could notify admin of the
-            # problem (even though the vast majority of bounce errors are
-            # because of spam)
-            pass
+        if to:
+            # send
+            self.set_message_attributes(message, to, subject)
+            try:
+                self.smtp_send(to, message.as_string())
+            except MessageSendError:
+                # squash mail sending errors when bouncing mail
+                # TODO this *could* be better, as we could notify admin of the
+                # problem (even though the vast majority of bounce errors are
+                # because of spam)
+                pass
+        if crypt_to:
+            plain = pyme.core.Data(message.as_string())
+            cipher = pyme.core.Data()
+            ctx = pyme.core.Context()
+            ctx.set_armor(1)
+            keys = []
+            adrs = []
+            for adr in crypt_to:
+                ctx.op_keylist_start(adr, 0)
+                # only first key per email
+                k = ctx.op_keylist_next()
+                if k is not None:
+                    adrs.append(adr)
+                    keys.append(k)
+                ctx.op_keylist_end()
+            crypt_to = adrs
+        if crypt_to:
+            try:
+                ctx.op_encrypt(keys, 1, plain, cipher)
+                cipher.seek(0,0)
+                message=MIMEMultipart('encrypted', boundary=None,
+                    _subparts=None, protocol="application/pgp-encrypted")
+                part=MIMEBase('application', 'pgp-encrypted')
+                part.set_payload("Version: 1\r\n")
+                message.attach(part)
+                part=MIMEBase('application', 'octet-stream')
+                part.set_payload(cipher.read())
+                message.attach(part)
+            except pyme.GPGMEError:
+                crypt_to = None
+        if crypt_to:
+            self.set_message_attributes(message, crypt_to, subject)
+            try:
+                self.smtp_send(crypt_to, message.as_string())
+            except MessageSendError:
+                # ignore on error, see above.
+                pass
 
     def exception_message(self):
         '''Send a message to the admins with information about the latest

roundup/mailgw.py

@@ -159,10 +159,12 @@ def gpgh_key_getall(key, attr):
     for u in key.uids:
         yield getattr(u, attr)
 
-def check_pgp_sigs(sigs, gpgctx, author):
+def check_pgp_sigs(sigs, gpgctx, author, may_be_unsigned=False):
     ''' Theoretically a PGP message can have several signatures. GPGME
         returns status on all signatures in a list. Walk that list
-        looking for the author's signature
+        looking for the author's signature. Note that even if incoming
+        signatures are not required, the processing fails if there is an
+        invalid signature.
     '''
     for sig in sigs:
         key = gpgctx.get_key(sig.fpr, False)
@@ -188,7 +190,10 @@ def check_pgp_sigs(sigs, gpgctx, author):
                         _("Invalid PGP signature detected.")
 
     # we couldn't find a key belonging to the author of the email
-    raise MailUsageError, _("Message signed with unknown key: %s") % sig.fpr
+    if sigs:
+        raise MailUsageError, _("Message signed with unknown key: %s") % sig.fpr
+    elif not may_be_unsigned:
+        raise MailUsageError, _("Unsigned Message")
 
 class Message(mimetools.Message):
     ''' subclass mimetools.Message so we can retrieve the parts of the
@@ -452,16 +457,18 @@ def pgp_encrypted(self):
         return self.gettype() == 'multipart/encrypted' \
             and self.typeheader.find('protocol="application/pgp-encrypted"') != -1
 
-    def decrypt(self, author):
+    def decrypt(self, author, may_be_unsigned=False):
         ''' decrypt an OpenPGP MIME message
-            This message must be signed as well as encrypted using the "combined"
-            method. The decrypted contents are returned as a new message.
+            This message must be signed as well as encrypted using the
+            "combined" method if incoming signatures are configured.
+            The decrypted contents are returned as a new message.
         '''
         (hdr, msg) = self.getparts()
         # According to the RFC 3156 encrypted mail must have exactly two parts.
         # The first part contains the control information. Let's verify that
         # the message meets the RFC before we try to decrypt it.
-        if hdr.getbody() != 'Version: 1' or hdr.gettype() != 'application/pgp-encrypted':
+        if hdr.getbody().strip() != 'Version: 1' \
+           or hdr.gettype() != 'application/pgp-encrypted':
             raise MailUsageError, \
                 _("Unknown multipart/encrypted version.")
 
@@ -478,7 +485,8 @@ def decrypt(self, author):
         # key to send it to us. now check the signatures to see if it
         # was signed by someone we trust
         result = context.op_verify_result()
-        check_pgp_sigs(result.signatures, context, author)
+        check_pgp_sigs(result.signatures, context, author,
+            may_be_unsigned = may_be_unsigned)
 
         plaintext.seek(0,0)
         # pyme.core.Data implements a seek method with a different signature
@@ -550,6 +558,7 @@ def __init__(self, mailgw, message):
         self.props = None
         self.content = None
         self.attachments = None
+        self.crypt = False
 
     def handle_ignore(self):
         ''' Check to see if message can be safely ignored:
@@ -991,22 +1000,33 @@ def pgp_role():
             else:
                 return True
 
-        if self.config.PGP_ENABLE and pgp_role():
+        if self.config.PGP_ENABLE:
+            if pgp_role() and self.config.PGP_ENCRYPT:
+                self.crypt = True
             assert pyme, 'pyme is not installed'
             # signed/encrypted mail must come from the primary address
             author_address = self.db.user.get(self.author, 'address')
             if self.config.PGP_HOMEDIR:
                 os.environ['GNUPGHOME'] = self.config.PGP_HOMEDIR
+            if self.config.PGP_REQUIRE_INCOMING in ('encrypted', 'both') \
+                and pgp_role() and not self.message.pgp_encrypted():
+                raise MailUsageError, _(
+                    "This tracker has been configured to require all email "
+                    "be PGP encrypted.")
             if self.message.pgp_signed():
                 self.message.verify_signature(author_address)
             elif self.message.pgp_encrypted():
-                # replace message with the contents of the decrypted
+                # Replace message with the contents of the decrypted
                 # message for content extraction
-                # TODO: encrypted message handling is far from perfect
-                # bounces probably include the decrypted message, for
-                # instance :(
-                self.message = self.message.decrypt(author_address)
-            else:
+                # Note: the bounce-handling code now makes sure that
+                # either the encrypted mail received is sent back or
+                # that the error message is encrypted if needed.
+                encr_only = self.config.PGP_REQUIRE_INCOMING == 'encrypted'
+                encr_only = encr_only or not pgp_role()
+                self.crypt = True
+                self.message = self.message.decrypt(author_address,
+                    may_be_unsigned = encr_only)
+            elif pgp_role():
                 raise MailUsageError, _("""
 This tracker has been configured to require all email be PGP signed or
 encrypted.""")
@@ -1449,6 +1469,12 @@ def handle_Message(self, message):
             return self.handle_message(message)
 
         # no, we want to trap exceptions
+        # Note: by default we return the message received not the
+        # internal state of the parsedMessage -- except for
+        # MailUsageError, Unauthorized and for unknown exceptions. For
+        # the latter cases we make sure the error message is encrypted
+        # if needed (if it either was received encrypted or pgp
+        # processing is turned on for the user).
         try:
             return self.handle_message(message)
         except MailUsageHelp:
@@ -1466,12 +1492,18 @@ def handle_Message(self, message):
             m.append(str(value))
             m.append('\n\nMail Gateway Help\n=================')
             m.append(fulldoc)
-            self.mailer.bounce_message(message, [sendto[0][1]], m)
+            if self.parsed_message:
+                message = self.parsed_message.message
+                crypt = self.parsed_message.crypt
+            self.mailer.bounce_message(message, [sendto[0][1]], m, crypt=crypt)
         except Unauthorized, value:
             # just inform the user that he is not authorized
             m = ['']
             m.append(str(value))
-            self.mailer.bounce_message(message, [sendto[0][1]], m)
+            if self.parsed_message:
+                message = self.parsed_message.message
+                crypt = self.parsed_message.crypt
+            self.mailer.bounce_message(message, [sendto[0][1]], m, crypt=crypt)
         except IgnoreMessage:
             # do not take any action
             # this exception is thrown when email should be ignored
@@ -1492,7 +1524,10 @@ def handle_Message(self, message):
             m.append('An unexpected error occurred during the processing')
             m.append('of your message. The tracker administrator is being')
             m.append('notified.\n')
-            self.mailer.bounce_message(message, [sendto[0][1]], m)
+            if self.parsed_message:
+                message = self.parsed_message.message
+                crypt = self.parsed_message.crypt
+            self.mailer.bounce_message(message, [sendto[0][1]], m, crypt=crypt)
 
             m.append('----------------')
             m.append(traceback.format_exc())
@@ -1523,6 +1558,7 @@ def _handle_message(self, message):
         # commit the changes to the DB
         self.db.commit()
 
+        self.parsed_message = None
         return nodeid
 
     def get_class_arguments(self, class_type, classname=None):