summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorByron Jones <bjones@mozilla.com>2012-05-17 08:58:32 +0200
committerByron Jones <bjones@mozilla.com>2012-05-17 08:58:32 +0200
commit9bc1a835aff5b65e9f827d90b6f9a0db12c16fe9 (patch)
tree63341ff1fdc3f79edbd8669f0198339cbacfdcc2
parentcff766c15cffe2f1bd92fe361d9389353cedd2f4 (diff)
downloadbugzilla-9bc1a835aff5b65e9f827d90b6f9a0db12c16fe9.tar.gz
bugzilla-9bc1a835aff5b65e9f827d90b6f9a0db12c16fe9.tar.xz
Bug 753632: allow private comments/attachments to trigger securemail
-rw-r--r--extensions/SecureMail/Extension.pm153
-rw-r--r--extensions/SecureMail/template/en/default/hook/admin/groups/edit-field.html.tmpl4
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 %]