limit password reuse, core and svc_acct, #29354
authorMark Wells <mark@freeside.biz>
Fri, 13 Nov 2015 00:49:39 +0000 (16:49 -0800)
committerMark Wells <mark@freeside.biz>
Fri, 13 Nov 2015 00:52:51 +0000 (16:52 -0800)
12 files changed:
FS/FS/ClientAPI/MyAccount.pm
FS/FS/ClientAPI/Signup.pm
FS/FS/Conf.pm
FS/FS/Mason.pm
FS/FS/Password_Mixin.pm [new file with mode: 0644]
FS/FS/Schema.pm
FS/FS/password_history.pm [new file with mode: 0644]
FS/FS/svc_acct.pm
FS/MANIFEST
FS/t/password_history.t [new file with mode: 0644]
httemplate/edit/process/svc_acct.cgi
httemplate/misc/process/change-password.html

index 10b2652..9bbde88 100644 (file)
@@ -2947,13 +2947,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.
@@ -3217,8 +3219,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;
 
index 3208396..a9678b0 100644 (file)
@@ -698,6 +698,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;
index fad786c..c259b7a 100644 (file)
@@ -4252,6 +4252,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',
index 9a4a892..ba56cff 100644 (file)
@@ -384,6 +384,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 (file)
index 0000000..af4c5e2
--- /dev/null
@@ -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;
index d0b69ca..e55a8f1 100644 (file)
@@ -4904,6 +4904,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 (file)
index 0000000..dd527b9
--- /dev/null
@@ -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;
+
index 9636b3e..a8cb7ba 100644 (file)
@@ -7,7 +7,11 @@ use base qw( FS::svc_Domain_Mixin
              FS::svc_Radius_Mixin
              FS::svc_Tower_Mixin
              FS::svc_IP_Mixin
-             FS::svc_Common );
+             FS::Password_Mixin
+             FS::svc_Common
+           );
+
+use strict;
 use vars qw( $DEBUG $me $conf $skip_fuzzyfiles
              $dir_prefix @shells $usernamemin
              $usernamemax $passwordmin $passwordmax
@@ -701,6 +705,9 @@ sub insert {
     'child_objects' => $self->child_objects,
     %options,
   );
+
+  $error ||= $self->insert_password_history;
+
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
@@ -985,6 +992,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;
index 4f3b6aa..b3679b5 100644 (file)
@@ -798,3 +798,5 @@ FS/cust_pkg_reason_fee.pm
 t/cust_pkg_reason_fee.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 (file)
index 0000000..b7a05fd
--- /dev/null
@@ -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";
index 9cac2c5..d75ff92 100755 (executable)
@@ -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 ) {
index 7cab9c4..d58ce54 100644 (file)
@@ -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...