[freeside-commits] branch master updated. 45d0f6c6325fb8ab5fdc478a7dc278872defa479
Jonathan Prykop
jonathan at 420.am
Fri Nov 20 23:59:36 PST 2015
The branch, master has been updated
via 45d0f6c6325fb8ab5fdc478a7dc278872defa479 (commit)
from 8248d1c6ba608044c8f66a53daab254f476d5c6d (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 45d0f6c6325fb8ab5fdc478a7dc278872defa479
Author: Jonathan Prykop <jonathan at freeside.biz>
Date: Sat Nov 21 01:54:21 2015 -0600
RT#29354: Password Security in Email
diff --git a/FS/FS/ClientAPI/MyAccount.pm b/FS/FS/ClientAPI/MyAccount.pm
index a6bde82..9847e5f 100644
--- a/FS/FS/ClientAPI/MyAccount.pm
+++ b/FS/FS/ClientAPI/MyAccount.pm
@@ -2995,12 +2995,6 @@ 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);
-
$error ||= $svc_acct->is_password_allowed($p->{'new_password'})
|| $svc_acct->set_password($p->{'new_password'})
|| $svc_acct->replace();
diff --git a/FS/FS/Password_Mixin.pm b/FS/FS/Password_Mixin.pm
index 990f735..bcad546 100644
--- a/FS/FS/Password_Mixin.pm
+++ b/FS/FS/Password_Mixin.pm
@@ -6,8 +6,13 @@ use FS::password_history;
use Authen::Passphrase;
use Authen::Passphrase::BlowfishCrypt;
# https://rt.cpan.org/Ticket/Display.html?id=72743
+use Data::Password qw(:all);
our $DEBUG = 0;
+our $conf;
+FS::UID->install_callback( sub {
+ $conf = FS::Conf->new;
+});
our $me = '[' . __PACKAGE__ . ']';
@@ -38,7 +43,34 @@ sub is_password_allowed {
my $self = shift;
my $password = shift;
- # check length and complexity here
+ # basic checks using Data::Password;
+ # options for Data::Password
+ $DICTIONARY = 4; # minimum length of disallowed words
+ $MINLEN = $conf->config('passwordmin') || 6;
+ $MAXLEN = $conf->config('passwordmax') || 8;
+ $GROUPS = 4; # must have all 4 'character groups': numbers, symbols, uppercase, lowercase
+ # other options use the defaults listed below:
+ # $FOLLOWING = 3; # disallows more than 3 chars in a row, by alphabet or keyboard (ie abcd or asdf)
+ # $SKIPCHAR = undef; # set to true to skip checking for bad characters
+ # # lists of disallowed words
+ # @DICTIONARIES = qw( /usr/share/dict/web2 /usr/share/dict/words /usr/share/dict/linux.words );
+
+ my $error = IsBadPassword($password);
+ $error = 'must contain at least one each of numbers, symbols, and lowercase and uppercase letters'
+ if $error eq 'contains less than 4 character groups'; # avoid confusion
+ $error = 'Invalid password - ' . $error if $error;
+ return $error if $error;
+
+ #check against known usernames
+ my @disallowed_names = $self->password_disallowed_names;
+ foreach my $noname (@disallowed_names) {
+ if ($password =~ /$noname/i) {
+ #keeping message ambiguous to avoid leaking personal info
+ return 'Password contains a disallowed word';
+ }
+ }
+
+ return '' unless $self->get($self->primary_key); # for validating new passwords pre-insert
my $no_reuse = 3;
# allow override here if we really must
@@ -75,6 +107,17 @@ sub is_password_allowed {
'';
}
+=item password_disallowed_names
+
+Override to return a list additional words (eg usernames) not
+to be used by passwords on this service.
+
+=cut
+
+sub password_disallowed_names {
+ return ();
+}
+
=item password_history_key
Returns the name of the field in L<FS::password_history> that's the foreign
diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index 9323976..e7ec4a2 100644
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -2676,6 +2676,31 @@ sub virtual_maildir {
$self->domain. '/maildirs/'. $self->username. '/';
}
+=item password_disallowed_names
+
+Override, for L<FS::Password_Mixin>. Not really intended for other use.
+
+=cut
+
+sub password_disallowed_names {
+ my $self = shift;
+ my $dbh = dbh;
+ my $results = {};
+ foreach my $field ( qw( username finger ) ) {
+ my $sql = 'SELECT DISTINCT '.$field.' FROM svc_acct';
+ my $sth = $dbh->prepare($sql)
+ or die "Error preparing $sql: ". $dbh->errstr;
+ $sth->execute()
+ or die "Error executing $sql: ". $sth->errstr;
+ foreach my $row (@{$sth->fetchall_arrayref}, $self->get($field)) {
+ foreach my $word (split(/\s+/,$$row[0])) {
+ $results->{lc($word)} = 1;
+ }
+ }
+ }
+ return keys %$results;
+}
+
=back
=head1 CLASS METHODS
diff --git a/debian/control b/debian/control
index 2764f70..30e9f31 100644
--- a/debian/control
+++ b/debian/control
@@ -27,7 +27,8 @@ Depends: gnupg,ghostscript,gsfonts,gzip,latex-xcolor,
libcache-simple-timedexpiry-perl,libchart-perl,libclass-container-perl,
libclass-data-inheritable-perl,libclass-returnvalue-perl,libcolor-scheme-perl,
libcompress-zlib-perl,libconvert-binhex-perl,libcrypt-passwdmd5-perl,
- libcrypt-ssleay-perl,libcss-squish-perl,libdate-manip-perl,libdbd-mysql-perl,
+ libcrypt-ssleay-perl,libcss-squish-perl,
+ libdata-password-perl,libdate-manip-perl,libdbd-mysql-perl,
libdbd-pg-perl,libdbi-perl,libdbix-dbschema-perl,libdbix-searchbuilder-perl,
libdevel-stacktrace-perl,libdevel-symdump-perl,liberror-perl,
libexcel-writer-xlsx-perl,libexception-class-perl,libfile-counterfile-perl,
diff --git a/httemplate/edit/svc_acct.cgi b/httemplate/edit/svc_acct.cgi
index 31678a9..0cf0c20 100755
--- a/httemplate/edit/svc_acct.cgi
+++ b/httemplate/edit/svc_acct.cgi
@@ -50,7 +50,12 @@
'required' => $part_svc->part_svc_column('_password')->required ) %>
<TD>
<INPUT TYPE="text" ID="clear_password" NAME="clear_password" VALUE="<% $password %>" SIZE=<% $pmax2 %> MAXLENGTH=<% $pmax %>>
- <& /elements/random_pass.html, 'clear_password' &>
+ <& /elements/random_pass.html, 'clear_password' &><BR>
+ <DIV ID="clear_password_result" STYLE="font-size: smaller"></DIV>
+ <& '/elements/validate_password.html',
+ 'fieldid' => 'clear_password',
+ 'svcnum' => $svcnum
+ &>
</TD>
</TR>
%}else{
diff --git a/httemplate/elements/change_password.html b/httemplate/elements/change_password.html
index 625ba1f..7d8daae 100644
--- a/httemplate/elements/change_password.html
+++ b/httemplate/elements/change_password.html
@@ -16,6 +16,12 @@
<& /elements/random_pass.html, $pre.'password', 'randomize' &>
<INPUT TYPE="submit" VALUE="change">
<INPUT TYPE="button" VALUE="cancel" onclick="<%$pre%>toggle(false)">
+ <DIV ID="<%$pre%>password_result" STYLE="font-size: smaller"></DIV>
+ <& '/elements/validate_password.html',
+ 'fieldid' => $pre.'password',
+ 'svcnum' => $svc_acct->svcnum,
+
+ &>
% if ( $error ) {
<BR><SPAN STYLE="color: #ff0000"><% $error |h %></SPAN>
% }
diff --git a/httemplate/elements/random_pass.html b/httemplate/elements/random_pass.html
index b215b77..14bbb58 100644
--- a/httemplate/elements/random_pass.html
+++ b/httemplate/elements/random_pass.html
@@ -1,13 +1,23 @@
<INPUT TYPE="button" VALUE="<% emt($label) %>" onclick="randomPass()">
<SCRIPT TYPE="text/javascript">
function randomPass() {
+ var lower='<% join('', 'a'..'z') %>';
+ var upper='<% join('', 'A'..'Z') %>';
+ var number='<% join('', '0'..'9') %>';
+ var symbol='`~!@#$%^&*-_=+:;<>,.?';
+ var pw_set=lower+upper+number+symbol;
+ var pass=[];
+ pass.push(lower.charAt(Math.floor(Math.random() * lower.length)));
+ pass.push(upper.charAt(Math.floor(Math.random() * lower.length)));
+ pass.push(number.charAt(Math.floor(Math.random() * number.length)));
+ pass.push(symbol.charAt(Math.floor(Math.random() * symbol.length)));
var i=0;
- var pw_set='<% join('', 'a'..'z', 'A'..'Z', '0'..'9' ) %>';
- var pass='';
- while(i < 8) {
+ while(i < 4) {
i++;
- pass += pw_set.charAt(Math.floor(Math.random() * pw_set.length));
+ pass.push(pw_set.charAt(Math.floor(Math.random() * pw_set.length)));
}
+ for(var j, x, i = pass.length; i; j = Math.floor(Math.random() * i), x = pass[--i], pass[i] = pass[j], pass[j] = x);
+ pass = pass.join('');
document.getElementById('<% $id %>').value = pass;
}
</SCRIPT>
diff --git a/httemplate/elements/validate_password.html b/httemplate/elements/validate_password.html
new file mode 100644
index 0000000..fd2cb6c
--- /dev/null
+++ b/httemplate/elements/validate_password.html
@@ -0,0 +1,58 @@
+<%doc>
+
+To validate passwords via javascript/xmlhttp:
+
+ <INPUT ID="password_field" TYPE="text">
+ <DIV ID="password_field_result">
+ <& '/elements/validate_password.html',
+ fieldid => 'password_field',
+ svcnum => $svcnum
+ &>
+
+The ID of the input field can be anything; the ID of the DIV in which to display results
+should be the input id plus '_result'.
+
+</%doc>
+
+<& '/elements/xmlhttp.html',
+ 'url' => $p.'misc/xmlhttp-validate_password.html',
+ 'subs' => [ 'validate_password' ],
+ 'method' => 'POST', # important not to put passwords in url
+&>
+<SCRIPT>
+function add_password_validation (fieldid) {
+ var inputfield = document.getElementById(fieldid);
+ inputfield.onchange = function () {
+ var fieldid = this.id+'_result';
+ var resultfield = document.getElementById(fieldid);
+ if (this.value) {
+ resultfield.innerHTML = '<SPAN STYLE="color: blue;">Validating password...</SPAN>';
+ validate_password('fieldid',fieldid,'svcnum','<% $opt{'svcnum'} %>','password',this.value,
+ function (result) {
+ result = JSON.parse(result);
+ var resultfield = document.getElementById(result.fieldid);
+ if (resultfield) {
+ if (result.valid) {
+ resultfield.innerHTML = '<SPAN STYLE="color: green;">Password valid!</SPAN>';
+ } else if (result.error) {
+ resultfield.innerHTML = '<SPAN STYLE="color: red;">'+result.error+'</SPAN>';
+ } else {
+ result.syserror = result.syserror || 'Server error';
+ resultfield.innerHTML = '<SPAN STYLE="color: red;">'+result.syserror+'</SPAN>';
+ }
+ }
+ }
+ );
+ } else {
+ resultfield.innerHTML = '';
+ }
+ };
+}
+add_password_validation('<% $opt{'fieldid'} %>');
+</SCRIPT>
+
+<%init>
+my %opt = @_;
+</%init>
+
+
diff --git a/httemplate/misc/xmlhttp-validate_password.html b/httemplate/misc/xmlhttp-validate_password.html
new file mode 100644
index 0000000..28dbf64
--- /dev/null
+++ b/httemplate/misc/xmlhttp-validate_password.html
@@ -0,0 +1,50 @@
+<%doc>
+Requires cgi params 'password' (plaintext) and 'sub' ('validate_password' is only
+acceptable value.) Also accepts 'svcnum' (for svc_acct, will otherwise create an
+empty dummy svc_acct) and 'fieldid' (for html post-processing, passed along in
+results for convenience.)
+
+Returns a json-encoded hashref with keys of 'valid' (set to 1 if object is valid),
+'error' (error text if password is invalid) or 'syserror' (error text if password
+could not be validated.) Only one of these keys will be set. Will also set
+'fieldid' if it was passed.
+</%doc>
+
+<% encode_json($result) %>
+
+<%init>
+
+my $validate_password = sub {
+ my %arg = $cgi->param('arg');
+ my %result;
+
+ $result{'fieldid'} = $arg{'fieldid'}
+ if $arg{'fieldid'} =~ /^\w+$/;
+
+ $result{'syserror'} = 'Request is not POST' unless $cgi->request_method eq 'POST';
+ return \%result if $result{'syserror'};
+
+ my $password = $arg{'password'};
+ $result{'syserror'} = 'Invoked without password' unless $password;
+ return \%result if $result{'syserror'};
+
+ my $svcnum = $arg{'svcnum'};
+ $result{'syserror'} = 'Invalid svcnum' unless $svcnum =~ /^\d*$/;
+ return \%result if $result{'syserror'};
+
+ my $svc_acct = $svcnum
+ ? qsearchs('svc_acct',{'svcnum' => $svcnum})
+ : (new FS::svc_acct {});
+ $result{'syserror'} = 'Could not find service' unless $svc_acct;
+ return \%result if $result{'syserror'};
+
+ $result{'error'} = $svc_acct->is_password_allowed($password);
+ $result{'valid'} = 1 unless $result{'error'};
+ return \%result;
+};
+
+my $result = ($cgi->param('sub') eq 'validate_password')
+ ? &$validate_password()
+ : { 'syserror' => 'Invalid sub' };
+
+</%init>
-----------------------------------------------------------------------
Summary of changes:
FS/FS/ClientAPI/MyAccount.pm | 6 ---
FS/FS/Password_Mixin.pm | 45 +++++++++++++++++-
FS/FS/svc_acct.pm | 25 ++++++++++
debian/control | 3 +-
httemplate/edit/svc_acct.cgi | 7 ++-
httemplate/elements/change_password.html | 6 +++
httemplate/elements/random_pass.html | 18 ++++++--
httemplate/elements/validate_password.html | 58 ++++++++++++++++++++++++
httemplate/misc/xmlhttp-validate_password.html | 50 ++++++++++++++++++++
9 files changed, 205 insertions(+), 13 deletions(-)
create mode 100644 httemplate/elements/validate_password.html
create mode 100644 httemplate/misc/xmlhttp-validate_password.html
More information about the freeside-commits
mailing list