diff options
| -rw-r--r-- | extensions/SecureMail/Extension.pm | 153 | ||||
| -rw-r--r-- | extensions/SecureMail/template/en/default/hook/admin/groups/edit-field.html.tmpl | 4 |
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 %] |