From 8e16e006afaf77b51329d443498a11500e1b429f Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Thu, 30 Jul 2015 14:57:23 -0600 Subject: [PATCH 01/11] Freeside sessionkey Should Not Be Stored in Plaintext Add configurable option for storing the session key (in the access_user_session table) as a salted SHA256 hash) based on setting in the configuration. If the 'hashsalt' configuration setting is a string of positive length, use the salt string as input to the hash stored as the sessionkey. After setting the hashsalt, you will need to restart Apache, freeside-queued, etc. You will also need to close and reopen your browser and flush your cookies. --- FS/FS/Conf.pm | 7 ++ FS/FS/Record.pm | 41 +++++++++- FS/FS/access_user_session.pm | 2 + FS/t/Record.t | 140 +++++++++++++++++++++++++++++++++-- 4 files changed, 183 insertions(+), 7 deletions(-) diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index c936082665..27520e16ed 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -831,6 +831,13 @@ my $validate_email = sub { $_[0] =~ 'type' => 'checkbox', }, + { + 'key' => 'hashsalt', + 'section' => 'billing', + 'description' => 'Hash salt string (enables hashing of session keys/cookies)', + 'type' => 'text', + }, + { 'key' => 'encryption', 'section' => 'billing', diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm index 70d4f672ec..9a15e20e7b 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -22,6 +22,7 @@ use FS::Schema qw(dbdef); use FS::SearchCache; use FS::Msgcat qw(gettext); #use FS::Conf; #dependency loop bs, in install_callback below instead +use Digest::SHA qw(sha256_hex); use FS::part_virtual_field; @@ -59,6 +60,7 @@ our $conf_encryption = ''; our $conf_encryptionmodule = ''; our $conf_encryptionpublickey = ''; our $conf_encryptionprivatekey = ''; +our $conf_hashsalt = ''; FS::UID->install_callback( sub { eval "use FS::Conf;"; @@ -430,6 +432,18 @@ sub qsearch { ) { my $value = $record->{$field}; + # If searching for the user session, search for SHA256'ed + # (Also works for other ::hashed_fields ...) + if ( $conf_hashsalt + && defined(eval '@FS::'. $stable . '::hashed_fields') + && scalar( eval '@FS::'. $stable . '::hashed_fields') + ) { + foreach my $hashed_field_name (eval '@FS::'. $stable . '::hashed_fields') { + next if $hashed_field_name ne $field; # continue if this isn't a hashed field + $value = sha256_hex($value.$conf_hashsalt); + } + } + my $op = (ref($value) && $value->{op}) ? $value->{op} : '='; $value = $value->{'value'} if ref($value); my $type = dbdef->table($table)->column($field)->type; @@ -1299,9 +1313,9 @@ sub insert { my $table = $self->table; # Encrypt before the database - if ( defined(eval '@FS::'. $table . '::encrypted_fields') + if ( $conf_encryption + && defined(eval '@FS::'. $table . '::encrypted_fields') && scalar( eval '@FS::'. $table . '::encrypted_fields') - && $conf_encryption ) { foreach my $field (eval '@FS::'. $table . '::encrypted_fields') { next if $field eq 'payinfo' @@ -1314,6 +1328,17 @@ sub insert { } } + # SHA256 before the database + if ( $conf_hashsalt + && defined(eval '@FS::'. $table . '::hashed_fields') + && scalar( eval '@FS::'. $table . '::hashed_fields') + ) { + foreach my $field (eval '@FS::'. $table . '::hashed_fields') { + $saved->{$field} = $self->getfield($field); + $self->setfield($field, sha256_hex($self->getfield($field).$conf_hashsalt)); + } + } + #false laziness w/delete my @real_fields = grep { defined($self->getfield($_)) && $self->getfield($_) ne "" } @@ -1557,6 +1582,18 @@ sub replace { } } + # SHA256 hash for replace + $saved = {}; + if ( $conf_hashsalt + && defined(eval '@FS::'. $new->table . '::hashed_fields') + && scalar( eval '@FS::'. $new->table . '::hashed_fields') + ) { + foreach my $field (eval '@FS::'. $new->table . '::hashed_fields') { + $saved->{$field} = $new->getfield($field); + $new->setfield($field, sha256_hex($new->getfield($field).$conf_hashsalt)); + } + } + #my @diff = grep $new->getfield($_) ne $old->getfield($_), $old->fields; my %diff = map { ($new->getfield($_) ne $old->getfield($_)) ? ($_, $new->getfield($_)) : () } $old->fields; diff --git a/FS/FS/access_user_session.pm b/FS/FS/access_user_session.pm index 7845d92aa5..d56cd1b808 100644 --- a/FS/FS/access_user_session.pm +++ b/FS/FS/access_user_session.pm @@ -3,6 +3,8 @@ use base qw( FS::Record ); use strict; +our @hashed_fields = ('sessionkey'); + =head1 NAME FS::access_user_session - Object methods for access_user_session records diff --git a/FS/t/Record.t b/FS/t/Record.t index 00de1eda30..0a39a0a13d 100644 --- a/FS/t/Record.t +++ b/FS/t/Record.t @@ -1,5 +1,135 @@ -BEGIN { $| = 1; print "1..1\n" } -END {print "not ok 1\n" unless $loaded;} -use FS::Record; -$loaded=1; -print "ok 1\n"; +use strict; +use warnings; +use FS::Conf; +use Data::Dumper qw( Dumper ); +use FS::UID qw( adminsuidsetup ); +use Digest::SHA qw(sha256_hex); +use Test::More tests => 26; + +# Stock freeside does equivalent of the following +BEGIN { use_ok('FS::Record') } + +# If adminuser passed in, assume this is being run in foreground +my $freeside_uid = getpwnam('freeside'); +my $adminuser = shift || ''; +if (!length $adminuser) { + note <<"USAGE"; +Syntax for longer user-initiated tests: + prove $0 :: [admin-user_name] + OR + perl $0 [admin-user_name] + +USAGE +} + +SKIP: { + skip 'test(s) if not run as freeside user or if DB adminuser unspecified', 25 + if ($< != $freeside_uid) || !length($adminuser); + + require_ok('FS::access_user_session'); + + my $dbh = adminsuidsetup $adminuser; + + my $conf = FS::Conf->new; + + # 1. Without salt + local $FS::Record::conf_hashsalt = ''; + my $cookie = 'this is a test '. time(); + my $record1 = new FS::access_user_session { + usernum => 3, + sessionkey => $cookie, + start_date => 0, + }; + + my $error = $record1->insert; + ok(!$error, 'Insert unhashed record in access_user_session') or diag explain $error,$record1; + my @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + ok(scalar @records == 1,'qsearch finds single matching record just inserted'); + cmp_ok($records[0]->sessionkey,'eq',$cookie,'Unhashed sessionkey matches'); + + my $record2 = new FS::access_user_session { # Record 2 will replace record 1 + usernum => 3, + sessionkey => $cookie, + start_date => 2, + sessionnum => $record1->{'Hash'}->{'sessionnum'}, + }; + $error = $record2->replace($record1); + ok(!$error, 'Replace unhashed record in access_user_session with different start_date') or diag explain $error,$record2,$record1; + @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + ok(scalar @records == 1,'qsearch finds single matching record just replaced'); + cmp_ok($records[0]->sessionkey,'eq',$cookie,'Unhashed sessionkey matches after replacing'); + cmp_ok($records[0]->start_date,'==',2,'Replaced record has new start_date'); + + # 2. First salt + local $FS::Record::conf_hashsalt = 'test it'; + my $saltrecord1 = new FS::access_user_session { + usernum => 3, + sessionkey => $cookie, + start_date => 0, + }; + + $error = $saltrecord1->insert; + ok(!$error, 'Insert record in access_user_session with first salt') or diag explain $error,$saltrecord1; + my $shacookie1 = sha256_hex($cookie.$FS::Record::conf_hashsalt); + @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + ok(scalar @records == 1,'qsearch finds single matching record just inserted with first salt'); + cmp_ok($records[0]->sessionkey,'eq',$shacookie1,'First salted sessionkey matches'); + + my $saltrecord2 = new FS::access_user_session { + usernum => 3, + sessionkey => $cookie, + start_date => 2, # Increment start date for replace + sessionnum => $saltrecord1->{'Hash'}->{'sessionnum'}, + }; + $error = $saltrecord2->replace($saltrecord1); + ok(!$error, 'Replace hashed record (salted with 1st salt) in access_user_session with different start_date') or diag explain $error,$saltrecord2,$saltrecord1; + @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + ok(scalar @records == 1,'qsearch finds single matching record (salted with 1st salt) just replaced'); + cmp_ok($records[0]->sessionkey,'eq',$shacookie1,'First Hashed sessionkey matches'); + cmp_ok($records[0]->start_date,'==',2,'Replaced record has new start date'); + + # 3. Salt no. 2 + local $FS::Record::conf_hashsalt = 'test it2'; + my $salt2record1 = new FS::access_user_session { + usernum => 3, + sessionkey => $cookie, + start_date => 0, + }; + + $error = $salt2record1->insert; + ok(!$error, 'Insert record in access_user_session with 2nd salt') or diag explain $error,$salt2record1; + my $shacookie2 = sha256_hex($cookie.$FS::Record::conf_hashsalt); + @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + ok(scalar @records == 1,'qsearch finds single matching record (salted with 2nd salt) just inserted with second salt'); + cmp_ok($records[0]->sessionkey,'eq',$shacookie2,'Second salted sessionkey matches (salted with 2nd salt) matches'); + + my $salt2record2 = new FS::access_user_session { + usernum => 3, + sessionkey => $cookie, + start_date => 2, # Increment start date for replace + sessionnum => $salt2record1->{'Hash'}->{'sessionnum'}, + }; + $error = $salt2record2->replace($salt2record1); + ok(!$error, 'Replace 2nd hashed record (salted with 2nd salt) access_user_session with different start_date') or diag explain $error,$salt2record2,$salt2record1; + @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + ok(scalar @records == 1,'qsearch finds single matching record (salted with 2nd salt) just replaced'); + cmp_ok($records[0]->sessionkey,'eq',$shacookie2,'2nd hashed cookie matches'); + cmp_ok($records[0]->start_date,'==',2,'Replaced record has new start date'); + + # 4. No salt ... again + # Update the unsalted (unhashed) record in step 1. + local $FS::Record::conf_hashsalt = ''; + my $record3 = new FS::access_user_session { + usernum => 3, + sessionkey => $cookie, + start_date => 4, # Increment start date for replace + sessionnum => $record1->{'Hash'}->{'sessionnum'}, + }; + $error = $record3->replace($record2); + ok(!$error, 'Replace unhashed record (2nd time around) in access_user_session with different start_date') or diag explain $error,$record3,$record2; + @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); + cmp_ok($records[0]->sessionkey,'eq',$cookie,'2nd time around, able to replace original plain cookie/sessionnum'); + cmp_ok($records[0]->start_date,'==',4,'2nd time around, replaced record has even newer start_date'); + +} + From 72153e62b398f0e3f433f62d39a569928350b9e1 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Thu, 6 Aug 2015 10:19:50 -0600 Subject: [PATCH 02/11] Add overlooked line that mentions the $conf_hashsalt in install_callback --- FS/FS/Record.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm index 9a15e20e7b..c7160393bf 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -74,6 +74,7 @@ FS::UID->install_callback( sub { my $nw_coords = $conf->exists('geocode-require_nw_coordinates'); $lat_lower = $nw_coords ? 1 : -90; $lon_upper = $nw_coords ? -1 : 180; + $conf_hashsalt = $conf->config('hashsalt'); $File::CounterFile::DEFAULT_DIR = $conf->base_dir . "/counters.". datasrc; From fd8e7f1a0f9f82e469a0451608702350a9267c62 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Thu, 6 Aug 2015 10:38:51 -0600 Subject: [PATCH 03/11] Add overlooked $conf_hashsalt in callback --- FS/FS/Record.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm index 9a15e20e7b..c7160393bf 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -74,6 +74,7 @@ FS::UID->install_callback( sub { my $nw_coords = $conf->exists('geocode-require_nw_coordinates'); $lat_lower = $nw_coords ? 1 : -90; $lon_upper = $nw_coords ? -1 : 180; + $conf_hashsalt = $conf->config('hashsalt'); $File::CounterFile::DEFAULT_DIR = $conf->base_dir . "/counters.". datasrc; From 78a3ea0fdae6384aba408b64d84ad0fe00bd21ae Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Wed, 12 Aug 2015 16:51:57 -0600 Subject: [PATCH 04/11] Remove salted hash from FS::Record's update method. (The field passed into update was already hashed.) --- FS/FS/Record.pm | 12 ---------- FS/t/Record.t | 59 ++----------------------------------------------- 2 files changed, 2 insertions(+), 69 deletions(-) diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm index c7160393bf..84abc710de 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -1583,18 +1583,6 @@ sub replace { } } - # SHA256 hash for replace - $saved = {}; - if ( $conf_hashsalt - && defined(eval '@FS::'. $new->table . '::hashed_fields') - && scalar( eval '@FS::'. $new->table . '::hashed_fields') - ) { - foreach my $field (eval '@FS::'. $new->table . '::hashed_fields') { - $saved->{$field} = $new->getfield($field); - $new->setfield($field, sha256_hex($new->getfield($field).$conf_hashsalt)); - } - } - #my @diff = grep $new->getfield($_) ne $old->getfield($_), $old->fields; my %diff = map { ($new->getfield($_) ne $old->getfield($_)) ? ($_, $new->getfield($_)) : () } $old->fields; diff --git a/FS/t/Record.t b/FS/t/Record.t index 0a39a0a13d..44ecc21933 100644 --- a/FS/t/Record.t +++ b/FS/t/Record.t @@ -4,7 +4,7 @@ use FS::Conf; use Data::Dumper qw( Dumper ); use FS::UID qw( adminsuidsetup ); use Digest::SHA qw(sha256_hex); -use Test::More tests => 26; +use Test::More tests => 11; # Stock freeside does equivalent of the following BEGIN { use_ok('FS::Record') } @@ -23,7 +23,7 @@ USAGE } SKIP: { - skip 'test(s) if not run as freeside user or if DB adminuser unspecified', 25 + skip 'test(s) if not run as freeside user or if DB adminuser unspecified', 10 if ($< != $freeside_uid) || !length($adminuser); require_ok('FS::access_user_session'); @@ -47,19 +47,6 @@ SKIP: { ok(scalar @records == 1,'qsearch finds single matching record just inserted'); cmp_ok($records[0]->sessionkey,'eq',$cookie,'Unhashed sessionkey matches'); - my $record2 = new FS::access_user_session { # Record 2 will replace record 1 - usernum => 3, - sessionkey => $cookie, - start_date => 2, - sessionnum => $record1->{'Hash'}->{'sessionnum'}, - }; - $error = $record2->replace($record1); - ok(!$error, 'Replace unhashed record in access_user_session with different start_date') or diag explain $error,$record2,$record1; - @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - ok(scalar @records == 1,'qsearch finds single matching record just replaced'); - cmp_ok($records[0]->sessionkey,'eq',$cookie,'Unhashed sessionkey matches after replacing'); - cmp_ok($records[0]->start_date,'==',2,'Replaced record has new start_date'); - # 2. First salt local $FS::Record::conf_hashsalt = 'test it'; my $saltrecord1 = new FS::access_user_session { @@ -75,19 +62,6 @@ SKIP: { ok(scalar @records == 1,'qsearch finds single matching record just inserted with first salt'); cmp_ok($records[0]->sessionkey,'eq',$shacookie1,'First salted sessionkey matches'); - my $saltrecord2 = new FS::access_user_session { - usernum => 3, - sessionkey => $cookie, - start_date => 2, # Increment start date for replace - sessionnum => $saltrecord1->{'Hash'}->{'sessionnum'}, - }; - $error = $saltrecord2->replace($saltrecord1); - ok(!$error, 'Replace hashed record (salted with 1st salt) in access_user_session with different start_date') or diag explain $error,$saltrecord2,$saltrecord1; - @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - ok(scalar @records == 1,'qsearch finds single matching record (salted with 1st salt) just replaced'); - cmp_ok($records[0]->sessionkey,'eq',$shacookie1,'First Hashed sessionkey matches'); - cmp_ok($records[0]->start_date,'==',2,'Replaced record has new start date'); - # 3. Salt no. 2 local $FS::Record::conf_hashsalt = 'test it2'; my $salt2record1 = new FS::access_user_session { @@ -102,34 +76,5 @@ SKIP: { @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); ok(scalar @records == 1,'qsearch finds single matching record (salted with 2nd salt) just inserted with second salt'); cmp_ok($records[0]->sessionkey,'eq',$shacookie2,'Second salted sessionkey matches (salted with 2nd salt) matches'); - - my $salt2record2 = new FS::access_user_session { - usernum => 3, - sessionkey => $cookie, - start_date => 2, # Increment start date for replace - sessionnum => $salt2record1->{'Hash'}->{'sessionnum'}, - }; - $error = $salt2record2->replace($salt2record1); - ok(!$error, 'Replace 2nd hashed record (salted with 2nd salt) access_user_session with different start_date') or diag explain $error,$salt2record2,$salt2record1; - @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - ok(scalar @records == 1,'qsearch finds single matching record (salted with 2nd salt) just replaced'); - cmp_ok($records[0]->sessionkey,'eq',$shacookie2,'2nd hashed cookie matches'); - cmp_ok($records[0]->start_date,'==',2,'Replaced record has new start date'); - - # 4. No salt ... again - # Update the unsalted (unhashed) record in step 1. - local $FS::Record::conf_hashsalt = ''; - my $record3 = new FS::access_user_session { - usernum => 3, - sessionkey => $cookie, - start_date => 4, # Increment start date for replace - sessionnum => $record1->{'Hash'}->{'sessionnum'}, - }; - $error = $record3->replace($record2); - ok(!$error, 'Replace unhashed record (2nd time around) in access_user_session with different start_date') or diag explain $error,$record3,$record2; - @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - cmp_ok($records[0]->sessionkey,'eq',$cookie,'2nd time around, able to replace original plain cookie/sessionnum'); - cmp_ok($records[0]->start_date,'==',4,'2nd time around, replaced record has even newer start_date'); - } From d9fd356b9932788888a8f579d0138b08dd45fdb5 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Mon, 14 Dec 2015 14:40:17 -0700 Subject: [PATCH 05/11] In FS::Record, use sha512_hex for sessionkey --- FS/FS/Record.pm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm index 84abc710de..33d7217a5d 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -22,7 +22,7 @@ use FS::Schema qw(dbdef); use FS::SearchCache; use FS::Msgcat qw(gettext); #use FS::Conf; #dependency loop bs, in install_callback below instead -use Digest::SHA qw(sha256_hex); +use Digest::SHA qw(sha512_hex); use FS::part_virtual_field; @@ -433,7 +433,7 @@ sub qsearch { ) { my $value = $record->{$field}; - # If searching for the user session, search for SHA256'ed + # If searching for the user session, search for SHA512'ed # (Also works for other ::hashed_fields ...) if ( $conf_hashsalt && defined(eval '@FS::'. $stable . '::hashed_fields') @@ -441,7 +441,7 @@ sub qsearch { ) { foreach my $hashed_field_name (eval '@FS::'. $stable . '::hashed_fields') { next if $hashed_field_name ne $field; # continue if this isn't a hashed field - $value = sha256_hex($value.$conf_hashsalt); + $value = sha512_hex($value.$conf_hashsalt); } } @@ -1329,14 +1329,14 @@ sub insert { } } - # SHA256 before the database + # SHA512 before the database if ( $conf_hashsalt && defined(eval '@FS::'. $table . '::hashed_fields') && scalar( eval '@FS::'. $table . '::hashed_fields') ) { foreach my $field (eval '@FS::'. $table . '::hashed_fields') { $saved->{$field} = $self->getfield($field); - $self->setfield($field, sha256_hex($self->getfield($field).$conf_hashsalt)); + $self->setfield($field, sha512_hex($self->getfield($field).$conf_hashsalt)); } } From 1a08a4b5eff952b3f780f442b70e7106f9048c55 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Mon, 14 Dec 2015 14:41:55 -0700 Subject: [PATCH 06/11] In access_user_session table, expand sessionkey to 128 (from 80) for sha512_hex --- FS/FS/Schema.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm index c8b9b631da..d307810099 100644 --- a/FS/FS/Schema.pm +++ b/FS/FS/Schema.pm @@ -5623,7 +5623,7 @@ sub tables_hashref { 'access_user_session' => { 'columns' => [ 'sessionnum', 'serial', '', '', '', '', - 'sessionkey', 'varchar', '', $char_d, '', '', + 'sessionkey', 'varchar', '', 128, '', '', 'usernum', 'int', '', '', '', '', 'start_date', @date_type, '', '', 'last_date', @date_type, '', '', From dc83c647745c4753ec78a0c25ca9d93292575135 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Mon, 14 Dec 2015 14:46:56 -0700 Subject: [PATCH 07/11] Generate initial hashsalt (for sessionkeys) if it doesn't exist already --- FS/FS/Upgrade.pm | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm index ffc04bab77..d97a467b4f 100644 --- a/FS/FS/Upgrade.pm +++ b/FS/FS/Upgrade.pm @@ -156,6 +156,22 @@ If you need to continue using the old Form 477 report, turn on the enable_banned_pay_pad() unless length($conf->config('banned_pay-pad')); + # generate initial hashsalt (for sessionkeys) if it doesn't exist already + my @conf_hashsalt_key = grep { /^hashsalt$/ } map { $_->{'key'} } @FS::Conf::config_items; + if (scalar(@conf_hashsalt_key)) { # using hash salts + my $hashsalt_entry = qsearchs('conf', { 'name' => 'hashsalt'}); + unless ($hashsalt_entry) { + my $salt_length = int(rand(26)) + 25; # between 25 an 50 inclusive + my @chars = ("A" .. "Z", "a" .. "z", 0 .. 9, qw(! $ % ^ *) ); + my $init_salt = join("", @chars[ map { rand @chars } ( 1 .. $salt_length ) ]); + my $hs = new FS::conf { + 'name' => 'hashsalt', + 'value' => $init_salt, + }; + my $ret = $hs->insert; + warn "Unable to insert default hash salt for session keys\n" if $ret; + } + } } sub upgrade_overlimit_groups { From 4da342c44e7bc76b36b5e1ad5fb7553b14c806d5 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Mon, 14 Dec 2015 14:50:01 -0700 Subject: [PATCH 08/11] Return t/Record.t back to its original content --- FS/t/Record.t | 85 +++------------------------------------------------ 1 file changed, 5 insertions(+), 80 deletions(-) diff --git a/FS/t/Record.t b/FS/t/Record.t index 44ecc21933..00de1eda30 100644 --- a/FS/t/Record.t +++ b/FS/t/Record.t @@ -1,80 +1,5 @@ -use strict; -use warnings; -use FS::Conf; -use Data::Dumper qw( Dumper ); -use FS::UID qw( adminsuidsetup ); -use Digest::SHA qw(sha256_hex); -use Test::More tests => 11; - -# Stock freeside does equivalent of the following -BEGIN { use_ok('FS::Record') } - -# If adminuser passed in, assume this is being run in foreground -my $freeside_uid = getpwnam('freeside'); -my $adminuser = shift || ''; -if (!length $adminuser) { - note <<"USAGE"; -Syntax for longer user-initiated tests: - prove $0 :: [admin-user_name] - OR - perl $0 [admin-user_name] - -USAGE -} - -SKIP: { - skip 'test(s) if not run as freeside user or if DB adminuser unspecified', 10 - if ($< != $freeside_uid) || !length($adminuser); - - require_ok('FS::access_user_session'); - - my $dbh = adminsuidsetup $adminuser; - - my $conf = FS::Conf->new; - - # 1. Without salt - local $FS::Record::conf_hashsalt = ''; - my $cookie = 'this is a test '. time(); - my $record1 = new FS::access_user_session { - usernum => 3, - sessionkey => $cookie, - start_date => 0, - }; - - my $error = $record1->insert; - ok(!$error, 'Insert unhashed record in access_user_session') or diag explain $error,$record1; - my @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - ok(scalar @records == 1,'qsearch finds single matching record just inserted'); - cmp_ok($records[0]->sessionkey,'eq',$cookie,'Unhashed sessionkey matches'); - - # 2. First salt - local $FS::Record::conf_hashsalt = 'test it'; - my $saltrecord1 = new FS::access_user_session { - usernum => 3, - sessionkey => $cookie, - start_date => 0, - }; - - $error = $saltrecord1->insert; - ok(!$error, 'Insert record in access_user_session with first salt') or diag explain $error,$saltrecord1; - my $shacookie1 = sha256_hex($cookie.$FS::Record::conf_hashsalt); - @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - ok(scalar @records == 1,'qsearch finds single matching record just inserted with first salt'); - cmp_ok($records[0]->sessionkey,'eq',$shacookie1,'First salted sessionkey matches'); - - # 3. Salt no. 2 - local $FS::Record::conf_hashsalt = 'test it2'; - my $salt2record1 = new FS::access_user_session { - usernum => 3, - sessionkey => $cookie, - start_date => 0, - }; - - $error = $salt2record1->insert; - ok(!$error, 'Insert record in access_user_session with 2nd salt') or diag explain $error,$salt2record1; - my $shacookie2 = sha256_hex($cookie.$FS::Record::conf_hashsalt); - @records = FS::Record::qsearch( access_user_session => { sessionkey => $cookie } ); - ok(scalar @records == 1,'qsearch finds single matching record (salted with 2nd salt) just inserted with second salt'); - cmp_ok($records[0]->sessionkey,'eq',$shacookie2,'Second salted sessionkey matches (salted with 2nd salt) matches'); -} - +BEGIN { $| = 1; print "1..1\n" } +END {print "not ok 1\n" unless $loaded;} +use FS::Record; +$loaded=1; +print "ok 1\n"; From 55d7f62f60b49e524b6e55c96062da32e003a096 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Mon, 14 Dec 2015 16:02:18 -0700 Subject: [PATCH 09/11] Modified FS::Conf description to state that changing the salt will force a new login for all sessions in progress. --- FS/FS/Conf.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index 27520e16ed..85de4ffc1e 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -834,7 +834,7 @@ my $validate_email = sub { $_[0] =~ { 'key' => 'hashsalt', 'section' => 'billing', - 'description' => 'Hash salt string (enables hashing of session keys/cookies)', + 'description' => 'Hash salt string enables hashing of session keys/cookies. Changing salt string will force a new login for all sessions in progress.', 'type' => 'text', }, From a2eaeff4583a5fe4fe39eb9c8e2404615be917b5 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Mon, 14 Dec 2015 16:18:46 -0700 Subject: [PATCH 10/11] Fix typo in comment --- FS/FS/Upgrade.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm index d97a467b4f..c14cc7cb95 100644 --- a/FS/FS/Upgrade.pm +++ b/FS/FS/Upgrade.pm @@ -161,7 +161,7 @@ If you need to continue using the old Form 477 report, turn on the if (scalar(@conf_hashsalt_key)) { # using hash salts my $hashsalt_entry = qsearchs('conf', { 'name' => 'hashsalt'}); unless ($hashsalt_entry) { - my $salt_length = int(rand(26)) + 25; # between 25 an 50 inclusive + my $salt_length = int(rand(26)) + 25; # between 25 and 50 inclusive my @chars = ("A" .. "Z", "a" .. "z", 0 .. 9, qw(! $ % ^ *) ); my $init_salt = join("", @chars[ map { rand @chars } ( 1 .. $salt_length ) ]); my $hs = new FS::conf { From fd0c91d26496358d9f87ed033722f9f884e4f927 Mon Sep 17 00:00:00 2001 From: Weldon Whipple Date: Tue, 15 Dec 2015 09:19:27 -0700 Subject: [PATCH 11/11] Tweak description of hashsalt in FS::Conf --- FS/FS/Conf.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index 85de4ffc1e..6d798b2f03 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -834,7 +834,7 @@ my $validate_email = sub { $_[0] =~ { 'key' => 'hashsalt', 'section' => 'billing', - 'description' => 'Hash salt string enables hashing of session keys/cookies. Changing salt string will force a new login for all sessions in progress.', + 'description' => 'Hash salt string (enables hashing of session keys/cookies). Changing salt string will force a new login for all sessions in progress.', 'type' => 'text', },