[Rt-commit] rt branch, 4.4/support-openssl-crl-check, updated. rt-4.4.4-163-g5d180841f2
Dianne Skoll
dianne at bestpractical.com
Fri Nov 20 12:14:23 EST 2020
The branch, 4.4/support-openssl-crl-check has been updated
via 5d180841f2c5eaa951b55ffd500603c801fa7ec3 (commit)
from 6c31f8b5cba0b13d81c0a423b1aadbd49f172e4f (commit)
Summary of changes:
etc/RT_Config.pm.in | 9 +++
lib/RT/Config.pm | 13 ++++
lib/RT/Crypt/SMIME.pm | 96 ++++++++++++++++-----------
t/crypt/smime/{crl-download.t => crl-check.t} | 8 ++-
t/crypt/smime/revoked.t | 21 +++++-
5 files changed, 103 insertions(+), 44 deletions(-)
rename t/crypt/smime/{crl-download.t => crl-check.t} (80%)
- Log -----------------------------------------------------------------
commit 5d180841f2c5eaa951b55ffd500603c801fa7ec3
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Fri Nov 20 12:00:39 2020 -0500
Update as per code review:
1: Add DownloadCRL and DownloadCRLTimeout options to %SMIME config hash
2: Don't use OpenSSL's -crl_download option; always download ourselves using
LWP so we can control the timeout.
3: Never download a CRL from an untrusted CA
4: Skip CRL download tests unles environment variable RT_TEST_DOWNLOAD_CRL is s et to true.
diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index f8f7866c02..bcd23dd8d7 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -3060,6 +3060,13 @@ function, or a hash (to look up by address). If the hash is used, the
Set C<OtherCertificatesToSend> to path to a PEM-formatted certificate file.
Certificates in the file will be include in outgoing signed emails.
+Set C<DownloadCRL> to a true value to have RT check for revoked certificates
+using OCSP or downloading a CRL if no OCSP server is given. By default,
+C<DownloadCRL> is disabled.
+
+Set C<DownloadCRLTimeout> to the timeout in seconds for downloading a CRL.
+The default timeout is 30 seconds.
+
See L<RT::Crypt::SMIME> for details.
=back
@@ -3074,6 +3081,8 @@ Set( %SMIME,
AcceptUntrustedCAs => undef,
Passphrase => undef,
OtherCertificatesToSend => undef,
+ DownloadCRL => 0,
+ DownloadCRLTimeout => 30,
);
=head2 GnuPG configuration
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 431a12ca22..95c5260305 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -839,6 +839,19 @@ our %META;
delete $opt->{CAPath};
}
}
+
+ if (!exists $opt->{DownloadCRL} ) {
+ $opt->{DownloadCRL} = 0;
+ }
+ if (!exists $opt->{DownloadCRLTimeout} ) {
+ $opt->{DownloadCRLTimeout} = 0;
+ }
+ if ($opt->{DownloadCRL} && ! RT::Crypt::SMIME->SupportsCRLfile) {
+ $opt->{DownloadCRL} = 0;
+ $RT::Logger->warn(
+ "Your version of OpenSSL does not support the -CRLfile option; disabling \$SMIME{DownloadCRL}"
+ );
+ }
},
},
GnuPG => {
diff --git a/lib/RT/Crypt/SMIME.pm b/lib/RT/Crypt/SMIME.pm
index 9521e39c80..9a72cf4057 100644
--- a/lib/RT/Crypt/SMIME.pm
+++ b/lib/RT/Crypt/SMIME.pm
@@ -64,8 +64,8 @@ use String::ShellQuote 'shell_quote';
use LWP;
# This will be set to a true value by Probe
-# if "openssl verify" supports the -crl_download option
-our $OpenSSL_Supports_CRL_Download;
+# if "openssl verify" supports the -CRLfile option
+our $OpenSSL_Supports_CRLfile;
=head1 NAME
@@ -87,6 +87,8 @@ You should start from reading L<RT::Crypt>.
'' => 'fallback',
},
OtherCertificatesToSend => '/opt/rt4/var/data/smime/other-certs.pem',
+ DownloadCRL => 0,
+ DownloadCRLTimeout => 30,
);
=head3 OpenSSL
@@ -133,6 +135,19 @@ Certificates in the file will be include in outgoing signed emails.
Depending on use cases, you might need to include a chain of certificates so
receiving agents can verify. CA could also be included here.
+=head3 DownloadCRL
+
+A boolean option that determines whether or not we attempt to check if
+a certificate is revoked by downloading a CRL or checking against an
+OCSP URL. The default value is false (do not check.) Additionally,
+if AcceptUntrustedCAs is true, RT will I<never> download a CRL or
+check an OCSP URL for a certificate signed by an untrusted CA.
+
+=head3 DownloadCRLTimeout
+
+Timeout in seconds for downloading a CRL or issuer certificate for
+OCSP checking. The default is 30 seconds.
+
=head2 Keyring configuration
RT looks for keys in the directory configured in the L</Keyring> option
@@ -219,11 +234,11 @@ sub Probe {
} else {
($buf, $err) = ('', '');
# Interrogate openssl verify command to see if it supports
- # the -crl_download option.
+ # the -CRLfile option.
safe_run_child { run3( [$bin, 'verify', '-help'],
\undef, \$buf, \$err) };
- if ($err =~ /crl_download/) {
- $OpenSSL_Supports_CRL_Download = 1;
+ if ($err =~ /-CRLfile/) {
+ $OpenSSL_Supports_CRLfile = 1;
}
return 1;
}
@@ -973,54 +988,46 @@ sub GetCertificateInfo {
stderr => ''
);
+ # First, check if the certificate verifies without checking
+ # revocation status
+ $self->RunOpenSSLVerify($PEM, \%res);
+
+ if ($res{info}[0]{TrustLevel} != 2) {
+ # Not signed by trusted CA; return
+ return %res;
+ }
+
+ # If we're not configured to check CRLs, just return
+ # what we have.
+ return %res unless (RT::Config->Get('SMIME')->{'DownloadCRL'});
+
# Check if certificate has been revoked using OCSP if the cert has
# an OCSP URL. Unfortunately, Crypt::X509 doesn't let us query
# for OCSP URLs, so we need to run OpenSSL.
my $ocsp_result = $self->CheckRevocationUsingOCSP($PEM, \%res);
if ($ocsp_result) {
+ # We got a definitive result from OCSP; return
return %res;
}
+ # OCSP didn't give us a result. Try downloading CRL
my $note_about_crl_download = '';
- if (!defined($ocsp_result)) {
- if ($OpenSSL_Supports_CRL_Download) {
- $self->RunOpenSSLVerify($PEM, \%res, '-crl_check', '-crl_download');
- if ($res{stderr} !~ /unable to get certificate CRL/) {
- return %res;
- }
- # If OpenSSL said: "Error loading CRL from URL", try to fetch
- # the URL ourselves. OpenSSL gives that misleading message if
- # the CRL is in DER format rather than PEM. So we download
- # it ourselves, try to convert from DER to PEM, and then
- # rerun with -CRLfile <crl.pem>
- if ($res{stderr} =~ /Error loading CRL from (https?:.*)/i) {
- my $tmpdir = File::Temp::tempdir( TMPDIR => 1, CLEANUP => 1 );
- my $crl_file = $self->DownloadAndConvertCRLToPEM($tmpdir, $1);
- if ($crl_file) {
- $self->RunOpenSSLVerify($PEM, \%res, '-crl_check', '-CRLfile', $crl_file);
- return %res;
- }
+ if ($OpenSSL_Supports_CRLfile) {
+ # We fetch the CRL file ourselves using LWP rather than
+ # using OpenSSL's -crl_download option so we can
+ # control the timeout.
+ my $tmpdir = File::Temp::tempdir( TMPDIR => 1, CLEANUP => 1 );
+ my ($url) = @{$cert->CRLDistributionPoints};
+ if ($url) {
+ my $crl_file = $self->DownloadAndConvertCRLToPEM($tmpdir, $url);
+ if ($crl_file) {
+ $self->RunOpenSSLVerify($PEM, \%res, '-crl_check', '-CRLfile', $crl_file);
+ } else {
+ $res{info}[0]{Trust} .= " (NOTE: Unable to download CRL)";
}
}
-
- # If we got here and we support -crl_download, then clearly
- # the CRL download failed, so make a note of that
- if ($OpenSSL_Supports_CRL_Download) {
- $note_about_crl_download = " (NOTE: Unable to download CRL)";
- }
}
- # One of these cases has happened to get here:
- # 1) We don't support -crl_download,
- # 2) We do support -crl_download but it failed
- # 3) OCSP told us that the certificate is still good
- #
- # Run without -crl_check -crl_download
-
- $self->RunOpenSSLVerify($PEM, \%res);
- if (($res{info}[0]{TrustTerse} // '') eq 'full') {
- $res{info}[0]{Trust} .= $note_about_crl_download;
- }
return %res;
}
@@ -1087,6 +1094,8 @@ sub DownloadAndConvertCRLToPEM
{
my ($self, $tmpdir, $url) = @_;
my $ua = LWP::UserAgent->new(env_proxy => 1);
+ $ua->timeout(RT::Config->Get('SMIME')->{DownloadCRLTimeout});
+
my $resp = $ua->get($url);
return undef unless $resp->is_success;
@@ -1131,6 +1140,7 @@ sub CheckRevocationUsingOCSP
my $tmpdir = File::Temp::tempdir( TMPDIR => 1, CLEANUP => 1 );
my $issuer = File::Spec->catfile($tmpdir, 'issuer.crt');
my $ua = LWP::UserAgent->new(env_proxy => 1);
+ $ua->timeout(RT::Config->Get('SMIME')->{DownloadCRLTimeout});
my $resp = $ua->get($issuer_url);
return undef unless $resp->is_success;
@@ -1175,4 +1185,10 @@ sub CheckRevocationUsingOCSP
return undef;
}
+# Accessor function to query if OpenSSL supports -CRLfile
+# without having to know a package variable name.
+sub SupportsCRLfile {
+ return $OpenSSL_Supports_CRLfile;
+};
+
1;
diff --git a/t/crypt/smime/crl-download.t b/t/crypt/smime/crl-check.t
similarity index 80%
rename from t/crypt/smime/crl-download.t
rename to t/crypt/smime/crl-check.t
index 08d19cc17a..89e01cc0c8 100644
--- a/t/crypt/smime/crl-download.t
+++ b/t/crypt/smime/crl-check.t
@@ -13,13 +13,17 @@ RT->Config->Set('SMIME', Enable => 1,
OpenSSL => $openssl,
Keyring => $keyring,
CAPath => $ca,
+ DownloadCRL => 1,
);
RT::Test::SMIME->import_key('sender-crl at example.com');
+if (!RT::Crypt::SMIME->SupportsCRLfile) {
+ RT::Test::plan( skip_all => 'This version of openssl does not support the -CRLfile option');
+}
-if (!$::RT::Crypt::SMIME::OpenSSL_Supports_CRL_Download) {
- RT::Test::plan( skip_all => 'This version of openssl does not support the -crl_download option');
+if (!$ENV{RT_TEST_DOWNLOAD_CRL}) {
+ RT::Test::plan( skip_all => 'Skipping tests that would download a CRL because RT_TEST_DOWNLOAD_CRL environment variable not set to 1');
}
my $crt;
diff --git a/t/crypt/smime/revoked.t b/t/crypt/smime/revoked.t
index e88004c61a..3c23577c50 100644
--- a/t/crypt/smime/revoked.t
+++ b/t/crypt/smime/revoked.t
@@ -13,13 +13,18 @@ RT->Config->Set('SMIME', Enable => 1,
OpenSSL => $openssl,
Keyring => $keyring,
CAPath => $ca,
+ DownloadCRL => 1,
);
RT::Test::SMIME->import_key('revoked at example.com');
-if (!$::RT::Crypt::SMIME::OpenSSL_Supports_CRL_Download) {
- RT::Test::plan( skip_all => 'This version of openssl does not support the -crl_download option');
+if (!RT::Crypt::SMIME->SupportsCRLfile) {
+ RT::Test::plan( skip_all => 'This version of openssl does not support the -CRLfile option');
+}
+
+if (!$ENV{RT_TEST_DOWNLOAD_CRL}) {
+ RT::Test::plan( skip_all => 'Skipping tests that would download a CRL because RT_TEST_DOWNLOAD_CRL environment variable not set to 1');
}
my $crt;
@@ -47,4 +52,16 @@ is ($res{info}[0]{TrustTerse}, 'none (revoked certificate)', 'TrustTerse indicat
is ($res{info}[0]{TrustTerse}, 'none (revoked certificate)', 'TrustTerse indicates revoked certificate');
}
+# If we have not enabled CRL downloading, the cert should verify
+RT->Config->Set('SMIME', Enable => 1,
+ Passphrase => {'revoked\@example.com' => '123456'},
+ OpenSSL => $openssl,
+ Keyring => $keyring,
+ CAPath => $ca,
+ DownloadCRL => 0,
+);
+%res = RT::Crypt::SMIME->GetCertificateInfo(Certificate => $crt);
+is ($res{info}[0]{Trust}, 'Signed by trusted CA DigiCert SHA2 Secure Server CA');
+is ($res{info}[0]{TrustTerse}, 'full');
+
done_testing;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list