[Rt-commit] rt branch, 4.4/crypt-minor-fixes-2, created. rt-4.4.4-243-ge611760cfd

? sunnavy sunnavy at bestpractical.com
Mon Feb 8 11:55:39 EST 2021


The branch, 4.4/crypt-minor-fixes-2 has been created
        at  e611760cfdd74301f6dc00283a07179a11fcaa38 (commit)

- Log -----------------------------------------------------------------
commit b1e6dfd2f8dc71ba70aaf00e7c11d55fe133b6d0
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Tue Feb 2 11:14:57 2021 -0500

    Include S/MIME certificate serial number in tooltip

diff --git a/lib/RT/Crypt/SMIME.pm b/lib/RT/Crypt/SMIME.pm
index 3b3373f7ae..e7540bb558 100644
--- a/lib/RT/Crypt/SMIME.pm
+++ b/lib/RT/Crypt/SMIME.pm
@@ -524,6 +524,7 @@ sub Verify {
             $res{'status'} = $self->FormatStatus({
                 Operation => "Verify", Status => "BAD",
                 Message => "The signing CA was not trusted",
+                Serial => $signer->{'Serial Number'} || 'unknown',
                 UserString => $signer->{User}[0]{String},
                 ExpireTimestamp => $signer->{Expire}->Unix(),
                 CreatedTimestamp => $signer->{Created}->Unix(),
@@ -560,6 +561,7 @@ sub Verify {
             Message => "The signature is good, unknown signer",
             ExpireTimestamp => $signer->{Expire}->Unix(),
             CreatedTimestamp => $signer->{Created}->Unix(),
+            Serial => $signer->{'Serial Number'} || 'unknown',
             Trust => "UNKNOWN",
         });
         return %res;
@@ -574,6 +576,7 @@ sub Verify {
     $res{'status'} = $self->FormatStatus({
         Operation => "Verify", Status => "DONE",
         Message => "The signature is good, signed by ".$signer->{User}[0]{String}.", assured by " . $signer->{Issuer}[0]{String} . ", trust is ".$signer->{TrustTerse},
+        Serial => $signer->{'Serial Number'} || 'unknown',
         UserString => $signer->{User}[0]{String},
         Trust => uc($signer->{TrustTerse}),
         Issuer => $signer->{Issuer}[0]{String},
diff --git a/share/html/Elements/CryptStatus b/share/html/Elements/CryptStatus
index a608d7ca62..c116145667 100644
--- a/share/html/Elements/CryptStatus
+++ b/share/html/Elements/CryptStatus
@@ -90,6 +90,7 @@ sub VerifyTooltip {
     $tooltip .= "\n" . loc('Signature Created') . ': ' . DisplayDate($line->{Timestamp}) if $line->{Timestamp};                 # GNUPG
     $tooltip .= "\n" . loc('Signer') . ': ' . $line->{UserString} if $line->{UserString};                                       # SMIME
     $tooltip .= "\n" . loc('Issuer') . ': ' . $line->{Issuer} if $line->{Issuer};                                               # SMIME
+    $tooltip .= "\n" . loc('Serial Number') . ': ' . $line->{Serial} if $line->{Serial};                                        # SMIME
     $tooltip .= "\n" . loc('Certificate Created') . ': ' . DisplayDate($line->{CreatedTimestamp}) if $line->{CreatedTimestamp}; # SMIME
     if ($protocol eq 'SMIME') {
         $tooltip .= "\n" . loc('Certificate Expires') . ': ';

commit 92aff8842dd62df5623384cf25097076876d7df8
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Tue Feb 2 11:15:36 2021 -0500

    Update tests for newly-included S/MIME certificate serial number

diff --git a/t/mail/smime/incoming.t b/t/mail/smime/incoming.t
index 82249d1c52..625b441661 100644
--- a/t/mail/smime/incoming.t
+++ b/t/mail/smime/incoming.t
@@ -151,6 +151,7 @@ RT::Test->close_mailgate_ok($mail);
             {   Status           => 'DONE',
                 UserString       => '"Enoch Root" <root at example.com>',
                 Trust            => 'FULL',
+                Serial           => '9974010075738841110',
                 Issuer           => '"CA Owner" <ca.owner at example.com>',
                 CreatedTimestamp => re('^\d+$'),
                 Message =>
@@ -208,6 +209,7 @@ RT::Test->close_mailgate_ok($mail);
                     Protocol         => 'SMIME',
                     Operation        => 'Verify',
                     Status           => 'DONE',
+                    Serial           => '9974010075738841110',
                     Message =>
                         'The signature is good, signed by "Enoch Root" <root at example.com>, assured by "CA Owner" <ca.owner at example.com>, trust is full',
                     UserString => '"Enoch Root" <root at example.com>',
diff --git a/t/web/smime/outgoing.t b/t/web/smime/outgoing.t
index fd85917516..227a923025 100644
--- a/t/web/smime/outgoing.t
+++ b/t/web/smime/outgoing.t
@@ -222,7 +222,7 @@ foreach my $mail ( map cleanup_headers($_), @{ $mail{'signed_encrypted'} } ) {
         'Signature status correctly displayed'
     );
     $m->content_like(
-        qr{<span title="Signer: "sender" <sender\@example.com>\nIssuer: "CA Owner" <ca.owner\@example.com>\nCertificate Created: .* 2013\nCertificate Expires: .* 2023">}m,
+        qr{<span title="Signer: "sender" <sender\@example.com>\nIssuer: "CA Owner" <ca.owner\@example.com>\nSerial Number: 9974010075738841109\nCertificate Created: .* 2013\nCertificate Expires: .* 2023">}m,
         'Tooltip correctly displayed'
     );
 

commit be277569dd34e5cf36895a572257bfa899772133
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Tue Feb 2 14:36:27 2021 -0500

    Add ability to download S/MIME certificates

diff --git a/lib/RT/Crypt/SMIME.pm b/lib/RT/Crypt/SMIME.pm
index e7540bb558..546247696c 100644
--- a/lib/RT/Crypt/SMIME.pm
+++ b/lib/RT/Crypt/SMIME.pm
@@ -1255,4 +1255,54 @@ sub SupportsCRLfile {
     return $OpenSSL_Supports_CRLfile;
 };
 
+# Given a Transaction object, find the application/x-rt-original-message
+# (if any).  If found, try to find the S/MIME signature.  Return
+# the certificate found in that signature.
+# Returns: undef if we could not find a certificate; otherwise, an
+# S/MIME certificate in PEM format.
+sub GetCertificateForTransaction {
+    my ($self, $txn) = @_;
+
+    my $attachments = $txn->Attachments;
+    my $found;
+
+    # Look for the application/x-rt-original-message attachment
+    while (my $a = $attachments->Next) {
+        if (lc($a->ContentType) eq 'application/x-rt-original-message') {
+            $found = $a;
+            last;
+        }
+    }
+    return undef unless $found;
+
+    my $str = $found->Content;
+    return undef unless $str;
+
+    my $tmpdir = File::Temp::tempdir( TMPDIR => 1, CLEANUP => 1 );
+    my $p = MIME::Parser->new();
+    $p->output_dir($tmpdir);
+    my $entity = $p->parse_data($str);
+    return undef unless $entity;
+
+    # Now look for application/pkcs7-signature
+    $found = undef;
+    foreach my $part ($entity->parts_DFS) {
+        if (lc($part->mime_type) eq 'application/pkcs7-signature') {
+            $found = $part;
+            last;
+        }
+    }
+    return undef unless $found;
+    return undef unless $found->bodyhandle;
+
+    # Feed to openssl and extract the certificate
+    my $pkcs7 = $found->bodyhandle->as_string;
+    my $out = '';
+    my $err = '';
+    safe_run_child { run3( [$self->OpenSSLPath, 'pkcs7', '-inform', 'DER', '-print_certs' ],
+                           \$pkcs7, \$out, \$err ) };
+    return undef unless $out =~ /-----BEGIN CERTIFICATE-----/;
+    return $out;
+}
+
 1;
diff --git a/share/html/Crypt/GetSMIMECert.html b/share/html/Crypt/GetSMIMECert.html
new file mode 100644
index 0000000000..8fdc210e6b
--- /dev/null
+++ b/share/html/Crypt/GetSMIMECert.html
@@ -0,0 +1,96 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2021 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<& /Elements/Header,
+   Title => $title
+ &>
+<& /Elements/Tabs &>
+
+<& /Elements/ListActions, actions => \@results &>
+
+<%ARGS>
+$Transaction => undef
+$title => loc('Download S/MIME Certificate')
+</%ARGS>
+
+<%INIT>
+my @results;
+if (!$Transaction || $Transaction !~ /^\d+$/) {
+    push(@results, loc('Transaction ID must be supplied to download an S/MIME certificate.'));
+} else {
+    my $txn = RT::Transaction->new( $session{'CurrentUser'} );
+    my ($status, $msg) = $txn->Load($Transaction);
+    if (!$status) {
+        push(@results, $msg);
+    } else {
+        my $cert = RT::Crypt::SMIME->GetCertificateForTransaction($txn);
+        if (!$cert) {
+            push(@results, loc('Could not find S/MIME certificate for specified transaction'));
+        } else {
+            # We don't really need the user, but we try to get it
+            # anyway just to give the certificate a sensible filename
+            # when it is downloaded.  If we can't get the user, or the
+            # user lacks and email address and name, we just default
+            # to "smime.crt" for the download filename.
+            my $name;
+            if ($txn->Creator) {
+                my $u = RT::User->new($session{CurrentUser});
+                $u->Load($txn->Creator);
+                $name = $u->EmailAddress || $u->Name || "smime";
+            } else {
+                $name = "smime";
+            }
+            $r->content_type('application/x-x509-user-cert');
+            $r->header_out('Content-Disposition' => "attachment; filename=\"$name.crt\"");
+            $m->out($cert);
+            $m->flush_buffer;
+            $m->abort();
+        }
+    }
+}
+
+</%INIT>
diff --git a/share/html/Elements/CryptStatus b/share/html/Elements/CryptStatus
index c116145667..18ba96bb3a 100644
--- a/share/html/Elements/CryptStatus
+++ b/share/html/Elements/CryptStatus
@@ -110,18 +110,18 @@ sub VerifyTooltip {
 
 # Generate a download link for the public key
 sub KeyDownloadLink {
-    my ($protocol, $line) = @_;
+    my ($Message, $protocol, $line) = @_;
     my $txt = '';
     if ($protocol eq 'GnuPG') {
         if ($line->{Fingerprint} && $line->{Fingerprint} !~ /[^0-9A-F]/i) {
             $txt = '<a href="' . RT->Config->Get('WebPath') . '/Crypt/GetGPGPubkey.html?Fingerprint=' . $line->{Fingerprint} . '"> ' . loc('(Download Public Key)') . '</a>';
         }
+    } elsif ($protocol eq 'SMIME') {
+        if ($Message && $Message->TransactionObj && $Message->TransactionObj->Creator) {
+            $txt = '<a href="' . RT->Config->Get('WebPath') .  '/Crypt/GetSMIMECert.html?Transaction=' . $Message->TransactionObj->Id . '"> ' . loc('(Download S/MIME Certificate)') . '</a>';
+        }
     }
 
-    # There isn't really a feasible way to download the S/MIME
-    # certificate, unfortunately.  However, since RT makes the
-    # original message available, the S/MIME cert could be
-    # extracted from that if necessary.
     return $txt;
 }
 
@@ -224,7 +224,7 @@ foreach my $run ( @runs ) {
             push @messages, {
                 Tag     => $protocol,
                 Classes => ['verify', lc $line->{Status}, 'trust-'.($line->{Trust} || 'UNKNOWN')],
-                Value   => '<span title="' . $m->interp->apply_escapes(VerifyTooltip($protocol, $line), 'h') . '">' . $m->interp->apply_escapes(loc( $line->{'Message'} ), 'h') . '</span>' . KeyDownloadLink($protocol, $line),
+                Value   => '<span title="' . $m->interp->apply_escapes(VerifyTooltip($protocol, $line), 'h') . '">' . $m->interp->apply_escapes(loc( $line->{'Message'} ), 'h') . '</span>' . KeyDownloadLink($Message, $protocol, $line),
             };
         }
         else {

commit 6d6304741f8b15466261ea010a944a498e258a8a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat Feb 6 07:34:56 2021 +0800

    Fix user PrivateKey test of setting it to an invalid value
    
    Previously it wrongly passed because $form wasn't updated after
    submission.

diff --git a/t/web/admin_user.t b/t/web/admin_user.t
index 2504bc8ff1..4ada807d94 100644
--- a/t/web/admin_user.t
+++ b/t/web/admin_user.t
@@ -72,8 +72,7 @@ $m->submit_form_ok(
     },
     'submit PrivateKey form'
 );
-is( $form->find_input('PrivateKey')->value,
-    'C798591AA831DBFB', 'set private key' );
+$m->text_contains(q{Invalid key C798591AA831DBFB for address 'rt-test at example.com'});
 
 
 diag "Test user searches";

commit 09ad19c38b0b4b382faa85e8767315f3be6ff4e4
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat Feb 6 06:33:17 2021 +0800

    Switch from key to fingerprint for user PrivateKey
    
    This is consistent with other GnuPG key selects.

diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 90bf6c3700..2b09cba9af 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -2091,7 +2091,23 @@ sub PrivateKey {
     }
 
     my $key = $self->FirstAttribute('PrivateKey') or return undef;
-    return $key->Content;
+    my $content = $key->Content;
+
+    if ( length $content < 40 ) {
+        # not fingerprint, try to update it
+        my %tmp = RT::Crypt->GetKeysForSigning( Signer => $content, Protocol => 'GnuPG' );
+        if ( !$tmp{exit_code} && $tmp{info} && $tmp{info}[0] ) {
+            my $user = RT::User->new( RT->SystemUser );
+            $user->Load( $self->Id );
+            $user->SetPrivateKey( $tmp{info}[0]{Fingerprint} );
+            return $tmp{info}[0]{Fingerprint};
+        }
+        else {
+            RT->Logger->warning("Couldn't find private key for $content");
+        }
+    }
+
+    return $content;
 }
 
 sub SetPrivateKey {
@@ -2117,6 +2133,8 @@ sub SetPrivateKey {
         my %tmp = RT::Crypt->GetKeysForSigning( Signer => $key, Protocol => 'GnuPG' );
         return (0, $self->loc("No such key or it's not suitable for signing"))
             if $tmp{'exit_code'} || !$tmp{'info'};
+        # In case $key is key id instead of fingerprint
+        $key = $tmp{'info'}[0]{Fingerprint};
     }
 
     my ($status, $msg) = $self->SetAttribute(
diff --git a/share/html/Admin/Users/Keys.html b/share/html/Admin/Users/Keys.html
index 36b9a7e353..bbef3d376f 100644
--- a/share/html/Admin/Users/Keys.html
+++ b/share/html/Admin/Users/Keys.html
@@ -101,7 +101,7 @@ my $email = $UserObj->EmailAddress;
 
 if (RT::Config->Get('GnuPG')->{Enable}) {
     my %keys_meta = RT::Crypt->GetKeysForSigning( Signer => $email, Protocol => 'GnuPG' );
-    @potential_keys = map $_->{'Key'}, @{ $keys_meta{'info'} || [] };
+    @potential_keys = map $_->{'Fingerprint'}, @{ $keys_meta{'info'} || [] };
 
     $ARGS{'PrivateKey'} = $m->comp('/Widgets/Form/Select:Process',
         Name      => 'PrivateKey',

commit e611760cfdd74301f6dc00283a07179a11fcaa38
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat Feb 6 07:37:06 2021 +0800

    Update tests for the "key => fingerprint" change of PrivateKey select

diff --git a/t/security/CVE-2012-4735-sign-any-key.t b/t/security/CVE-2012-4735-sign-any-key.t
index 78e9d1bace..16b22fde97 100644
--- a/t/security/CVE-2012-4735-sign-any-key.t
+++ b/t/security/CVE-2012-4735-sign-any-key.t
@@ -19,7 +19,7 @@ my %secret_keys;
     for my $key (@{$info{info}}) {
         my $user = $key->{User}[0]{String};
         $user = (Email::Address->parse( $user ))[0]->address;
-        $secret_keys{$user} = $key->{Key};
+        $secret_keys{$user} = $key->{Fingerprint};
     }
 }
 
diff --git a/t/web/admin_user.t b/t/web/admin_user.t
index 4ada807d94..1079598f8c 100644
--- a/t/web/admin_user.t
+++ b/t/web/admin_user.t
@@ -43,7 +43,7 @@ is( $form->find_input('PrivateKey')->value,
     '__empty_value__', 'default no private key' );
 $m->submit_form_ok(
     {
-        fields => { PrivateKey => 'D328035D84881F1B' },
+        fields => { PrivateKey => 'F0CB3B482CFA485680A4A0BDD328035D84881F1B' },
         button => 'Update',
     },
     'submit PrivateKey form'
@@ -52,7 +52,7 @@ $m->submit_form_ok(
 $m->content_contains('Set private key');
 $form = $m->form_with_fields('PrivateKey');
 is( $form->find_input('PrivateKey')->value,
-    'D328035D84881F1B', 'set private key' );
+    'F0CB3B482CFA485680A4A0BDD328035D84881F1B', 'set private key' );
 $m->submit_form_ok(
     {
         fields => { PrivateKey => '__empty_value__' },

-----------------------------------------------------------------------


More information about the rt-commit mailing list