[Rt-commit] rt branch, 4.2.14-releng, updated. rt-4.2.14rc2-1-g96075fc

[Shawn Moore]

The branch, 4.2.14-releng has been updated
       via  96075fc4169e164a81e527110722e16bcae0d40a (commit)
      from  74e13acf775e116d6af21809fe9c999aa8ece263 (commit)

Summary of changes:
 lib/RT/User.pm |  2 +-
 lib/RT/Util.pm | 20 ++++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

- Log -----------------------------------------------------------------
commit 96075fc4169e164a81e527110722e16bcae0d40a
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Mon Jul 10 11:48:28 2017 -0400

    Add a "binary" option to opt out of UTF8 encoding
    
    The SHA256 branch of IsPassword generates binary values to compare,
    which may lead to comparing two strings with a different number of
    Unicode characters, even when both strings have 26 octets (since UTF8 is
    a variable-length encoding). This triggers an error in constant_time_eq
    which demands both strings are the same length.
    
    When comparing binary values pass this flag to avoid treating the
    inputs as UTF8.

diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 4e450b9..b969d24 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -1012,7 +1012,7 @@ sub IsPassword {
         my $salt = substr($hash, 0, 4, "");
         return 0 unless RT::Util::constant_time_eq(
             substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26),
-            $hash
+            $hash, 1
         );
     } elsif (length $stored == 32) {
         # Hex nonsalted-md5
diff --git a/lib/RT/Util.pm b/lib/RT/Util.pm
index 317a208..06cd046 100644
--- a/lib/RT/Util.pm
+++ b/lib/RT/Util.pm
@@ -166,6 +166,9 @@ 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.
 
+Strings that should be treated as binary octets rather than Unicode text
+should pass a true value for the binary flag.
+
 This code has been tested to do what it claims. Do not change it without
 thorough statistical timing analysis to validate the changes.
 
@@ -177,7 +180,7 @@ B<https://en.wikipedia.org/wiki/Timing_attack>
 =cut
 
 sub constant_time_eq {
-    my ($a, $b) = @_;
+    my ($a, $b, $binary) = @_;
 
     my $result = 0;
 
@@ -191,9 +194,18 @@ sub constant_time_eq {
         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));
+        my (@a_octets, @b_octets);
+
+        if ($binary) {
+            @a_octets = ord($a_char);
+            @b_octets = ord($b_char);
+        }
+        else {
+            # encode() is set to die on malformed
+            @a_octets = unpack("C*", encode('UTF-8', $a_char, Encode::FB_CROAK));
+            @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++) {

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