From 4b147e668c23fd3011885ed94d84f4f3bb27c71f Mon Sep 17 00:00:00 2001 From: Jonathan Prykop Date: Mon, 7 Dec 2015 17:46:45 -0600 Subject: [PATCH] RT#29354: Password Security in Email [customer fields, images, js files] --- FS/FS/Password_Mixin.pm | 34 +++++++++++++++++++++ FS/FS/svc_acct.pm | 1 + ..._validation.html => add_password_validation.js} | 16 +++++----- .../FS-SelfService/cgi/change_password.html | 10 +++--- fs_selfservice/FS-SelfService/cgi/images/error.png | Bin 0 -> 666 bytes fs_selfservice/FS-SelfService/cgi/images/tick.png | Bin 0 -> 537 bytes .../cgi/process_forgot_password.html | 19 ++++++++++-- fs_selfservice/FS-SelfService/cgi/selfservice.cgi | 9 ++++++ .../cgi/{send_xmlhttp.html => send_xmlhttp.js} | 2 -- fs_selfservice/FS-SelfService/cgi/signup.cgi | 28 ----------------- fs_selfservice/FS-SelfService/cgi/signup.html | 12 ++++---- httemplate/elements/validate_password.html | 8 +++-- 12 files changed, 85 insertions(+), 54 deletions(-) rename fs_selfservice/FS-SelfService/cgi/{add_password_validation.html => add_password_validation.js} (54%) create mode 100644 fs_selfservice/FS-SelfService/cgi/images/error.png create mode 100644 fs_selfservice/FS-SelfService/cgi/images/tick.png rename fs_selfservice/FS-SelfService/cgi/{send_xmlhttp.html => send_xmlhttp.js} (98%) diff --git a/FS/FS/Password_Mixin.pm b/FS/FS/Password_Mixin.pm index 3129366c7..834fd6fc3 100644 --- a/FS/FS/Password_Mixin.pm +++ b/FS/FS/Password_Mixin.pm @@ -67,6 +67,40 @@ sub is_password_allowed { return '' unless $self->get($self->primary_key); # for validating new passwords pre-insert + #check against customer fields + my $cust_main = $self->cust_main; + if ($cust_main) { + my @words; + # words from cust_main + foreach my $field ( qw( last first daytime night fax mobile ) ) { + push @words, split(/\W/,$cust_main->get($field)); + } + # words from cust_location + foreach my $loc ($cust_main->cust_location) { + foreach my $field ( qw(address1 address2 city county state zip) ) { + push @words, split(/\W/,$loc->get($field)); + } + } + # words from cust_contact & contact_phone + foreach my $contact (map { $_->contact } $cust_main->cust_contact) { + foreach my $field ( qw(last first) ) { + push @words, split(/\W/,$contact->get($field)); + } + # not hugely useful right now, hyphenless stored values longer than password max, + # but max will probably be increased eventually... + foreach my $phone ( qsearch('contact_phone', {'contactnum' => $contact->contactnum}) ) { + push @words, split(/\W/,$phone->get('phonenum')); + } + } + # do the actual checking + foreach my $word (@words) { + next unless length($word) > 2; + if ($password =~ /$word/i) { + return qq(Password contains account information '$word'); + } + } + } + my $no_reuse = 3; # allow override here if we really must diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm index 38cebc1de..53b12f181 100644 --- a/FS/FS/svc_acct.pm +++ b/FS/FS/svc_acct.pm @@ -2686,6 +2686,7 @@ sub password_svc_check { my ($self, $password) = @_; foreach my $field ( qw(username finger) ) { foreach my $word (split(/\W+/,$self->get($field))) { + next unless length($word) > 2; if ($password =~ /$word/i) { return qq(Password contains account information '$word'); } diff --git a/fs_selfservice/FS-SelfService/cgi/add_password_validation.html b/fs_selfservice/FS-SelfService/cgi/add_password_validation.js similarity index 54% rename from fs_selfservice/FS-SelfService/cgi/add_password_validation.html rename to fs_selfservice/FS-SelfService/cgi/add_password_validation.js index e349fd7ad..e2e3227f1 100644 --- a/fs_selfservice/FS-SelfService/cgi/add_password_validation.html +++ b/fs_selfservice/FS-SelfService/cgi/add_password_validation.js @@ -1,5 +1,4 @@ - + diff --git a/fs_selfservice/FS-SelfService/cgi/change_password.html b/fs_selfservice/FS-SelfService/cgi/change_password.html index ef665545a..879faf285 100644 --- a/fs_selfservice/FS-SelfService/cgi/change_password.html +++ b/fs_selfservice/FS-SelfService/cgi/change_password.html @@ -28,11 +28,11 @@
-<%= include('send_xmlhttp') %> -<%= include('add_password_validation') %> - + + + diff --git a/fs_selfservice/FS-SelfService/cgi/images/error.png b/fs_selfservice/FS-SelfService/cgi/images/error.png new file mode 100644 index 0000000000000000000000000000000000000000..628cf2dae3d419ae220c8928ac71393b480745a3 GIT binary patch literal 666 zcmV;L0%iS)P)eOSYYtbpBV}~vsBnU!_?2tr-P=|^T zED%wc9ezHgW@NMb!^uT_|SvCpFLJylbx zY%bpaTGI8IYXMN$9w<3j9VkA~NYOKEQXsj?6a9_hcwfU$acAhJhB)zb_w@MVUEy@S zX&I>K-R!bhu3?(6bHWIg$HEl7{9g>>&l_qdd+UYb(1~BCo9LptNq&8>!yoJ3Ui(i5 zRJ|XnYBklL!{@$-7=3mJ>P@1c=7Oc79e-V7yf+%lD2!I;Y&nXBZ>=B!5?CB>LvEx6 znI%n)qqi$#X#wKB(U7XP2P=+4{b@j#r%9-K(8UqtSDk>0UKzf*HM9yqMZ1D!$2MdZ zR=`U>0zhOH1XqN?nY@AQqB7)Fp4{v&dKXvb43hZKvnN8;Po;+jY*}~*Z|W9Q0W%{D z^T}Cc<|r(Su=1K=P5>Z4 zg`et&Va}tdzBS-G-ZcO)zCWpJvGQwrHZ`@wpM420ac@bI5~KkTFfGEM3sPWO8co4^fI6lPnA)Y{ef%@{+SnoUk0+dW+*{8WvF8}}l07*qoM6N<$g7cXs A&j0`b literal 0 HcmV?d00001 diff --git a/fs_selfservice/FS-SelfService/cgi/images/tick.png b/fs_selfservice/FS-SelfService/cgi/images/tick.png new file mode 100644 index 0000000000000000000000000000000000000000..a9925a06ab02db30c1e7ead9c701c15bc63145cb GIT binary patch literal 537 zcmV+!0_OdRP)Hs{AQG2a)rMyf zFQK~pm1x3+7!nu%-M`k}``c>^00{o_1pjWJUTfl8mg=3qGEl8H@}^@w`VUx0_$uy4 z2FhRqKX}xI*?Tv1DJd8z#F#0c%*~rM30HE1@2o5m~}ZyoWhqv>ql{V z1ZGE0lgcoK^lx+eqc*rAX1Ky;Xx3U%u#zG!m-;eD1Qsn@kf3|F9qz~|95=&g3(7!X zB}JAT>RU;a%vaNOGnJ%e1=K6eAh43c(QN8RQ6~GP%O}Jju$~Ld*%`mO1p +
<%= if (!$error) { @@ -23,16 +24,27 @@ - + + + - - + + END @@ -40,6 +52,7 @@ END %>
New password: + + + + + + +
Re-enter new password:
+
<%= $body_footer %> diff --git a/fs_selfservice/FS-SelfService/cgi/selfservice.cgi b/fs_selfservice/FS-SelfService/cgi/selfservice.cgi index 5845122ef..aff9bcae6 100755 --- a/fs_selfservice/FS-SelfService/cgi/selfservice.cgi +++ b/fs_selfservice/FS-SelfService/cgi/selfservice.cgi @@ -95,6 +95,7 @@ my @nologin_actions = (qw( process_forgot_password do_process_forgot_password process_forgot_password_session + validate_password_nologin )); push @actions, @nologin_actions; my %nologin_actions = map { $_=>1 } @nologin_actions; @@ -1132,6 +1133,14 @@ sub validate_password { ) } +sub validate_password_nologin { + $action = 'validate_password'; #use same landing page + validate_passwd( + map { $_ => scalar($cgi->param($_)) } + qw( fieldid check_password ) + ) +} + #-- sub do_template { diff --git a/fs_selfservice/FS-SelfService/cgi/send_xmlhttp.html b/fs_selfservice/FS-SelfService/cgi/send_xmlhttp.js similarity index 98% rename from fs_selfservice/FS-SelfService/cgi/send_xmlhttp.html rename to fs_selfservice/FS-SelfService/cgi/send_xmlhttp.js index ac85cb2f7..e2991684a 100644 --- a/fs_selfservice/FS-SelfService/cgi/send_xmlhttp.html +++ b/fs_selfservice/FS-SelfService/cgi/send_xmlhttp.js @@ -1,4 +1,3 @@ - diff --git a/fs_selfservice/FS-SelfService/cgi/signup.cgi b/fs_selfservice/FS-SelfService/cgi/signup.cgi index 072ce96be..817fdd310 100755 --- a/fs_selfservice/FS-SelfService/cgi/signup.cgi +++ b/fs_selfservice/FS-SelfService/cgi/signup.cgi @@ -508,31 +508,3 @@ use FS::SelfService qw( regionselector expselect popselector domainselector didselector ); -sub add_password_validation { - my $fieldid = shift; - my $out = ''; - if ((-e './send_xmlhttp.html') && (-e './add_password_validation.html')) { - my $template = new Text::Template( TYPE => 'FILE', - SOURCE => "./send_xmlhttp.html", - DELIMITERS => [ '<%=', '%>' ], - UNTAINT => 1, - ) - or die $Text::Template::ERROR; - $out .= $template->fill_in( PACKAGE => 'FS::SelfService::_signupcgi' ); - $template = new Text::Template( TYPE => 'FILE', - SOURCE => "./add_password_validation.html", - DELIMITERS => [ '<%=', '%>' ], - UNTAINT => 1, - ) - or die $Text::Template::ERROR; - $out .= $template->fill_in( PACKAGE => 'FS::SelfService::_signupcgi' ); - $out .= < -add_password_validation('$fieldid'); - -ENDOUT - } - return $out; -} - - diff --git a/fs_selfservice/FS-SelfService/cgi/signup.html b/fs_selfservice/FS-SelfService/cgi/signup.html index 5900ba689..def52991e 100755 --- a/fs_selfservice/FS-SelfService/cgi/signup.html +++ b/fs_selfservice/FS-SelfService/cgi/signup.html @@ -386,12 +386,12 @@ ENDOUT Password -
-ENDOUT - - $OUT .= add_password_validation('new_password'); - - $OUT .= < + + + diff --git a/httemplate/elements/validate_password.html b/httemplate/elements/validate_password.html index fd2cb6ca0..a488c4f16 100644 --- a/httemplate/elements/validate_password.html +++ b/httemplate/elements/validate_password.html @@ -32,13 +32,15 @@ function add_password_validation (fieldid) { result = JSON.parse(result); var resultfield = document.getElementById(result.fieldid); if (resultfield) { + var errorimg = ''; + var validimg = ''; if (result.valid) { - resultfield.innerHTML = 'Password valid!'; + resultfield.innerHTML = validimg+'Password valid!'; } else if (result.error) { - resultfield.innerHTML = ''+result.error+''; + resultfield.innerHTML = errorimg+''+result.error+''; } else { result.syserror = result.syserror || 'Server error'; - resultfield.innerHTML = ''+result.syserror+''; + resultfield.innerHTML = errorimg+''+result.syserror+''; } } } -- 2.11.0