Re: Patches for CVE-2011-0009
On 01/27/2011 12:43 AM, Xavier Bachelot wrote: On 01/26/2011 09:16 PM, Xavier Bachelot wrote: On 01/26/2011 12:00 AM, Xavier Bachelot wrote: Hi, I've been looking at the issue for both rt 3.6 and 3.8. I have a rather full featured patch for 3.8 and I took the Debian patch for 3.6. However, I'm not happy with 3.6, it's lacking the script to fix all the passwords. I'll try to come up with something better in the next few days. Here's my WIP for reference. Regards, Xavier Here are the updated patches against master and el5 branches. I only have an rt 3.6 to test against, so the 3.8 patch is not run time tested, but I'm confident. The only missing bit is a paragraph about the password mass-update script in the UPGRADING file for 3.6. Sorry, slightly wrong patches, it was missing the patch to the UPGRADING file. Here is a fixed one for 3.8. I've pushed the 3.6 patch to el5. http://koji.fedoraproject.org/koji/taskinfo?taskID=2744662 https://admin.fedoraproject.org/updates/rt3-3.6.10-2.el5 Ralf, Mark, I let you give a test at 3.8 on Rawhide/F14/F13 and EL6, respectively. Xavier, please don't try to rush it. I'm not trying to rush anything. I'm satisfied with the patches I have, so I've built the rpms for the OS release I'm running in production. I've indeed tested it locally. However I can't test against rawhide, F13, F14 and EL6, because I don't have any RT instance with this releases. I obviously won't commit anything to this branches and let this for people that can actually test the patches. So far, from visual inspection only, I am not necessarily opposed to your patch but I like the debian patches more. Imho, the Debian patches are incomplete. It only fixes password hashes upon user login, which is not enough to fix the security issue. You can't expect to force all users to log in and all hashes that have not been updated are still vulnerable to a brute force attack. The only real solution is to use the vulnerable-passwords script to mass update them. This script is missing with the Debian patches. My own patches were created using the commits from the 3.8-salted_password branch from RT's git repository, that I then adapted to target 3.6. Regards, Xavier -- Fedora Extras Perl SIG http://www.fedoraproject.org/wiki/Extras/SIGs/Perl perl-devel mailing list perl-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/perl-devel
Re: Patches for CVE-2011-0009
On 01/27/2011 12:31 PM, Xavier Bachelot wrote: On 01/27/2011 12:43 AM, Xavier Bachelot wrote: On 01/26/2011 09:16 PM, Xavier Bachelot wrote: On 01/26/2011 12:00 AM, Xavier Bachelot wrote: Hi, I've been looking at the issue for both rt 3.6 and 3.8. I have a rather full featured patch for 3.8 and I took the Debian patch for 3.6. However, I'm not happy with 3.6, it's lacking the script to fix all the passwords. I'll try to come up with something better in the next few days. Here's my WIP for reference. Regards, Xavier Here are the updated patches against master and el5 branches. I only have an rt 3.6 to test against, so the 3.8 patch is not run time tested, but I'm confident. The only missing bit is a paragraph about the password mass-update script in the UPGRADING file for 3.6. Sorry, slightly wrong patches, it was missing the patch to the UPGRADING file. Here is a fixed one for 3.8. I've pushed the 3.6 patch to el5. http://koji.fedoraproject.org/koji/taskinfo?taskID=2744662 https://admin.fedoraproject.org/updates/rt3-3.6.10-2.el5 Ralf, Mark, I let you give a test at 3.8 on Rawhide/F14/F13 and EL6, respectively. Xavier, please don't try to rush it. I'm not trying to rush anything. I'm satisfied with the patches I have, so I've built the rpms for the OS release I'm running in production. I've indeed tested it locally. However I can't test against rawhide, F13, F14 and EL6, because I don't have any RT instance with this releases. I obviously won't commit anything to this branches and let this for people that can actually test the patches. So far, from visual inspection only, I am not necessarily opposed to your patch but I like the debian patches more. Imho, the Debian patches are incomplete. It only fixes password hashes upon user login, This exactly why I like them. They are working transparently without user or maintainer interaction. which is not enough to fix the security issue. You can't expect to force all users to log in and all hashes that have not been updated are still vulnerable to a brute force attack. The only real solution is to use the vulnerable-passwords script to mass update them. Partially agreed. A mass update is the only way to assure this. However, one can not expect maintainers to run this script, nor can we run this script during rpm updates. That said, my current preference for Fedora 13 and 14 is a combination of * Adopting Debian's patch. * Adding the vulnerable-passwords script. For Fedora 15 backporting from bestpractical's upstream (which is, AFAIU, you did) is feasible, because upgrading Fedora's will always require maintainer interaction. In an ideal world, IMO, a solution would look differently: Launching start up would perform a mass conversion. BTW: Did you check if rt's upgrade scripts take care about this CVE (I haven't yet)? This script is missing with the Debian patches. My own patches were created using the commits from the 3.8-salted_password branch from RT's git repository, that I then adapted to target 3.6. Ralf -- Fedora Extras Perl SIG http://www.fedoraproject.org/wiki/Extras/SIGs/Perl perl-devel mailing list perl-de...@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/perl-devel
Re: Patches for CVE-2011-0009
On 01/26/2011 12:00 AM, Xavier Bachelot wrote:
Hi,
I've been looking at the issue for both rt 3.6 and 3.8.
I have a rather full featured patch for 3.8 and I took the Debian patch
for 3.6. However, I'm not happy with 3.6, it's lacking the script to fix
all the passwords. I'll try to come up with something better in the next
few days. Here's my WIP for reference.
Regards,
Xavier
Here are the updated patches against master and el5 branches. I only
have an rt 3.6 to test against, so the 3.8 patch is not run time tested,
but I'm confident.
The only missing bit is a paragraph about the password mass-update
script in the UPGRADING file for 3.6.
Regards,
Xavier
From b90eb699c62d66ad0f77df8efd52c99bad2256b2 Mon Sep 17 00:00:00 2001
From: Xavier Bachelot xav...@bachelot.org
Date: Wed, 26 Jan 2011 19:39:58 +0100
Subject: [PATCH] CVE-2011-0009
---
rt3-3.6.10-salted_passwords.patch | 232 +
rt3.spec |8 +-
2 files changed, 239 insertions(+), 1 deletions(-)
create mode 100644 rt3-3.6.10-salted_passwords.patch
diff --git a/rt3-3.6.10-salted_passwords.patch b/rt3-3.6.10-salted_passwords.patch
new file mode 100644
index 000..e5422be
--- /dev/null
+++ b/rt3-3.6.10-salted_passwords.patch
@@ -0,0 +1,232 @@
+diff -Naur rt-3.6.10.orig/configure rt-3.6.10/configure
+--- rt-3.6.10.orig/configure 2009-11-30 19:47:53.0 +0100
rt-3.6.10/configure 2011-01-26 11:23:31.0 +0100
+@@ -2646,6 +2646,8 @@
+
+ ac_config_files=$ac_config_files Makefile etc/RT_Config.pm lib/RT.pm bin/mason_handler.svc bin/webmux.pl
+
++ac_config_files=$ac_config_files etc/upgrade/vulnerable-passwords
++
+ cat confcache \_ACEOF
+ # This file is a shell script that caches the results of configure
+ # tests run on this system so they can be shared between configure
+@@ -3356,6 +3358,7 @@
+ lib/RT.pm) CONFIG_FILES=$CONFIG_FILES lib/RT.pm ;;
+ bin/mason_handler.svc) CONFIG_FILES=$CONFIG_FILES bin/mason_handler.svc ;;
+ bin/webmux.pl) CONFIG_FILES=$CONFIG_FILES bin/webmux.pl ;;
++etc/upgrade/vulnerable-passwords) CONFIG_FILES=$CONFIG_FILES etc/upgrade/vulnerable-passwords ;;
+
+ *) as_fn_error invalid argument: \`$ac_config_target' $LINENO 5;;
+ esac
+@@ -3783,7 +3786,8 @@
+ ;;
+ bin/rt:F) chmod ug+x $ac_file
+ ;;
+-
++etc/upgrade/vulnerable-passwords:F) chmod ug+x $ac_file
++;;
+ esac
+ done # for ac_tag
+
+diff -Naur rt-3.6.10.orig/etc/upgrade/vulnerable-passwords.in rt-3.6.10/etc/upgrade/vulnerable-passwords.in
+--- rt-3.6.10.orig/etc/upgrade/vulnerable-passwords.in 1970-01-01 01:00:00.0 +0100
rt-3.6.10/etc/upgrade/vulnerable-passwords.in 2011-01-26 11:00:45.0 +0100
+@@ -0,0 +1,93 @@
++#!@PERL@
++
++use strict;
++use warnings;
++
++use lib @LOCAL_LIB_PATH@;
++use lib @RT_LIB_PATH@;
++
++use RT;
++RT::LoadConfig;
++RT::Init;
++
++$| = 1;
++
++use Getopt::Long;
++use Digest::SHA;
++my $fix;
++GetOptions(fix! = \$fix);
++
++use RT::Users;
++my $users = RT::Users-new( $RT::SystemUser );
++$users-Limit(
++FIELD = 'Password',
++OPERATOR = 'IS NOT',
++VALUE = 'NULL',
++ENTRYAGGREGATOR = 'AND',
++);
++$users-Limit(
++FIELD = 'Password',
++OPERATOR = '!=',
++VALUE = '*NO-PASSWORD*',
++ENTRYAGGREGATOR = 'AND',
++);
++$users-Limit(
++FIELD = 'Password',
++OPERATOR = 'NOT STARTSWITH',
++VALUE = '!',
++ENTRYAGGREGATOR = 'AND',
++);
++push @{$users-{'restrictions'}{ main.Password }}, AND, {
++field = 'LENGTH(main.Password)',
++op = '',
++value = '40',
++};
++
++my $count = $users-Count;
++if ($count == 0) {
++print No users with unsalted or weak cryptography found.\n;
++exit 0;
++}
++
++if ($fix) {
++print Upgrading $count users...\n;
++while (my $u = $users-Next) {
++my $stored = $u-__Value(Password);
++my $raw;
++if (length $stored == 32) {
++$raw = pack(H*,$stored);
++} elsif (length $stored == 22) {
++$raw = MIME::Base64::decode_base64($stored);
++} elsif (length $stored == 13) {
++printf %20s = Old crypt() format, cannot upgrade\n, $u-Name;
++} else {
++printf %20s = Unknown password format!\n, $u-Name;
++}
++next unless $raw;
++
++my $salt = pack(C4,map{int rand(256)} 1..4);
++my $sha = Digest::SHA::sha256(
++$salt . $raw
++);
++$u-_Set(
++Field = Password,
++Value = MIME::Base64::encode_base64(
++$salt . substr($sha,0,26), ),
++);
++}
++print Done.\n;
++exit 0;
++} else {
++if ($count 20) {
++print $count users found with unsalted or weak-cryptography passwords:\n;
++print Id | Name\n, -x9, +, -x9, \n;
++while (my $u = $users-Next) {
++printf %8d | %s\n, $u-Id, $u-Name;
++}
++} else {
++print $count users found
Re: Patches for CVE-2011-0009
On 01/26/2011 09:16 PM, Xavier Bachelot wrote: On 01/26/2011 12:00 AM, Xavier Bachelot wrote: Hi, I've been looking at the issue for both rt 3.6 and 3.8. I have a rather full featured patch for 3.8 and I took the Debian patch for 3.6. However, I'm not happy with 3.6, it's lacking the script to fix all the passwords. I'll try to come up with something better in the next few days. Here's my WIP for reference. Regards, Xavier Here are the updated patches against master and el5 branches. I only have an rt 3.6 to test against, so the 3.8 patch is not run time tested, but I'm confident. The only missing bit is a paragraph about the password mass-update script in the UPGRADING file for 3.6. Sorry, slightly wrong patches, it was missing the patch to the UPGRADING file. Here is a fixed one for 3.8. I've pushed the 3.6 patch to el5. http://koji.fedoraproject.org/koji/taskinfo?taskID=2744662 https://admin.fedoraproject.org/updates/rt3-3.6.10-2.el5 Ralf, Mark, I let you give a test at 3.8 on Rawhide/F14/F13 and EL6, respectively. X. From c9ff93446d9c1c5ea8e864c6d0be9526bc181dab Mon Sep 17 00:00:00 2001 From: Xavier Bachelot xav...@bachelot.org Date: Tue, 25 Jan 2011 23:25:52 +0100 Subject: [PATCH] CVE-2011-0009 --- rt3-3.8.8-salted_passwords.patch | 260 ++ rt3.spec |7 +- 2 files changed, 266 insertions(+), 1 deletions(-) create mode 100644 rt3-3.8.8-salted_passwords.patch diff --git a/rt3-3.8.8-salted_passwords.patch b/rt3-3.8.8-salted_passwords.patch new file mode 100644 index 000..3cf5780 --- /dev/null +++ b/rt3-3.8.8-salted_passwords.patch @@ -0,0 +1,260 @@ +diff --git a/UPGRADING b/UPGRADING +--- a/UPGRADING b/UPGRADING +@@ -18,6 +18,18 @@ + well. + + *** ++UPGRADING FROM 3.8.8 and earlier - Changes: ++ ++Previous versions of RT used a password hashing scheme which was too ++easy to reverse, which could allow attackers with read access to the ++RT database to possibly compromise users' passwords. Even if RT does ++no password authentication itself, it may still store these weak ++password hashes -- using ExternalAuth does not guarantee that you are ++not vulnerable! To upgrade stored passwords to a stronger hash, run: ++ ++perl etc/upgrade/vulnerable-passwords ++ ++ + UPGRADING FROM 3.8.7 and earlier - Changes: + + RT's ChartFont option has been changed from a string to a hash which +diff --git a/configure b/configure +--- a/configure b/configure +@@ -3886,7 +3886,7 @@ + fi + + +-ac_config_files=$ac_config_files etc/upgrade/3.8-branded-queues-extension etc/upgrade/3.8-ical-extension etc/upgrade/split-out-cf-categories sbin/rt-attributes-viewer sbin/rt-dump-database sbin/rt-setup-database sbin/rt-test-dependencies sbin/rt-email-digest sbin/rt-email-dashboards sbin/rt-clean-sessions sbin/rt-shredder sbin/rt-validator sbin/rt-email-group-admin sbin/rt-server bin/fastcgi_server bin/mason_handler.fcgi bin/mason_handler.scgi bin/standalone_httpd bin/rt-crontool bin/rt-mailgate bin/rt ++ac_config_files=$ac_config_files etc/upgrade/3.8-branded-queues-extension etc/upgrade/3.8-ical-extension etc/upgrade/split-out-cf-categories etc/upgrade/vulnerable-passwords sbin/rt-attributes-viewer sbin/rt-dump-database sbin/rt-setup-database sbin/rt-test-dependencies sbin/rt-email-digest sbin/rt-email-dashboards sbin/rt-clean-sessions sbin/rt-shredder sbin/rt-validator sbin/rt-email-group-admin sbin/rt-server bin/fastcgi_server bin/mason_handler.fcgi bin/mason_handler.scgi bin/standalone_httpd bin/rt-crontool bin/rt-mailgate bin/rt + + + ac_config_files=$ac_config_files Makefile etc/RT_Config.pm lib/RT.pm bin/mason_handler.svc bin/webmux.pl t/data/configs/apache2.2+mod_perl.conf t/data/configs/apache2.2+fastcgi.conf +@@ -4594,6 +4594,7 @@ + etc/upgrade/3.8-branded-queues-extension) CONFIG_FILES=$CONFIG_FILES etc/upgrade/3.8-branded-queues-extension ;; + etc/upgrade/3.8-ical-extension) CONFIG_FILES=$CONFIG_FILES etc/upgrade/3.8-ical-extension ;; + etc/upgrade/split-out-cf-categories) CONFIG_FILES=$CONFIG_FILES etc/upgrade/split-out-cf-categories ;; ++etc/upgrade/vulnerable-passwords) CONFIG_FILES=$CONFIG_FILES etc/upgrade/vulnerable-passwords ;; + sbin/rt-attributes-viewer) CONFIG_FILES=$CONFIG_FILES sbin/rt-attributes-viewer ;; + sbin/rt-dump-database) CONFIG_FILES=$CONFIG_FILES sbin/rt-dump-database ;; + sbin/rt-setup-database) CONFIG_FILES=$CONFIG_FILES sbin/rt-setup-database ;; +@@ -5034,6 +5035,8 @@ + ;; + etc/upgrade/split-out-cf-categories:F) chmod ug+x $ac_file + ;; ++etc/upgrade/vulnerable-passwords:F) chmod ug+x $ac_file ++;; + sbin/rt-attributes-viewer:F) chmod ug+x $ac_file + ;; + sbin/rt-dump-database:F) chmod ug+x $ac_file +diff --git a/etc/upgrade/vulnerable-passwords.in b/etc/upgrade/vulnerable-passwords.in +new file mode 100755 +index 000..c28d2b8 +--- /dev/null
Patches for CVE-2011-0009
Hi,
I've been looking at the issue for both rt 3.6 and 3.8.
I have a rather full featured patch for 3.8 and I took the Debian patch
for 3.6. However, I'm not happy with 3.6, it's lacking the script to fix
all the passwords. I'll try to come up with something better in the next
few days. Here's my WIP for reference.
Regards,
Xavier
diff --git a/lib/RT/User_Overlay.pm b/lib/RT/User_Overlay.pm
index 1ef8c4b..459968e 100755
--- a/lib/RT/User_Overlay.pm
+++ b/lib/RT/User_Overlay.pm
@@ -82,6 +82,7 @@ use RT::Principals;
use RT::ACE;
use RT::Interface::Email;
use Encode;
+use Digest::SHA;
# {{{ sub _Accessible
@@ -1061,12 +1062,19 @@ returns an MD5 hash of the password passed in, in hexadecimal encoding.
sub _GeneratePassword {
my $self = shift;
-my $password = shift;
-
-my $md5 = Digest::MD5-new();
-$md5-add(encode_utf8($password));
-return ($md5-hexdigest);
-
+my ($password, $salt) = @_;
+
+# Generate a random 4-byte salt
+$salt ||= pack(C4,map{int rand(256)} 1..4);
+
+# Encode the salt, and a truncated SHA256 of the MD5 of the
+# password The additional, un-necessary level of MD5 allows for
+# transparent upgrading to this scheme, from the previous unsalted
+# MD5 one.
+return MIME::Base64::encode_base64(
+$salt
+ . substr(Digest::SHA::sha256($salt . Digest::MD5::md5($password)),0,26)
+);
}
=head2 _GeneratePasswordBase64 PASSWORD
@@ -1119,9 +1127,7 @@ sub IsPassword {
my $self = shift;
my $value = shift;
-#TODO there isn't any apparent way to legitimately ACL this
-
-# RT does not allow null passwords
+# RT does not allow null passwords
if ( ( !defined($value) ) or ( $value eq '' ) ) {
return (undef);
}
@@ -1136,23 +1142,32 @@ sub IsPassword {
return(undef);
}
-# generate an md5 password
-if ($self-_GeneratePassword($value) eq $self-__Value('Password')) {
-return(1);
-}
-
-# if it's a historical password we say ok.
-if ($self-__Value('Password') eq crypt($value, $self-__Value('Password'))
-or $self-_GeneratePasswordBase64($value) eq $self-__Value('Password'))
-{
-# ...but upgrade the legacy password inplace.
-$self-SUPER::SetPassword( $self-_GeneratePassword($value) );
-return(1);
+my $stored = $self-__Value('Password');
+if (length $stored == 40) {
+# The truncated SHA256(salt,MD5(passwd)) form from 2010/12 is 40 characters long
+my $hash = MIME::Base64::decode_base64($stored);
+# The first 4 bytes are the salt, the rest is substr(SHA256,0,26)
+my $salt = substr($hash, 0, 4, );
+return substr(Digest::SHA::sha256($salt . Digest::MD5::md5($value)), 0, 26) eq $hash;
+} elsif (length $stored == 32) {
+# Hex nonsalted-md5
+return 0 unless Digest::MD5::md5_hex(Encode::encode_utf8($value)) eq $stored;
+} elsif (length $stored == 22) {
+# Base64 nonsalted-md5
+return 0 unless Digest::MD5::md5_base64(Encode::encode_utf8($value)) eq $stored;
+} elsif (length $stored == 13) {
+# crypt() output
+return 0 unless crypt(Encode::encode_utf8($value), $stored) eq $stored;
+} else {
+$RT::Logger-warn(Unknown password form);
+return 0;
}
-# no password check has succeeded. get out
-
-return (undef);
+# We got here by validating successfully, but with a legacy
+# password form. Update to the most recent form.
+my $obj = $self-isa(RT::CurrentUser) ? $self-UserObj : $self;
+$obj-_Set(Field = 'Password', Value = $self-_GeneratePassword($value) );
+return 1;
}
# }}}
From f0aacc11df5a13db3f55c643783753ed64d8bea0 Mon Sep 17 00:00:00 2001
From: Xavier Bachelot xav...@bachelot.org
Date: Tue, 25 Jan 2011 23:25:52 +0100
Subject: [PATCH] Patch for CVE-2011-0009
---
rt3-3.8.8-salted_passwords.patch | 269 ++
1 files changed, 269 insertions(+), 0 deletions(-)
create mode 100644 rt3-3.8.8-salted_passwords.patch
diff --git a/rt3-3.8.8-salted_passwords.patch b/rt3-3.8.8-salted_passwords.patch
new file mode 100644
index 000..76d27f2
--- /dev/null
+++ b/rt3-3.8.8-salted_passwords.patch
@@ -0,0 +1,269 @@
+--- rt-3.8.8/UPGRADING
rt-3.8.8/UPGRADING
+@@ -18,6 +18,18 @@
+ well.
+
+ ***
++UPGRADING FROM 3.8.8 and earlier - Changes:
++
++Previous versions of RT used a password hashing scheme which was too
++easy to reverse, which could allow attackers with read access to the
++RT database to possibly compromise users' passwords. Even if RT does
++no password authentication itself, it may still store these weak
++password hashes -- using ExternalAuth does not guarantee that you are
++not vulnerable! To upgrade stored passwords to a stronger hash, run:
++
++perl etc/upgrade/vulnerable-passwords
++
++
+ UPGRADING FROM 3.8.7 and earlier - Changes:
+
+ RT's ChartFont option has been changed from a string to a hash which
