Bug 753632: allow private comments/attachments to trigger securemail

2 files changed, 104 insertions, 53 deletions

diff --git a/extensions/SecureMail/Extension.pm b/extensions/SecureMail/Extension.pm
index 21541f845..e5a082380 100644
--- a/extensions/SecureMail/Extension.pm
+++ b/extensions/SecureMail/Extension.pm
@@ -23,6 +23,8 @@ package Bugzilla::Extension::SecureMail;
 use strict;
 use base qw(Bugzilla::Extension);
 
+use Bugzilla::Attachment;
+use Bugzilla::Comment;
 use Bugzilla::Group;
 use Bugzilla::Object;
 use Bugzilla::User;
@@ -36,7 +38,11 @@ use Crypt::OpenPGP;
 use Crypt::SMIME;
 use Encode;
 
-our $VERSION = '0.4';
+our $VERSION = '0.5';
+
+use constant SECURE_NONE => 0;
+use constant SECURE_BODY => 1;
+use constant SECURE_ALL  => 2;
 
 ##############################################################################
 # Creating new columns
@@ -46,9 +52,9 @@ our $VERSION = '0.4';
 ##############################################################################
 sub install_update_db {
     my ($self, $args) = @_;
-    
+
     my $dbh = Bugzilla->dbh;
-    $dbh->bz_add_column('groups', 'secure_mail', 
+    $dbh->bz_add_column('groups', 'secure_mail',
                         {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 0});
     $dbh->bz_add_column('profiles', 'public_key', { TYPE => 'LONGTEXT' });
 }
@@ -62,7 +68,7 @@ sub object_columns {
     my ($self, $args) = @_;
     my $class = $args->{'class'};
     my $columns = $args->{'columns'};
-    
+
     if ($class->isa('Bugzilla::Group')) {
         push(@$columns, 'secure_mail');
     }
@@ -71,13 +77,13 @@ sub object_columns {
     }
 }
 
-# Plug appropriate validators so we can check the validity of the two 
+# Plug appropriate validators so we can check the validity of the two
 # fields created by this extension, when new values are submitted.
 sub object_validators {
     my ($self, $args) = @_;
     my %args = %{ $args };
     my ($invocant, $validators) = @args{qw(class validators)};
-    
+
     if ($invocant->isa('Bugzilla::Group')) {
         $validators->{'secure_mail'} = \&Bugzilla::Object::check_boolean;
     }
@@ -85,9 +91,9 @@ sub object_validators {
         $validators->{'public_key'} = sub {
             my ($self, $value) = @_;
             $value = trim($value) || '';
-            
+
             return $value if $value eq '';
-            
+
             if ($value =~ /PUBLIC KEY/) {
                 # PGP keys must be ASCII-armoured.
                 if (!Crypt::OpenPGP::Armour->unarmour($value)) {
@@ -105,7 +111,7 @@ sub object_validators {
                 my $smime = Crypt::SMIME->new();
                 eval {
                     $smime->setPublicKey([$value]);
-                };                
+                };
                 if ($@) {
                     ThrowUserError('securemail_invalid_key',
                                    { errstr => $@ });
@@ -114,7 +120,7 @@ sub object_validators {
             else {
                 ThrowUserError('securemail_invalid_key');
             }
-            
+
             return $value;
         };
     }
@@ -136,11 +142,11 @@ sub object_update_columns {
     my ($self, $args) = @_;
     my $object  = $args->{'object'};
     my $columns = $args->{'columns'};
-    
+
     if ($object->isa('Bugzilla::Group')) {
         # This seems like a convenient moment to extract this value...
         $object->set('secure_mail', Bugzilla->cgi->param('secure_mail'));
-        
+
         push(@$columns, 'secure_mail');
     }
     elsif ($object->isa('Bugzilla::User')) {
@@ -156,13 +162,13 @@ sub user_preferences {
     my $handled = $args->{'handled'};
     my $vars    = $args->{'vars'};
     my $params  = Bugzilla->input_params;
-    
+
     return unless $tab eq 'securemail';
 
     # Create a new user object so we don't mess with the main one, as we
     # don't know where it's been...
     my $user = new Bugzilla::User(Bugzilla->user->id);
-    
+
     if ($save) {
         $user->set('public_key', $params->{'public_key'});
         $user->update();
@@ -173,7 +179,7 @@ sub user_preferences {
             $vars->{'test_email_sent'} = 1;
         }
     }
-    
+
     $vars->{'public_key'} = $user->{'public_key'};
 
     # Set the 'handled' scalar reference to true so that the caller
@@ -186,7 +192,7 @@ sub _send_test_email {
     my $template = Bugzilla->template_inner($user->settings->{'lang'}->{'value'});
 
     my $vars = {
-        to_user => $user->email, 
+        to_user => $user->email,
     };
 
     my $msg = "";
@@ -203,37 +209,57 @@ sub mailer_before_send {
     my ($self, $args) = @_;
 
     my $email = $args->{'email'};
-    
+    my $body  = $email->body;
+
     # Decide whether to make secure.
-    # This is a bit of a hack; it would be nice if it were more clear 
+    # This is a bit of a hack; it would be nice if it were more clear
     # what sort a particular email is.
     my $is_bugmail      = $email->header('X-Bugzilla-Status') ||
                           $email->header('X-Bugzilla-Type') eq 'request';
-    my $is_passwordmail = !$is_bugmail && ($email->body =~ /cfmpw.*cxlpw/s);
+    my $is_passwordmail = !$is_bugmail && ($body =~ /cfmpw.*cxlpw/s);
     my $is_test_email   = $email->header('X-Bugzilla-Type') =~ /securemail-test/ ? 1 : 0;
 
     if ($is_bugmail || $is_passwordmail || $is_test_email) {
         # Convert the email's To address into a User object
-        my $login = $email->header('To');        
+        my $login = $email->header('To');
         my $emailsuffix = Bugzilla->params->{'emailsuffix'};
         $login =~ s/$emailsuffix$//;
         my $user = new Bugzilla::User({ name => $login });
-        
-        # Default to secure. (Of course, this means if this extension has a 
-        # bug, lots of people are going to get bugmail falsely claiming their 
+
+        # Default to secure. (Of course, this means if this extension has a
+        # bug, lots of people are going to get bugmail falsely claiming their
         # bugs are secure and they need to add a key...)
-        my $make_secure = 1;
-        
+        my $make_secure = SECURE_ALL;
+
         if ($is_bugmail) {
-            # This is also a bit of a hack, but there's no header with the 
+            # This is also a bit of a hack, but there's no header with the
             # bug ID in. So we take the first number in the subject.
             my ($bug_id) = ($email->header('Subject') =~ /\[\D+(\d+)\]/);
             my $bug = new Bugzilla::Bug($bug_id);
-            if ($bug 
-                && !$bug->{'error'} 
-                && !grep($_->{secure_mail}, @{ $bug->groups_in })) 
-            {
-                $make_secure = 0;
+            if (!_should_secure_bug($bug)) {
+                $make_secure = SECURE_NONE;
+            }
+            # If the insider group has securemail enabled..
+            my $insider_group = Bugzilla::Group->new({ name => Bugzilla->params->{'insidergroup'} });
+            if ($insider_group->{secure_mail} && $make_secure == SECURE_NONE) {
+                # Encrypt if there are private comments on an otherwise public bug
+                while ($body =~ /[\r\n]--- Comment #(\d+)/g) {
+                    my $comment_id = $1;
+                    if ($comment_id) {
+                        my ($comment) = grep { $_->{count} == $comment_id } @{ $bug->comments };
+                        if ($comment && $comment->is_private) {
+                            $make_secure = SECURE_BODY;
+                            last;
+                        }
+                    }
+                }
+                # Encrypt if updating a private attachment without a comment
+                if ($email->header('X-Bugzilla-Changed-Fields') =~ /Attachment #(\d+)/) {
+                    my $attachment = Bugzilla::Attachment->new($1);
+                    if ($attachment && $attachment->isprivate) {
+                        $make_secure = SECURE_BODY;
+                    }
+                }
             }
         }
         elsif ($is_passwordmail) {
@@ -242,25 +268,50 @@ sub mailer_before_send {
             # key OR being in a security group means the mail is kept secure
             # (but, as noted above, the check is the other way around because
             # we default to secure).
-            if ($user && 
+            if ($user &&
                 !$user->{'public_key'} &&
-                !grep($_->{secure_mail}, @{ $user->groups })) 
+                !grep($_->{secure_mail}, @{ $user->groups }))
             {
-                $make_secure = 0;
-            }      
+                $make_secure = SECURE_NONE;
+            }
         }
-        
-        # If finding the user fails for some reason, but we determine we 
-        # should be encrypting, we want to make the mail safe. An empty key 
+
+        # If finding the user fails for some reason, but we determine we
+        # should be encrypting, we want to make the mail safe. An empty key
         # does that.
         my $public_key = $user ? $user->{'public_key'} : '';
-    
-        if ($make_secure) {
-            _make_secure($email, $public_key, $is_bugmail);
+
+        if ($make_secure != SECURE_NONE) {
+            _make_secure($email, $public_key, $is_bugmail && $make_secure == SECURE_ALL);
         }
     }
 }
 
+# Custom hook for bugzilla.mozilla.org (see bug 752400)
+sub bugmail_referenced_bugs {
+    my ($self, $args) = @_;
+    # Sanitise subjects of referenced bugs.
+    my $referenced_bugs = $args->{'referenced_bugs'};
+    # No need to sanitise subjects if the entire email will be secured.
+    return if _should_secure_bug($args->{'updated_bug'});
+    # Replace the subject if required
+    foreach my $ref (@$referenced_bugs) {
+        if (grep($_->{'secure_mail'}, @{ $ref->{'bug'}->groups_in })) {
+            $ref->{'short_desc'} = "(Secure bug)";
+        }
+    }
+}
+
+sub _should_secure_bug {
+    my ($bug) = @_;
+    # If there's a problem with the bug, err on the side of caution and mark it
+    # as secure.
+    return
+        !$bug
+        || $bug->{'error'}
+        || grep($_->{secure_mail}, @{ $bug->groups_in });
+}
+
 sub _make_secure {
     my ($email, $key, $sanitise_subject) = @_;
 
@@ -292,15 +343,15 @@ sub _make_secure {
 
         my $pubring = new Crypt::OpenPGP::KeyRing(Data => $key);
         my $pgp = new Crypt::OpenPGP(PubRing => $pubring);
-        
-        # "@" matches every key in the public key ring, which is fine, 
+
+        # "@" matches every key in the public key ring, which is fine,
         # because there's only one key in our keyring.
         #
         # We use the CAST5 cipher because the Rijndael (AES) module doesn't
         # like us for some reason I don't have time to debug fully.
         # ("key must be an untainted string scalar")
-        my $encrypted = $pgp->encrypt(Data       => $email->body, 
-                                      Recipients => "@", 
+        my $encrypted = $pgp->encrypt(Data       => $email->body,
+                                      Recipients => "@",
                                       Cipher     => 'CAST5',
                                       Armour     => 1);
         if (defined $encrypted) {
@@ -318,13 +369,13 @@ sub _make_secure {
         #####################
         my $smime = Crypt::SMIME->new();
         my $encrypted;
-        
+
         eval {
-            $smime->setPublicKey([$key]);                
+            $smime->setPublicKey([$key]);
             $encrypted = $smime->encrypt($email->as_string());
         };
-        
-        if (!$@) {      
+
+        if (!$@) {
             # We can't replace the Email::MIME object, so we have to swap
             # out its component parts.
             my $enc_obj = new Email::MIME($encrypted);
@@ -344,11 +395,11 @@ sub _make_secure {
           'bug_id'     => $bug_id,
           'maintainer' => Bugzilla->params->{'maintainer'}
         };
-        
+
         $template->process('account/email/encryption-required.txt.tmpl',
                            $vars, \$message)
           || ThrowTemplateError($template->error());
-        
+
         $email->body_set($message);
     }
 
diff --git a/extensions/SecureMail/template/en/default/hook/admin/groups/edit-field.html.tmpl b/extensions/SecureMail/template/en/default/hook/admin/groups/edit-field.html.tmpl
index 81436a46c..253fed29e 100644
--- a/extensions/SecureMail/template/en/default/hook/admin/groups/edit-field.html.tmpl
+++ b/extensions/SecureMail/template/en/default/hook/admin/groups/edit-field.html.tmpl
@@ -16,12 +16,12 @@
   #
   # Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
-[% IF group.is_bug_group %]
+[% IF group.is_bug_group || group.name == Param('insidergroup') %]
   <tr>
     <th>Secure Bugmail:</th>
     <td>
       <input type="checkbox" id="secure_mail" name="secure_mail"
               [% ' checked="checked"' IF group.secure_mail %]>
-    </td>  
+    </td>
   </tr>
 [% END %]