Re: Fixes for RT 3.x issue CVE-2011-0009 in testing: preapproval requested
On Thu, Jan 20, 2011 at 09:41:34AM +0000, Dominic Hargreaves wrote:
> On Thu, Jan 20, 2011 at 08:39:12AM +0000, Dominic Hargreaves wrote:
>
> > I attach the full interdiff for review. Please could you let me
> > know whether it would be okay to go ahead with this upload to unstable,
> > or whether a targetted upload covering just the security fix will be
> > required,
>
> No, really, attached now :)
I've just made a few minor changes in this version at the suggestion of
Niko Tyni. New debdiff attached.
I'll upload this version to unstable later today so that people have
access to the fix if they need it, unless someone says not to.
Cheers,
Dominic.
--
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)
diff -u request-tracker3.8-3.8.8/debian/rules request-tracker3.8-3.8.8/debian/rules
--- request-tracker3.8-3.8.8/debian/rules
+++ request-tracker3.8-3.8.8/debian/rules
@@ -133,6 +133,8 @@
install -m 644 debian/lintian-overrides $(RT3_PKG)/usr/share/lintian/overrides/$(RT3)
+ install -m 755 debian/scripts/vulnerable-passwords $(RT3_PKG)/usr/sbin/rt-vulnerable-passwords-3.8
+
# dh_link already made the pgsql and sqlite links
install -m 755 debian/scripts/dbconfig-install $(RT3_PKG)/usr/share/dbconfig-common/scripts/request-tracker3.8/install/mysql
install -m 755 debian/scripts/dbconfig-upgrade-3.8.2 $(RT3_PKG)/usr/share/dbconfig-common/scripts/request-tracker3.8/upgrade/mysql/3.8.2
diff -u request-tracker3.8-3.8.8/debian/postinst request-tracker3.8-3.8.8/debian/postinst
--- request-tracker3.8-3.8.8/debian/postinst
+++ request-tracker3.8-3.8.8/debian/postinst
@@ -99,7 +99,7 @@
then
ucfr request-tracker3.8 /etc/request-tracker3.8/RT_SiteConfig.d/50-debconf
ucfr request-tracker3.8 /etc/request-tracker3.8/RT_SiteConfig.pm
- ucfr request-tracker3.8 /etc/cron.d/request-tracker3.8
+ ucfr request-tracker3.8 /etc/cron.d/request-tracker38
# this should probably be registered by dbconfig-generate-include,
# but ucfr is idempotent so it doesn't hurt here anyway.
ucfr request-tracker3.8 /etc/request-tracker3.8/RT_SiteConfig.d/51-dbconfig-common
@@ -132,10 +132,30 @@
EOF
fi
mkdir -p /etc/cron.d
- ucf --debconf-ok $tfile /etc/cron.d/request-tracker3.8
+ if [ -f /etc/cron.d/request-tracker3.8 ]; then
+ mv /etc/cron.d/request-tracker3.8 /etc/cron.d/request-tracker38
+ ucf --purge /etc/cron.d/request-tracker3.8
+ ucfr --purge request-tracker3.8 /etc/cron.d/request-tracker3.8
+ fi
+ ucf --debconf-ok $tfile /etc/cron.d/request-tracker38
rm $tfile
}
+fix_vulnerable_passwords() {
+ if [ "$1" = "configure" ] && [ -n "$2" ] && \
+ dpkg --compare-versions "$2" lt 3.8.8-7
+ then
+ if su -c "/usr/sbin/rt-vulnerable-passwords-3.8 --fix" www-data; then
+ echo "rt-vulnerable-passwords-3.8 invoked successfully on upgrade"
+ else
+ echo "rt-vulnerable-passwords-3.8 exited with an error but the"
+ echo "package post-installation will continue. We recommend that"
+ echo "you check the above error and take corrective action to"
+ echo "ensure the privacy of your user's passwords."
+ fi
+ fi
+}
+
# The actual work starts here
. /usr/share/debconf/confmodule
@@ -155,6 +175,7 @@
maybe_handle_permissions
setup_cronjobs
ucf_register
+fix_vulnerable_passwords $@
case "$1" in
configure)
diff -u request-tracker3.8-3.8.8/debian/NEWS request-tracker3.8-3.8.8/debian/NEWS
--- request-tracker3.8-3.8.8/debian/NEWS
+++ request-tracker3.8-3.8.8/debian/NEWS
@@ -1,3 +1,11 @@
+request-tracker3.8 (3.8.8-7) unstable; urgency=high
+
+ * Upgrades to this package version will automatically upgrade the
+ passwords in your database to a salted hash, to guard against
+ brute-forcing attacks. This is a fix for CVE-2011-0009.
+
+ -- Dominic Hargreaves <dom@earth.li> Thu, 20 Jan 2011 10:06:56 +0000
+
request-tracker3.8 (3.8.8-1) unstable; urgency=low
* A new method of running RT, via an external fastcgi_server, is now
diff -u request-tracker3.8-3.8.8/debian/changelog request-tracker3.8-3.8.8/debian/changelog
--- request-tracker3.8-3.8.8/debian/changelog
+++ request-tracker3.8-3.8.8/debian/changelog
@@ -1,3 +1,19 @@
+request-tracker3.8 (3.8.8-7~test.5) unstable; urgency=high
+
+ * Correct name of file in cron.d to one which will be run by cron
+ (Closes: #607209)
+ * Apply patch from upstream reducing the severity of the
+ RTAddressRegexp warning message to "debug", to avoid the cron jobs
+ generating noise
+ * Remove completely misleading documentation from NOTES.Debian
+ relating to migrating between SQLite and other databases
+ (Closes: #608481)
+ * Correct name of libapache2-mod-fcgid in debian/conf/apache2-fcgid.conf
+ * Security fix: support salted passwords in database and upgrade
+ unsalted passwords (CVE-2011-0009)
+
+ -- Dominic Hargreaves <dom@earth.li> Thu, 20 Jan 2011 10:25:38 +0000
+
request-tracker3.8 (3.8.8-6) unstable; urgency=low
* Make sure /etc/cron.d exists in postinst before installing cronjob,
diff -u request-tracker3.8-3.8.8/debian/NOTES.Debian request-tracker3.8-3.8.8/debian/NOTES.Debian
--- request-tracker3.8-3.8.8/debian/NOTES.Debian
+++ request-tracker3.8-3.8.8/debian/NOTES.Debian
@@ -46,36 +46,6 @@
We strongly recommend that you use MySQL or PostgreSQL for production
use, by installing rt3.8-db-mysql or rt3.8-db-postgresql.
-DATABASE CONVERSION
--------------------
-
-An existing SQLite installation can be converted to MySQL or PostgreSQL
-by either
-
- * installing rt3.8-db-mysql or rt3.8-db-postgresql and then running
- 'dpkg-reconfigure request-tracker3.8' and picking the new choices, or
- * editing /etc/request-tracker3.8/RT_SiteConfig* and setting up the
- new database with 'rt-setup-database --action init' manually.
-
-In either case, if you want to keep existing database content, you
-should run
-
- rt-dump-database > dumpfile
-
-first. After the new database is configured, just run
-
- rt-setup-database --action insert --datafile dumpfile
-
-Note that you will need to reset the RT root password, it is not included
-in the dump.
-
-The reason the root password is not included is that rt-dump-database
-does not dump predefined system objects by default. If you have modified
-some of these, you may want to use 'rt-setup-database 0' to get the system
-objects too. However, be aware that 'rt-setup-database --action insert' on
-an already initialized database will then duplicate the objects.
-Suggestions on the best way to handle this situation are welcome.
-
DATABASE CREATION AND SCHEMA INSTALLATION
-----------------------------------------
diff -u request-tracker3.8-3.8.8/debian/control request-tracker3.8-3.8.8/debian/control
--- request-tracker3.8-3.8.8/debian/control
+++ request-tracker3.8-3.8.8/debian/control
@@ -40,7 +40,7 @@
libjs-scriptaculous, libjs-prototype, libipc-run-safehandles-perl,
perl (>= 5.10.0) | libencode-perl (>= 2.21),
perl (>= 5.12.0) | libcgi-pm-perl (>= 3.38), libcgi-fast-perl,
- libfcgi-procmanager-perl,
+ libfcgi-procmanager-perl, perl (>= 5.10.0) | libdigest-sha-perl,
${misc:Depends}
Recommends: speedy-cgi-perl, libdatetime-locale-perl, libdatetime-perl, cron-daemon
Suggests: rt3.8-rtfm
diff -u request-tracker3.8-3.8.8/debian/conf/apache2-fcgid.conf request-tracker3.8-3.8.8/debian/conf/apache2-fcgid.conf
--- request-tracker3.8-3.8.8/debian/conf/apache2-fcgid.conf
+++ request-tracker3.8-3.8.8/debian/conf/apache2-fcgid.conf
@@ -1,5 +1,5 @@
# To use RT together with mod_fcgid, available in the
-# libapache-mod-fcgid package, include this file with:
+# libapache2-mod-fcgid package, include this file with:
#
# Include /etc/request-tracker3.8/apache2-fcgid.conf
#
diff -u request-tracker3.8-3.8.8/debian/patches/00list request-tracker3.8-3.8.8/debian/patches/00list
--- request-tracker3.8-3.8.8/debian/patches/00list
+++ request-tracker3.8-3.8.8/debian/patches/00list
@@ -13,0 +14,2 @@
+60_rtaddressregexp_not_error
+74_salted_passwords
only in patch2:
unchanged:
--- request-tracker3.8-3.8.8.orig/debian/patches/60_rtaddressregexp_not_error.dpatch
+++ request-tracker3.8-3.8.8/debian/patches/60_rtaddressregexp_not_error.dpatch
@@ -0,0 +1,25 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 60_rtaddressregexp_not_error.dpatch by Jesse Vincent <jesse@bestpractical.com>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: commit 4409d6eab940357c04cb4e4c0b57ee82f1b0560d
+## DP: Author: Jesse Vincent <jesse@bestpractical.com>
+## DP: Date: Wed Jul 14 20:21:40 2010 +0200
+## DP:
+## DP: Drop the RTAddressRegexp down to a debug message so it stops complaining
+## DP: on any script being run on every RT instance on the planet.
+
+@DPATCH@
+diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
+index 051117a..7e7a659 100644
+--- a/lib/RT/Config.pm
++++ b/lib/RT/Config.pm
+@@ -354,7 +354,7 @@ our %META = (
+ my $value = $self->Get('RTAddressRegexp');
+ return if $value;
+
+- $RT::Logger->error(
++ $RT::Logger->debug(
+ 'The RTAddressRegexp option is not set in the config.'
+ .' Not setting this option results in additional SQL queries to'
+ .' check whether each address belongs to RT or not.'
only in patch2:
unchanged:
--- request-tracker3.8-3.8.8.orig/debian/patches/74_salted_passwords.dpatch
+++ request-tracker3.8-3.8.8/debian/patches/74_salted_passwords.dpatch
@@ -0,0 +1,98 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 74_salted_passwords.dpatch
+##
+## DP: Support and upgrade to salted passwords (security fix)
+diff -urN rt-3.8.8.orig//lib/RT/User_Overlay.pm rt-3.8.8/lib/RT/User_Overlay.pm
+--- rt-3.8.8.orig//lib/RT/User_Overlay.pm 2010-05-05 21:09:21.000000000 +0100
++++ rt-3.8.8/lib/RT/User_Overlay.pm 2011-01-16 18:33:45.000000000 +0000
+@@ -74,6 +74,7 @@
+ use RT::ACE;
+ use RT::Interface::Email;
+ use Encode;
++use Digest::SHA;
+
+ sub _OverlayAccessible {
+ {
+@@ -996,12 +997,19 @@
+
+ sub _GeneratePassword {
+ my $self = shift;
+- my $password = shift;
++ my ($password, $salt) = @_;
+
+- my $md5 = Digest::MD5->new();
+- $md5->add(encode_utf8($password));
+- return ($md5->hexdigest);
++ # 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),
++ "" # No newline
++ );
+ }
+
+ =head3 _GeneratePasswordBase64 PASSWORD
+@@ -1047,9 +1055,7 @@
+ 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);
+ }
+@@ -1064,23 +1070,32 @@
+ 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(encode_utf8($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;
+ }
+
+ sub CurrentUserRequireToSetPassword {
only in patch2:
unchanged:
--- request-tracker3.8-3.8.8.orig/debian/scripts/vulnerable-passwords
+++ request-tracker3.8-3.8.8/debian/scripts/vulnerable-passwords
@@ -0,0 +1,89 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+BEGIN {
+ use lib qw(/usr/share/request-tracker3.8/lib);
+ use RT;
+ RT::LoadConfig;
+ RT::Init;
+}
+
+$| = 1;
+
+use Getopt::Long;
+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 LENGTH(main.Password) < 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 with unsalted or weak-cryptography passwords\n";
+ }
+
+ print "\n", "Run again with --fix to upgrade.\n";
+ exit 1;
+}
Reply to: