[Rt-commit] rt branch, 4.2.14-releng, updated. rt-4.2.14rc1-13-g74e13ac
Shawn Moore
shawn at bestpractical.com
Thu Jun 15 14:54:51 EDT 2017
The branch, 4.2.14-releng has been updated
via 74e13acf775e116d6af21809fe9c999aa8ece263 (commit)
via 13281fcbef58fef5342e6251cfaa0fdf05e9b0a0 (commit)
via c98487eb88e7df552bd555d134f98d2b4bb62f3a (commit)
via 141c999a863bda66efe733e5f0842ece304be324 (commit)
via dc8dcd828cc05ec753e14b429c0d7864ed1ef033 (commit)
via 9d43aefa44c75e5d368f02eb6631218594b2a8a0 (commit)
via 2ac80e537b7972f16de40743a333824fcb792c9d (commit)
via 9d1d82f0291a1528c667fdd8c54ae8616ee17b11 (commit)
via 87fdd79647f61f60dcb69738bd8e6eae05889d29 (commit)
via 5e174fe2f2c9bd282b38e6b9c3b353e48143e81d (commit)
via 3143a4cf1b4f81354a7a47977d3bc2ca4de1c461 (commit)
via b9c2968bfcb6383876e399c0604dbe390939c517 (commit)
via aebb36bdf32cf1166d945403ab0bae8e034434a9 (commit)
from c26271fc4c6b768ba6be8b610d57b559e86a57ec (commit)
Summary of changes:
lib/RT.pm | 4 +++
lib/RT/Config.pm | 8 +++++
lib/RT/Interface/Web.pm | 4 +--
lib/RT/User.pm | 38 +++++++++++++++++------
lib/RT/Util.pm | 54 +++++++++++++++++++++++++++++++++
sbin/rt-test-dependencies.in | 2 +-
share/html/Dashboards/Subscription.html | 2 +-
share/html/Ticket/Attachment/dhandler | 6 ++--
t/web/csrf.t | 14 ++++-----
9 files changed, 110 insertions(+), 22 deletions(-)
- Log -----------------------------------------------------------------
commit 87fdd79647f61f60dcb69738bd8e6eae05889d29
Author: Aaron Kondziela <aaron at bestpractical.com>
Date: Tue Jan 24 18:20:28 2017 -0500
Fix timing sidechannel vulnerability in password checking
"eq" operators for comparing against passwords are replaced by a new
RT::Util::constant_time_eq to resolve a timing sidechannel vulnerability.
This addresses CVE-2017-5361.
Fixes: T#161960
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 1c0cc7c..4e450b9 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -84,6 +84,7 @@ use RT::Principals;
use RT::ACE;
use RT::Interface::Email;
use Text::Password::Pronounceable;
+use RT::Util;
sub _OverlayAccessible {
{
@@ -989,11 +990,17 @@ sub IsPassword {
# If it's a new-style (>= RT 4.0) password, it starts with a '!'
my (undef, $method, @rest) = split /!/, $stored;
if ($method eq "bcrypt") {
- return 0 unless $self->_GeneratePassword_bcrypt($value, @rest) eq $stored;
+ return 0 unless RT::Util::constant_time_eq(
+ $self->_GeneratePassword_bcrypt($value, @rest),
+ $stored
+ );
# Upgrade to a larger number of rounds if necessary
return 1 unless $rest[0] < RT->Config->Get('BcryptCost');
} elsif ($method eq "sha512") {
- return 0 unless $self->_GeneratePassword_sha512($value, @rest) eq $stored;
+ return 0 unless RT::Util::constant_time_eq(
+ $self->_GeneratePassword_sha512($value, @rest),
+ $stored
+ );
} else {
$RT::Logger->warn("Unknown hash method $method");
return 0;
@@ -1003,16 +1010,28 @@ sub IsPassword {
my $hash = MIME::Base64::decode_base64($stored);
# Decoding yields 30 byes; first 4 are the salt, the rest are substr(SHA256,0,26)
my $salt = substr($hash, 0, 4, "");
- return 0 unless substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26) eq $hash;
+ return 0 unless RT::Util::constant_time_eq(
+ substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26),
+ $hash
+ );
} elsif (length $stored == 32) {
# Hex nonsalted-md5
- return 0 unless Digest::MD5::md5_hex(Encode::encode( "UTF-8", $value)) eq $stored;
+ return 0 unless RT::Util::constant_time_eq(
+ Digest::MD5::md5_hex(Encode::encode( "UTF-8", $value)),
+ $stored
+ );
} elsif (length $stored == 22) {
# Base64 nonsalted-md5
- return 0 unless Digest::MD5::md5_base64(Encode::encode( "UTF-8", $value)) eq $stored;
+ return 0 unless RT::Util::constant_time_eq(
+ Digest::MD5::md5_base64(Encode::encode( "UTF-8", $value)),
+ $stored
+ );
} elsif (length $stored == 13) {
# crypt() output
- return 0 unless crypt(Encode::encode( "UTF-8", $value), $stored) eq $stored;
+ return 0 unless RT::Util::constant_time_eq(
+ crypt(Encode::encode( "UTF-8", $value), $stored),
+ $stored
+ );
} else {
$RT::Logger->warning("Unknown password form");
return 0;
@@ -1108,19 +1127,20 @@ sub GenerateAuthString {
=head3 ValidateAuthString
-Takes auth string and protected string. Returns true is protected string
+Takes auth string and protected string. Returns true if protected string
has been protected by user's L</AuthToken>. See also L</GenerateAuthString>.
=cut
sub ValidateAuthString {
my $self = shift;
- my $auth_string = shift;
+ my $auth_string_to_validate = shift;
my $protected = shift;
my $str = Encode::encode( "UTF-8", $self->AuthToken . $protected );
+ my $valid_auth_string = substr(Digest::MD5::md5_hex($str),0,16);
- return $auth_string eq substr(Digest::MD5::md5_hex($str),0,16);
+ return RT::Util::constant_time_eq( $auth_string_to_validate, $valid_auth_string );
}
=head2 SetDisabled
diff --git a/lib/RT/Util.pm b/lib/RT/Util.pm
index 58eb9fd..317a208 100644
--- a/lib/RT/Util.pm
+++ b/lib/RT/Util.pm
@@ -54,6 +54,8 @@ use warnings;
use base 'Exporter';
our @EXPORT = qw/safe_run_child mime_recommended_filename/;
+use Encode qw/encode/;
+
sub safe_run_child (&) {
my $our_pid = $$;
@@ -150,6 +152,58 @@ sub assert_bytes {
}
+=head2 C<constant_time_eq($a, $b)>
+
+Compares two strings for equality in constant-time. Replacement for the C<eq>
+operator designed to avoid timing side-channel vulnerabilities. Returns zero
+or one.
+
+This is intended for use in cryptographic subsystems for comparing well-formed
+data such as hashes - not for direct use with user input or as a general
+replacement for the C<eq> operator.
+
+The two string arguments B<MUST> be of equal length. If the lengths differ,
+this function will call C<die()>, as proceeding with execution would create
+a timing vulnerability. Length is defined by characters, not bytes.
+
+This code has been tested to do what it claims. Do not change it without
+thorough statistical timing analysis to validate the changes.
+
+Added to resolve CVE-2017-5361
+
+For more on timing attacks, see this Wikipedia article:
+B<https://en.wikipedia.org/wiki/Timing_attack>
+
+=cut
+
+sub constant_time_eq {
+ my ($a, $b) = @_;
+
+ my $result = 0;
+
+ # generic error message avoids potential information leaks
+ my $generic_error = "Cannot compare values";
+ die $generic_error unless defined $a and defined $b;
+ die $generic_error unless length $a == length $b;
+ die $generic_error if ref($a) or ref($b);
+
+ for (my $i = 0; $i < length($a); $i++) {
+ my $a_char = substr($a, $i, 1);
+ my $b_char = substr($b, $i, 1);
+
+ # encode() is set to die on malformed
+ my @a_octets = unpack("C*", encode('UTF-8', $a_char, Encode::FB_CROAK));
+ my @b_octets = unpack("C*", encode('UTF-8', $b_char, Encode::FB_CROAK));
+ die $generic_error if (scalar @a_octets) != (scalar @b_octets);
+
+ for (my $j = 0; $j < scalar @a_octets; $j++) {
+ $result |= $a_octets[$j] ^ $b_octets[$j];
+ }
+ }
+ return 0 + not $result;
+}
+
+
RT::Base->_ImportOverlays();
1;
commit 9d1d82f0291a1528c667fdd8c54ae8616ee17b11
Merge: 5e1d28f b9c2968
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 1 00:59:40 2017 +0000
Merge branch 'security/4.0/csrf-leak' into security/4.2.14-releng
commit 2ac80e537b7972f16de40743a333824fcb792c9d
Merge: 9d1d82f 5e174fe
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 1 01:00:30 2017 +0000
Merge branch 'security/4.0/email-parse-dos' into security/4.2.14-releng
diff --cc sbin/rt-test-dependencies.in
index f579b0f,7882a37..8d160dd
--- a/sbin/rt-test-dependencies.in
+++ b/sbin/rt-test-dependencies.in
@@@ -206,70 -196,85 +206,70 @@@ Devel::StackTrace 1.1
Digest::base
Digest::MD5 2.27
Digest::SHA
- Email::Address 1.897
-DBI 1.37
-Class::ReturnValue 0.40
-DBIx::SearchBuilder 1.59
-Text::Template 1.44
++Email::Address 1.908
+Email::Address::List 0.02
+Encode 2.64
+Errno
+File::Glob
File::ShareDir
File::Spec 0.8
+File::Temp 0.19
+HTML::Entities
+HTML::FormatText::WithLinks 0.14
+HTML::FormatText::WithLinks::AndTables
+HTML::Mason 1.43
+HTML::Mason::PSGIHandler 0.52
HTML::Quoted
+HTML::RewriteAttributes 0.05
HTML::Scrubber 0.08
-HTML::TreeBuilder
-HTML::FormatText
-Log::Dispatch 2.23
-Sys::Syslog 0.16
+HTTP::Message 6.0
+IPC::Run3
+JSON
+LWP::Simple
+List::MoreUtils
Locale::Maketext 1.06
+Locale::Maketext::Fuzzy 0.11
Locale::Maketext::Lexicon 0.32
-Locale::Maketext::Fuzzy
-MIME::Entity 5.425
+Log::Dispatch 2.30
+Mail::Header 2.12
Mail::Mailer 1.57
-Email::Address 1.908
-Text::Wrapper
-Time::ParseDate
-Time::HiRes
-File::Temp 0.19
-Text::Quoted 2.02
-Tree::Simple 1.04
-UNIVERSAL::require
-Regexp::Common
-Scalar::Util
+MIME::Entity 5.504
+Module::Refresh 0.03
Module::Versions::Report 1.05
-Cache::Simple::TimedExpiry
Encode 2.64
-CSS::Squish 0.06
-File::Glob
-Devel::StackTrace 1.19
-Text::Password::Pronounceable
-Devel::GlobalDestruction
-List::MoreUtils
Net::CIDR
+Plack 1.0002
+Plack::Handler::Starlet
+Regexp::Common
Regexp::Common::net::CIDR
Regexp::IPv6
-.
-
-$deps{'MASON'} = [ text_to_hash( << '.') ];
-HTML::Mason 1.43
-Errno
-Digest::MD5 2.27
-CGI::Cookie 1.20
+Role::Basic 0.12
+Scalar::Util
Storable 2.08
-Apache::Session 1.53
-XML::RSS 1.05
+Symbol::Global::Name 0.04
+Sys::Syslog 0.16
+Text::Password::Pronounceable
+Text::Quoted 2.07
+Text::Template 1.44
Text::WikiFormat 0.76
-CSS::Squish 0.06
-Devel::StackTrace 1.19
-JSON
-IPC::Run3
-.
-
-$deps{'PSGI'} = [ text_to_hash( << '.') ];
-CGI 3.38
-CGI::PSGI 0.12
-HTML::Mason::PSGIHandler 0.52
-Plack 0.9971
-Plack::Handler::Starlet
-CGI::Emulate::PSGI
+Text::Wrapper
+Time::HiRes
+Time::ParseDate
+Tree::Simple 1.04
+UNIVERSAL::require
+XML::RSS 1.05
.
-set_dep( PSGI => CGI => 4.00 ) if $] > 5.019003;
-
+set_dep( CORE => 'Symbol::Global::Name' => 0.05 ) if $] >= 5.019003;
+set_dep( CORE => CGI => 4.00 ) if $] > 5.019003;
$deps{'MAILGATE'} = [ text_to_hash( << '.') ];
-Getopt::Long
-LWP::UserAgent
-Pod::Usage
-.
-
-$deps{'SSL-MAILGATE'} = [ text_to_hash( << '.') ];
Crypt::SSLeay
-Net::SSL
-LWP::UserAgent 6.0
+Getopt::Long
LWP::Protocol::https
+LWP::UserAgent 6.0
Mozilla::CA
+Net::SSL
+Pod::Usage
.
$deps{'CLI'} = [ text_to_hash( << '.') ];
commit 9d43aefa44c75e5d368f02eb6631218594b2a8a0
Merge: 2ac80e5 aebb36b
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 1 01:04:30 2017 +0000
Merge branch 'security/4.0/attachment-xss-fix' into security/4.2.14-releng
diff --cc share/html/Ticket/Attachment/dhandler
index a711351,1cdedd7..afa471c
--- a/share/html/Ticket/Attachment/dhandler
+++ b/share/html/Ticket/Attachment/dhandler
@@@ -46,58 -46,52 +46,60 @@@
%#
%# END BPS TAGGED BLOCK }}}
<%perl>
- my ($ticket, $trans,$attach, $filename);
- my $arg = $m->dhandler_arg; # get rest of path
- if ($arg =~ m{^(\d+)/(\d+)}) {
- $trans = $1;
- $attach = $2;
- }
- else {
- Abort("Corrupted attachment URL.");
- }
- my $AttachmentObj = RT::Attachment->new($session{'CurrentUser'});
- $AttachmentObj->Load($attach) || Abort("Attachment '$attach' could not be loaded");
+my ( $ticket, $trans, $attach, $filename );
+my $arg = $m->dhandler_arg; # get rest of path
+if ( $arg =~ m{^(\d+)/(\d+)} ) {
+ $trans = $1;
+ $attach = $2;
+}
+else {
+ Abort("Corrupted attachment URL.");
+}
+my $AttachmentObj = RT::Attachment->new( $session{'CurrentUser'} );
+$AttachmentObj->Load($attach) || Abort("Attachment '$attach' could not be loaded");
+unless ( $AttachmentObj->id ) {
+ Abort("Bad attachment id. Couldn't find attachment '$attach'\n");
+}
+unless ( $AttachmentObj->TransactionId() == $trans ) {
+ Abort("Bad transaction number for attachment. $trans should be". $AttachmentObj->TransactionId() . "\n");
+}
- unless ($AttachmentObj->id) {
- Abort("Bad attachment id. Couldn't find attachment '$attach'\n");
- }
- unless ($AttachmentObj->TransactionId() == $trans ) {
- Abort("Bad transaction number for attachment. $trans should be".$AttachmentObj->TransactionId() ."\n");
+my $content = $AttachmentObj->OriginalContent;
+my $content_type = $AttachmentObj->ContentType || 'text/plain';
- if ( RT->Config->Get('AlwaysDownloadAttachments') ) {
- }
++my $attachment_regex = qr{^(image/svg\+xml|application/pdf)}i;
++if ( RT->Config->Get('AlwaysDownloadAttachments') || ($content_type =~ $attachment_regex) ) {
+ $r->headers_out->{'Content-Disposition'} = "attachment";
+}
+elsif ( !RT->Config->Get('TrustHTMLAttachments') ) {
- $content_type = 'text/plain' if ( $content_type =~ /^text\/html/i );
++ my $text_plain_regex = qr{^(text/html|application/xhtml\+xml|text/xml|application/xml)}i;
++ $content_type = 'text/plain' if ( $content_type =~ $text_plain_regex );
+}
+elsif (lc $content_type eq 'text/html') {
+ # If we're trusting and serving HTML for display not download, try to do
+ # inline <img> rewriting to be extra helpful.
+ my $count = RT::Interface::Web::RewriteInlineImages(
+ Content => \$content,
+ Attachment => $AttachmentObj,
+ );
+ RT->Logger->debug("Rewrote $count CID images when displaying original HTML attachment #$attach");
+}
- my $content_type = $AttachmentObj->ContentType || 'text/plain';
+my $enc = $AttachmentObj->OriginalEncoding || 'utf-8';
+my $iana = Encode::find_encoding($enc);
+ $iana = $iana ? $iana->mime_name : $enc;
- my $attachment_regex = qr{^(image/svg\+xml|application/pdf)}i;
- if ( RT->Config->Get('AlwaysDownloadAttachments') || ($content_type =~ $attachment_regex) ) {
- $r->headers_out->{'Content-Disposition'} = "attachment";
- }
- elsif (!RT->Config->Get('TrustHTMLAttachments')) {
- my $text_plain_regex = qr{^(text/html|application/xhtml\+xml|text/xml|application/xml)}i;
- $content_type = 'text/plain' if ( $content_type =~ $text_plain_regex );
- }
+require MIME::Types;
+my $mimetype = MIME::Types->new->type($content_type);
+unless ( $mimetype && $mimetype->isBinary ) {
+ $content_type .= ";charset=$iana";
+}
- my $enc = $AttachmentObj->OriginalEncoding || 'utf-8';
- my $iana = Encode::find_encoding( $enc );
- $iana = $iana? $iana->mime_name : $enc;
-
- require MIME::Types;
- my $mimetype = MIME::Types->new->type($content_type);
- unless ( $mimetype && $mimetype->isBinary ) {
- $content_type .= ";charset=$iana";
- }
-
- $r->content_type( $content_type );
- $m->clear_buffer();
- $m->out($AttachmentObj->OriginalContent);
- $m->abort;
+$r->content_type($content_type);
+$m->clear_buffer();
+$m->out($content);
+$m->abort;
</%perl>
<%attr>
AutoFlush => 0
commit dc8dcd828cc05ec753e14b429c0d7864ed1ef033
Merge: 9d43aef 87fdd79
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 1 01:04:38 2017 +0000
Merge branch 'security/4.2/password-timing-attack' into security/4.2.14-releng
commit 141c999a863bda66efe733e5f0842ece304be324
Merge: dc8dcd8 3143a4c
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 1 01:04:44 2017 +0000
Merge branch 'security/4.0/dash-subscription' into security/4.2.14-releng
commit c98487eb88e7df552bd555d134f98d2b4bb62f3a
Author: Aaron Kondziela <aaron at bestpractical.com>
Date: Fri Jan 27 23:53:47 2017 -0500
Correct name of config option RestrictLoginReferrer
The config option RestrictLoginReferrer was mistakenly used in Web.pm
as RestrictReferrerLogin, causing no effect when the config option was
set. We now issue an error if the incorrect name is used.
Fixes: T#171121
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 5312852..25de4f2 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -143,6 +143,14 @@ can be set for each config optin:
our %META;
%META = (
# General user overridable options
+ RestrictReferrerLogin => {
+ PostLoadCheck => sub {
+ my $self = shift;
+ if (defined($self->Get('RestrictReferrerLogin'))) {
+ RT::Logger->error("The config option 'RestrictReferrerLogin' is incorrect, and should be 'RestrictLoginReferrer' instead.");
+ }
+ },
+ },
DefaultQueue => {
Section => 'General',
Overridable => 1,
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 1393d76..268811e 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1426,7 +1426,7 @@ sub IsCompCSRFWhitelisted {
# golden. This acts on the presumption that external forms may
# hardcode a username and password -- if a malicious attacker knew
# both already, CSRF is the least of your problems.
- my $AllowLoginCSRF = not RT->Config->Get('RestrictReferrerLogin');
+ my $AllowLoginCSRF = not RT->Config->Get('RestrictLoginReferrer');
if ($AllowLoginCSRF and defined($args{user}) and defined($args{pass})) {
my $user_obj = RT::CurrentUser->new();
$user_obj->Load($args{user});
commit 13281fcbef58fef5342e6251cfaa0fdf05e9b0a0
Merge: 141c999 c98487e
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 1 01:05:49 2017 +0000
Merge branch 'security/4.2/referrer-login' into security/4.2.14-releng
commit 74e13acf775e116d6af21809fe9c999aa8ece263
Merge: c26271f 13281fc
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Jun 15 13:48:14 2017 -0400
Merge branch 'security/4.2.14-releng' into 4.2.14-releng
-----------------------------------------------------------------------
More information about the rt-commit
mailing list