summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Wells <mark@freeside.biz>2015-11-12 16:49:39 -0800
committerMark Wells <mark@freeside.biz>2015-11-13 13:00:42 -0500
commitd3f33fa4dbabb61cd94dac9f4f63cd8f249313da (patch)
treef9559ddf824ab90fcc68e1854e1219caeef5e907
parent75a3ac488dc908290e75edd09471e01dba2199cb (diff)
limit password reuse, core and svc_acct, #29354
-rw-r--r--FS/FS/ClientAPI/MyAccount.pm11
-rw-r--r--FS/FS/ClientAPI/Signup.pm3
-rw-r--r--FS/FS/Conf.pm7
-rw-r--r--FS/FS/Mason.pm1
-rw-r--r--FS/FS/Password_Mixin.pm165
-rw-r--r--FS/FS/Schema.pm46
-rw-r--r--FS/FS/password_history.pm174
-rw-r--r--FS/FS/svc_acct.pm10
-rw-r--r--FS/MANIFEST2
-rw-r--r--FS/t/password_history.t5
-rwxr-xr-xhttemplate/edit/process/svc_acct.cgi7
-rw-r--r--httemplate/misc/process/change-password.html4
12 files changed, 429 insertions, 6 deletions
diff --git a/FS/FS/ClientAPI/MyAccount.pm b/FS/FS/ClientAPI/MyAccount.pm
index 6e76e1d..53deaaa 100644
--- a/FS/FS/ClientAPI/MyAccount.pm
+++ b/FS/FS/ClientAPI/MyAccount.pm
@@ -2982,13 +2982,15 @@ sub myaccount_passwd {
)
&& ! $svc_acct->check_password($p->{'old_password'});
+ # should move password length checks into is_password_allowed
$error = 'Password too short.'
if length($p->{'new_password'}) < ($conf->config('passwordmin') || 6);
$error = 'Password too long.'
if length($p->{'new_password'}) > ($conf->config('passwordmax') || 8);
- $svc_acct->set_password($p->{'new_password'});
- $error ||= $svc_acct->replace();
+ $error ||= $svc_acct->is_password_allowed($p->{'new_password'})
+ || $svc_acct->set_password($p->{'new_password'})
+ || $svc_acct->replace();
#regular pw change in self-service should change contact pw too, otherwise its
#way too confusing. hell its confusing they're separate at all, but alas.
@@ -3267,8 +3269,9 @@ sub process_reset_passwd {
if ( $svc_acct ) {
- $svc_acct->set_password($p->{'new_password'});
- my $error = $svc_acct->replace();
+ my $error ||= $svc_acct->is_password_allowed($p->{'new_password'})
+ || $svc_acct->set_password($p->{'new_password'})
+ || $svc_acct->replace();
return { %$info, 'error' => $error } if $error;
diff --git a/FS/FS/ClientAPI/Signup.pm b/FS/FS/ClientAPI/Signup.pm
index 5d719c4..a178bec 100644
--- a/FS/FS/ClientAPI/Signup.pm
+++ b/FS/FS/ClientAPI/Signup.pm
@@ -694,6 +694,9 @@ sub new_customer {
map { $_ => $packet->{$_} }
qw( username _password sec_phrase popnum domsvc ),
};
+
+ my $error = $svc->is_password_allowed($packet->{_password});
+ return { error => $error } if $error;
my @acct_snarf;
my $snarfnum = 1;
diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index ffe5302..a4cc871 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -4053,6 +4053,13 @@ and customer address. Include units.',
},
{
+ 'key' => 'password-no_reuse',
+ 'section' => 'password',
+ 'description' => 'Minimum number of password changes before a password can be reused. By default, passwords can be reused without restriction.',
+ 'type' => 'text',
+ },
+
+ {
'key' => 'datavolume-forcemegabytes',
'section' => 'UI',
'description' => 'All data volumes are expressed in megabytes',
diff --git a/FS/FS/Mason.pm b/FS/FS/Mason.pm
index 98a75c8..58b3da7 100644
--- a/FS/FS/Mason.pm
+++ b/FS/FS/Mason.pm
@@ -409,6 +409,7 @@ if ( -e $addl_handler_use_file ) {
use FS::report_batch;
use FS::report_batch;
use FS::report_batch;
+ use FS::password_history;
# Sammath Naur
if ( $FS::Mason::addl_handler_use ) {
diff --git a/FS/FS/Password_Mixin.pm b/FS/FS/Password_Mixin.pm
new file mode 100644
index 0000000..af4c5e2
--- /dev/null
+++ b/FS/FS/Password_Mixin.pm
@@ -0,0 +1,165 @@
+package FS::Password_Mixin;
+
+use FS::Record qw(qsearch);
+use FS::Conf;
+use FS::password_history;
+use Authen::Passphrase;
+# use Authen::Passphrase::BlowfishCrypt; # ha ha, no.
+# https://rt.cpan.org/Ticket/Display.html?id=72743
+
+our $DEBUG = 1;
+our $conf;
+FS::UID->install_callback( sub {
+ $conf = FS::Conf->new;
+ # this is safe
+ eval "use Authen::Passphrase::BlowfishCrypt;";
+});
+
+our $me = '[' . __PACKAGE__ . ']';
+
+our $BLOWFISH_COST = 10;
+
+=head1 NAME
+
+FS::Password_Mixin - Object methods for accounts that have passwords governed
+by the password policy.
+
+=head1 METHODS
+
+=over 4
+
+=item is_password_allowed PASSWORD
+
+Checks the password against the system password policy. Returns an error
+message on failure, an empty string on success.
+
+This MUST NOT be called from check(). It should be called by the office UI,
+self-service ClientAPI, or other I<user-interactive> code that processes a
+password change, and only if the user has taken some action with the intent
+of changing the password.
+
+=cut
+
+sub is_password_allowed {
+ my $self = shift;
+ my $password = shift;
+
+ # check length and complexity here
+
+ if ( $conf->config('password-no_reuse') =~ /^(\d+)$/ ) {
+
+ my $no_reuse = $1;
+
+ # "the last N" passwords includes the current password and the N-1
+ # passwords before that.
+ warn "$me checking password reuse limit of $no_reuse\n" if $DEBUG;
+ my @latest = qsearch({
+ 'table' => 'password_history',
+ 'hashref' => { $self->password_history_key => $self->get($self->primary_key) },
+ 'order_by' => " ORDER BY created DESC LIMIT $no_reuse",
+ });
+
+ # don't check the first one; reusing the current password is allowed.
+ shift @latest;
+
+ foreach my $history (@latest) {
+ warn "$me previous password created ".$history->created."\n" if $DEBUG;
+ if ( $history->password_equals($password) ) {
+ my $message;
+ if ( $no_reuse == 1 ) {
+ $message = "This password is the same as your previous password.";
+ } else {
+ $message = "This password was one of the last $no_reuse passwords on this account.";
+ }
+ return $message;
+ }
+ } #foreach $history
+
+ } # end of no_reuse checking
+
+ '';
+}
+
+=item password_history_key
+
+Returns the name of the field in L<FS::password_history> that's the foreign
+key to this table.
+
+=cut
+
+sub password_history_key {
+ my $self = shift;
+ $self->table . '__' . $self->primary_key;
+}
+
+=item insert_password_history
+
+Creates a L<FS::password_history> record linked to this object, with its
+current password.
+
+=cut
+
+sub insert_password_history {
+ my $self = shift;
+ my $encoding = $self->_password_encoding;
+ my $password = $self->_password;
+ my $auth;
+
+ if ( $encoding eq 'bcrypt' or $encoding eq 'crypt' ) {
+
+ # it's smart enough to figure this out
+ $auth = Authen::Passphrase->from_crypt($password);
+
+ } elsif ( $encoding eq 'ldap' ) {
+
+ $password =~ s/^{PLAIN}/{CLEARTEXT}/i; # normalize
+ $auth = Authen::Passphrase->from_rfc2307($password);
+ if ( $auth->isa('Authen::Passphrase::Clear') ) {
+ # then we've been given the password in cleartext
+ $auth = $self->_blowfishcrypt( $auth->passphrase );
+ }
+
+ } elsif ( $encoding eq 'plain' ) {
+
+ $auth = $self->_blowfishcrypt( $password );
+
+ }
+
+ my $password_history = FS::password_history->new({
+ _password => $auth->as_rfc2307,
+ created => time,
+ $self->password_history_key => $self->get($self->primary_key),
+ });
+
+ my $error = $password_history->insert;
+ return "recording password history: $error" if $error;
+ '';
+
+}
+
+=item _blowfishcrypt PASSWORD
+
+For internal use: takes PASSWORD and returns a new
+L<Authen::Passphrase::BlowfishCrypt> object representing it.
+
+=cut
+
+sub _blowfishcrypt {
+ my $class = shift;
+ my $passphrase = shift;
+ return Authen::Passphrase::BlowfishCrypt->new(
+ cost => $BLOWFISH_COST,
+ salt_random => 1,
+ passphrase => $passphrase,
+ );
+}
+
+=back
+
+=head1 SEE ALSO
+
+L<FS::password_history>
+
+=cut
+
+1;
diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 1c6e993..9f2cb76 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -7187,6 +7187,52 @@ sub tables_hashref {
],
},
+ 'password_history' => {
+ 'columns' => [
+ 'passwordnum', 'serial', '', '', '', '',
+ '_password', 'varchar', 'NULL', $char_d, '', '',
+ 'encryption_method', 'varchar', 'NULL', $char_d, '', '',
+ 'created', @date_type, '', '',
+ # each table that needs password history gets a column here, and
+ # an entry in foreign_keys.
+ 'svc_acct__svcnum', 'int', 'NULL', '', '', '',
+ 'svc_dsl__svcnum', 'int', 'NULL', '', '', '',
+ 'svc_alarm__svcnum', 'int', 'NULL', '', '', '',
+ 'agent__agentnum', 'int', 'NULL', '', '', '',
+ 'contact__contactnum', 'int', 'NULL', '', '', '',
+ 'access_user__usernum', 'int', 'NULL', '', '', '',
+ ],
+ 'primary_key' => 'passwordnum',
+ 'unique' => [],
+ 'index' => [],
+ 'foreign_keys' => [
+ { columns => [ 'svc_acct__svcnum' ],
+ table => 'svc_acct',
+ references => [ 'svcnum' ],
+ },
+ { columns => [ 'svc_dsl__svcnum' ],
+ table => 'svc_dsl',
+ references => [ 'svcnum' ],
+ },
+ { columns => [ 'svc_alarm__svcnum' ],
+ table => 'svc_alarm',
+ references => [ 'svcnum' ],
+ },
+ { columns => [ 'agent__agentnum' ],
+ table => 'agent',
+ references => [ 'agentnum' ],
+ },
+ { columns => [ 'contact__contactnum' ],
+ table => 'contact',
+ references => [ 'contactnum' ],
+ },
+ { columns => [ 'access_user__usernum' ],
+ table => 'access_user',
+ references => [ 'usernum' ],
+ },
+ ],
+ },
+
# name type nullability length default local
#'new_table' => {
diff --git a/FS/FS/password_history.pm b/FS/FS/password_history.pm
new file mode 100644
index 0000000..dd527b9
--- /dev/null
+++ b/FS/FS/password_history.pm
@@ -0,0 +1,174 @@
+package FS::password_history;
+use base qw( FS::Record );
+
+use strict;
+use FS::Record qw( qsearch qsearchs );
+use Authen::Passphrase;
+
+# the only bit of autogenerated magic in here
+our @foreign_keys;
+FS::UID->install_callback(sub {
+ @foreign_keys = grep /__/, __PACKAGE__->dbdef_table->columns;
+});
+
+=head1 NAME
+
+FS::password_history - Object methods for password_history records
+
+=head1 SYNOPSIS
+
+ use FS::password_history;
+
+ $record = new FS::password_history \%hash;
+ $record = new FS::password_history { 'column' => 'value' };
+
+ $error = $record->insert;
+
+ $error = $new_record->replace($old_record);
+
+ $error = $record->delete;
+
+ $error = $record->check;
+
+=head1 DESCRIPTION
+
+An FS::password_history object represents a current or past password used
+by a login account, employee, or other account managed within Freeside.
+FS::password_history inherits from FS::Record. The following fields are
+currently supported:
+
+=over 4
+
+=item passwordnum - primary key
+
+=item _password - the encrypted password, as an RFC2307-style string
+("{CRYPT}$2a$08$..." or "{MD5}1ab201f..." or similar). This is a serialized
+L<Authen::Passphrase> object.
+
+=item created - the date the password was set to this value. The record with
+the most recent created time is the current password.
+
+=back
+
+Plus one of the following foreign keys:
+
+=over 4
+
+=item svc_acct__svcnum
+
+=item svc_dsl__svcnum
+
+=item svc_alarm__svcnum
+
+=item agent__agentnum
+
+=item contact__contactnum
+
+=item access_user__usernum
+
+=back
+
+=head1 METHODS
+
+=over 4
+
+=item new HASHREF
+
+Creates a new password history record. To add the record to the database,
+see L<"insert">.
+
+=cut
+
+sub table { 'password_history'; }
+
+=item insert
+
+=item delete
+
+=item replace OLD_RECORD
+
+=item check
+
+Checks all fields to make sure this is a valid password history record. If
+there is an error, returns the error, otherwise returns false. Called by the
+insert and replace methods.
+
+=cut
+
+sub check {
+ my $self = shift;
+
+ my $error =
+ $self->ut_numbern('passwordnum')
+ || $self->ut_anything('_password')
+ || $self->ut_numbern('create')
+ || $self->ut_numbern('create')
+ ;
+ return $error if $error;
+
+ # FKs are mutually exclusive
+ my $fk_in_use;
+ foreach my $fk ( @foreign_keys ) {
+ if ( $self->get($fk) ) {
+ $self->ut_numbern($fk);
+ return "multiple records linked to this password_history" if $fk_in_use;
+ $fk_in_use = $fk;
+ }
+ }
+
+ $self->SUPER::check;
+}
+
+=item linked_acct
+
+Returns the object that's using this password.
+
+=cut
+
+sub linked_acct {
+ my $self = shift;
+
+ foreach my $fk ( @foreign_keys ) {
+ if ( my $val = $self->get($fk) ) {
+ my ($table, $key) = split(/__/, $fk);
+ return qsearchs($table, { $key => $val });
+ }
+ }
+}
+
+=item password_equals PASSWORD
+
+Returns true if PASSWORD (plaintext) is the same as the one stored in the
+history record, false if not.
+
+=cut
+
+sub password_equals {
+
+ my ($self, $check_password) = @_;
+
+ # _password here is always LDAP-style.
+ try {
+ my $auth = Authen::Passphrase->from_rfc2307($self->_password);
+ return $auth->match($check_password);
+ } catch {
+ # if there's somehow bad data in the _password field, then it doesn't
+ # match anything. much better than having it match _everything_.
+ warn "password_history #" . $self->passwordnum . ": $_";
+ return '';
+ }
+
+}
+
+=back
+
+=head1 BUGS
+
+=head1 SEE ALSO
+
+L<FS::Record>
+
+=cut
+
+1;
+
diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index f307033..d3e23f2 100644
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -4,6 +4,7 @@ use base qw( FS::svc_Domain_Mixin FS::svc_PBX_Mixin
FS::svc_Radius_Mixin
FS::svc_Tower_Mixin
FS::svc_IP_Mixin
+ FS::Password_Mixin
FS::svc_Common
);
@@ -684,6 +685,9 @@ sub insert {
'child_objects' => $self->child_objects,
%options,
);
+
+ $error ||= $self->insert_password_history;
+
if ( $error ) {
$dbh->rollback if $oldAutoCommit;
return $error;
@@ -893,6 +897,12 @@ sub replace {
my $dbh = dbh;
$error = $new->SUPER::replace($old, @_); # usergroup here
+
+ # don't need to record this unless the password was changed
+ if ( $old->_password ne $new->_password ) {
+ $error ||= $new->insert_password_history;
+ }
+
if ( $error ) {
$dbh->rollback if $oldAutoCommit;
return $error if $error;
diff --git a/FS/MANIFEST b/FS/MANIFEST
index 5041ccd..f1195ac 100644
--- a/FS/MANIFEST
+++ b/FS/MANIFEST
@@ -858,3 +858,5 @@ FS/report_batch.pm
t/report_batch.t
FS/report_batch.pm
t/report_batch.t
+FS/password_history.pm
+t/password_history.t
diff --git a/FS/t/password_history.t b/FS/t/password_history.t
new file mode 100644
index 0000000..b7a05fd
--- /dev/null
+++ b/FS/t/password_history.t
@@ -0,0 +1,5 @@
+BEGIN { $| = 1; print "1..1\n" }
+END {print "not ok 1\n" unless $loaded;}
+use FS::password_history;
+$loaded=1;
+print "ok 1\n";
diff --git a/httemplate/edit/process/svc_acct.cgi b/httemplate/edit/process/svc_acct.cgi
index 9cac2c5..d75ff92 100755
--- a/httemplate/edit/process/svc_acct.cgi
+++ b/httemplate/edit/process/svc_acct.cgi
@@ -81,7 +81,12 @@ if ( $cgi->param('clear_password') eq '*HIDDEN*'
|| $cgi->param('clear_password') =~ /^\(.* encrypted\)$/ ) {
die "fatal: no previous account to recall hidden password from!" unless $old;
} else {
- $error ||= $new->set_password($cgi->param('clear_password'));
+ my $newpass = $cgi->param('clear_password');
+ if ( ! $old->check_password($newpass) ) {
+ # then the password is being changed
+ $error ||= $new->is_password_allowed($newpass)
+ || $new->set_password($newpass);
+ }
}
if ( ! $error ) {
diff --git a/httemplate/misc/process/change-password.html b/httemplate/misc/process/change-password.html
index 7cab9c4..d58ce54 100644
--- a/httemplate/misc/process/change-password.html
+++ b/httemplate/misc/process/change-password.html
@@ -11,7 +11,9 @@ die "access denied" unless (
( $curuser->access_right('Edit password') and
! $part_svc->restrict_edit_password )
);
-my $error = $svc_acct->set_password($cgi->param('password'))
+my $newpass = $cgi->param('password');
+my $error = $svc_acct->is_password_allowed($newpass)
+ || $svc_acct->set_password($newpass)
|| $svc_acct->replace;
# annoyingly specific to view/svc_acct.cgi, for now...