sipwise
/
ngcp-panel
mirror of https://github.com/sipwise/ngcp-panel.git
1
0
Fork 0
Code Issues Releases Wiki Activity

TT#151751 refactor persisting subscriber webpassword

Browse Source 
a multitude of issues popped after introducing bcrypted
webpasswords in the database. most recently the PATCH /api/susbcribers
rail was reported to reset the webpassword unintentionally.
subscriber login fails afterwards, which is a severe issue.

the bugs are adressed by this refactorings. the change also
introduces a global variable

  $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS

to control encrypting webpasswords. it is still enabled as of now,
but it's worth to consider disabling it. there other ways to have
a "cost" for an authentication request, eg. adding a simple
sleep(1sec).

Change-Id: I2d47d54a2d83568546ffdd2b211337a5f56be3a2
mr10.3
Rene Krenn 3 years ago committed by Hans-Peter Herzog
parent 55a8d5568f
commit 5e9066c4fb
7 changed files with 72 additions and 67 deletions
Show Stats Download Patch File Download Diff File

4
lib/NGCP/Panel/Controller/API/PasswordRecovery.pm
Unescape View File

@ -136,8 +136,10 @@ sub POST :Allow {
return; return;
} }
$c->log->debug("Updating subscriber password."); $c->log->debug("Updating subscriber password.");
my $webpassword = $form->params->{new_password};
$webpassword = NGCP::Panel::Utils::Auth::generate_salted_hash($webpassword) if $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS;
$subscriber->provisioning_voip_subscriber->update({ $subscriber->provisioning_voip_subscriber->update({
webpassword => NGCP::Panel::Utils::Auth::generate_salted_hash($form->params->{new_password}), webpassword => $webpassword,
}); });
$rs->delete; $rs->delete;
} }

5
lib/NGCP/Panel/Controller/Root.pm
Unescape View File

@ -629,7 +629,7 @@ sub login_jwt :Chained('/') :PathPart('login_jwt') :Args(0) :Method('POST') {
if ($authrs->first) { if ($authrs->first) {
my $password = $authrs->first->webpassword; my $password = $authrs->first->webpassword;
if (length $password > 40) { if (defined $password and length($password) > 40) {
my @splitted_pass = split /\$/, $password; my @splitted_pass = split /\$/, $password;
if (scalar @splitted_pass == 3) { if (scalar @splitted_pass == 3) {
#password is bcrypted with lower cost #password is bcrypted with lower cost
@ -664,8 +664,7 @@ sub login_jwt :Chained('/') :PathPart('login_jwt') :Args(0) :Method('POST') {
}, $pass)); }, $pass));
$auth_user = $authrs->search({webpassword => $db_b64salt . '$' . $usr_b64hash})->first; $auth_user = $authrs->search({webpassword => $db_b64salt . '$' . $usr_b64hash})->first;
} }
} } else {
else {
$auth_user = $authrs->search({webpassword => $pass})->first; $auth_user = $authrs->search({webpassword => $pass})->first;
} }
} }

15
lib/NGCP/Panel/Controller/Subscriber.pm
Unescape View File

@ -617,8 +617,10 @@ sub recover_webpassword :Chained('/') :PathPart('recoverwebpassword') :Args(0) {
$c->log->warn("invalid password recovery attempt for uuid '$uuid_string' from '".$c->qs($c->req->address)."'"); $c->log->warn("invalid password recovery attempt for uuid '$uuid_string' from '".$c->qs($c->req->address)."'");
$c->detach('/denied_page'); $c->detach('/denied_page');
} }
my $webpassword = $form->params->{password};
$webpassword = NGCP::Panel::Utils::Auth::generate_salted_hash($webpassword) if $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS;
$subscriber->provisioning_voip_subscriber->update({ $subscriber->provisioning_voip_subscriber->update({
webpassword => NGCP::Panel::Utils::Auth::generate_salted_hash($form->params->{password}), webpassword => $webpassword,
}); });
$rs->delete; $rs->delete;
}); });
@ -2941,9 +2943,10 @@ sub edit_master :Chained('master') :PathPart('edit') :Args(0) :Does(ACL) :ACLDet
my $prov_params = {}; my $prov_params = {};
$prov_params->{pbx_extension} = $form->params->{pbx_extension}; $prov_params->{pbx_extension} = $form->params->{pbx_extension};
$prov_params->{webusername} = $form->params->{webusername} || undef; $prov_params->{webusername} = $form->params->{webusername} || undef;
if($form->params->{webpassword}) { if ($form->params->{webpassword}) {
$prov_params->{webpassword} = $prov_params->{webpassword} = $form->params->{webpassword};
NGCP::Panel::Utils::Auth::generate_salted_hash($form->params->{webpassword}); $prov_params->{webpassword} = NGCP::Panel::Utils::Auth::generate_salted_hash($prov_params->{webpassword})
if $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS;
} }
$prov_params->{password} = $form->params->{password} $prov_params->{password} = $form->params->{password}
if($form->params->{password}); if($form->params->{password});
@ -3346,8 +3349,10 @@ sub webpass_edit :Chained('base') :PathPart('webpass/edit') :Args(0) {
my $subscriber = $c->stash->{subscriber}; my $subscriber = $c->stash->{subscriber};
my $prov_subscriber = $subscriber->provisioning_voip_subscriber; my $prov_subscriber = $subscriber->provisioning_voip_subscriber;
$schema->txn_do(sub { $schema->txn_do(sub {
my $webpassword = $form->values->{webpassword};
$webpassword = NGCP::Panel::Utils::Auth::generate_salted_hash($webpassword) if $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS;
$prov_subscriber->update({ $prov_subscriber->update({
webpassword => NGCP::Panel::Utils::Auth::generate_salted_hash($form->values->{webpassword}), webpassword => NGCP::Panel::Utils::Auth::generate_salted_hash($webpassword),
}); });
}); });
NGCP::Panel::Utils::Message::info( NGCP::Panel::Utils::Message::info(

4
lib/NGCP/Panel/Form/Subscriber.pm
Unescape View File

@ -202,11 +202,11 @@ sub update_fields {
my $c = $self->ctx; my $c = $self->ctx;
return unless $c; return unless $c;
if($c->config->{security}->{password_sip_autogenerate} && $self->field('password')) { if($c->config->{security}->{password_sip_autogenerate} and $self->field('password')) {
$self->field('password')->inactive(1); $self->field('password')->inactive(1);
$self->field('password')->required(0); $self->field('password')->required(0);
} }
if($c->config->{security}->{password_web_autogenerate} && $self->field('webpassword')) { if($c->config->{security}->{password_web_autogenerate} and $self->field('webpassword')) {
$self->field('webpassword')->inactive(1); $self->field('webpassword')->inactive(1);
$self->field('webpassword')->required(0); $self->field('webpassword')->required(0);
} }

55
lib/NGCP/Panel/Role/API/Subscribers.pm
Unescape View File

@ -55,18 +55,8 @@ sub resource_from_item {
delete $prov_resource->{domain_id}; delete $prov_resource->{domain_id};
delete $prov_resource->{account_id}; delete $prov_resource->{account_id};
my %resource = %{ merge($bill_resource, $prov_resource) }; my %resource = %{ merge($bill_resource, $prov_resource) };
my $change_passwords = 1;
$resource{administrative} = delete $resource{admin}; $resource{administrative} = delete $resource{admin};
if ($c->request->method eq 'PATCH') {
if ($resource{pre_patch_resource}) {
delete $resource{pre_patch_resource};
} else {
$change_passwords = 0;
$resource{pre_patch_resource} = 1;
}
}
unless($customer->product->class eq 'pbxaccount') { unless($customer->product->class eq 'pbxaccount') {
delete $resource{is_pbx_group}; delete $resource{is_pbx_group};
delete $resource{is_pbx_pilot}; delete $resource{is_pbx_pilot};
@ -86,20 +76,8 @@ sub resource_from_item {
$resource{email} = undef; $resource{email} = undef;
$resource{timezone} = undef; $resource{timezone} = undef;
} }
# if the webpassword length is 54 or 56 chars and it contains $, $resource{_password} = $resource{password};
# we assume that the password is encrypted, $resource{_webpassword} = $resource{webpassword};
# as we do not have an explicit flag for the password field
# wether it's encrypted or not, there is a chance that
# if somebody manages to create a 54 chars password containing
# '$', it will be detected as false positive, but
# - all webpasswords from mr8.5+ are meant to be encrypted
# - in case of the false positive result, the worse that happens
# the password is not returned to the user in plain-text
if ($change_passwords &&
$resource{webpassword} && (length $resource{webpassword}) =~ /^(54|56)$/ &&
$resource{webpassword} =~ /\$/) {
delete $resource{webpassword};
}
if(!$form){ if(!$form){
($form) = $self->get_form($c); ($form) = $self->get_form($c);
} }
@ -199,18 +177,20 @@ sub resource_from_item {
if ($c->user->show_passwords) { if ($c->user->show_passwords) {
foreach my $k(qw/password webpassword/) { foreach my $k(qw/password webpassword/) {
eval { eval {
if ($resource{$k} && $change_passwords) { if (not NGCP::Panel::Utils::Auth::is_salted_hash($resource{$k})) {
$resource{$k} = NGCP::Panel::Utils::Encryption::encrypt_rsa($c,$resource{$k}); $resource{'_' . $k} = NGCP::Panel::Utils::Encryption::encrypt_rsa($c,$resource{$k});
} else {
delete $resource{'_' . $k};
} }
}; };
if ($@) { if ($@) {
$c->log->error("Failed to encrypt $k: " . $@); $c->log->error("Failed to encrypt $k: " . $@);
delete $resource{$k} if $change_passwords; delete $resource{'_' . $k};
} }
} }
} else { } else {
foreach my $k(qw/password webpassword/) { foreach my $k(qw/password webpassword/) {
delete $resource{$k} if $change_passwords; delete $resource{'_' . $k};
} }
} }
} else { } else {
@ -221,18 +201,18 @@ sub resource_from_item {
} }
# TODO: make custom filtering configurable! # TODO: make custom filtering configurable!
foreach my $k(qw/password webpassword/) { foreach my $k (qw/password webpassword/) {
delete $resource{$k} if $change_passwords; delete $resource{'_' . $k};
} }
} }
if ($c->user->roles eq "subscriberadmin") { if ($c->user->roles eq "subscriberadmin") {
$resource{customer_id} = $contract_id; $resource{customer_id} = $contract_id;
if ($item->id != $c->user->voip_subscriber->id) { if ($item->id != $c->user->voip_subscriber->id) {
if (!$c->config->{security}->{password_sip_expose_subadmin}) { if (!$c->config->{security}->{password_sip_expose_subadmin}) {
delete $resource{password} if $change_passwords; delete $resource{_password};
} }
if (!$c->config->{security}->{password_web_expose_subadmin}) { if (!$c->config->{security}->{password_web_expose_subadmin}) {
delete $resource{webpassword} if $change_passwords; delete $resource{_webpassword};
} }
} }
} }
@ -253,6 +233,11 @@ sub hal_from_item {
$is_subadm = 0; $is_subadm = 0;
} }
delete $resource->{password};
delete $resource->{webpassword};
$resource->{password} = delete $resource->{_password} if exists $resource->{_password};
$resource->{webpassword} = delete $resource->{_webpassword} if exists $resource->{_webpassword};
my $hal = Data::HAL->new( my $hal = Data::HAL->new(
links => [ links => [
Data::HAL::Link->new( Data::HAL::Link->new(
@ -525,14 +510,12 @@ sub update_item {
contact_id => $resource->{contact_id}, contact_id => $resource->{contact_id},
}; };
if(defined $resource->{webpassword}) { if (exists $resource->{webpassword} and $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS) {
$resource->{webpassword} = NGCP::Panel::Utils::Auth::generate_salted_hash($resource->{webpassword}); $resource->{webpassword} = NGCP::Panel::Utils::Auth::generate_salted_hash($resource->{webpassword});
} }
my $provisioning_res = { my $provisioning_res = {
password => $resource->{password},
webusername => $resource->{webusername}, webusername => $resource->{webusername},
webpassword => $resource->{webpassword},
is_pbx_pilot => $resource->{is_pbx_pilot} // 0, is_pbx_pilot => $resource->{is_pbx_pilot} // 0,
is_pbx_group => $resource->{is_pbx_group} // 0, is_pbx_group => $resource->{is_pbx_group} // 0,
modify_timestamp => NGCP::Panel::Utils::DateTime::current_local, modify_timestamp => NGCP::Panel::Utils::DateTime::current_local,
@ -541,6 +524,8 @@ sub update_item {
pbx_extension => $resource->{pbx_extension}, pbx_extension => $resource->{pbx_extension},
$resource->{administrative} ? (admin => $resource->{administrative}) : (), $resource->{administrative} ? (admin => $resource->{administrative}) : (),
}; };
$provisioning_res->{password} = $resource->{password} if exists $resource->{password};
$provisioning_res->{webpassword} = $resource->{webpassword} if exists $resource->{webpassword};
if(is_true($resource->{is_pbx_group})) { if(is_true($resource->{is_pbx_group})) {
$provisioning_res->{pbx_hunt_policy} = $resource->{pbx_hunt_policy}; $provisioning_res->{pbx_hunt_policy} = $resource->{pbx_hunt_policy};
$provisioning_res->{pbx_hunt_timeout} = $resource->{pbx_hunt_timeout}; $provisioning_res->{pbx_hunt_timeout} = $resource->{pbx_hunt_timeout};

15
lib/NGCP/Panel/Utils/Auth.pm
Unescape View File

@ -10,6 +10,7 @@ use UUID;
use NGCP::Panel::Utils::Redis; use NGCP::Panel::Utils::Redis;
our $SALT_LENGTH = 128; our $SALT_LENGTH = 128;
our $ENCRYPT_SUBSCRIBER_WEBPASSWORDS = 1;
sub check_password { sub check_password {
my $pass = shift // return; my $pass = shift // return;
@ -114,6 +115,18 @@ sub perform_auth {
return $res; return $res;
} }
sub is_salted_hash {
my $password = shift;
if (length($password)
and (length($password) == 54 or length($password) == 56)
and $password =~ /\$/) {
return 1;
}
return 0;
}
sub perform_subscriber_auth { sub perform_subscriber_auth {
my ($c, $user, $domain, $pass) = @_; my ($c, $user, $domain, $pass) = @_;
my $res; my $res;
@ -132,7 +145,7 @@ sub perform_subscriber_auth {
}); });
my $sub = $authrs->first; my $sub = $authrs->first;
if(defined $sub && $sub->webpassword) { if(defined $sub && defined $sub->webpassword) {
my $sub_pass = $sub->webpassword; my $sub_pass = $sub->webpassword;
if (length $sub_pass > 40) { if (length $sub_pass > 40) {
my @splitted_pass = split /\$/, $sub_pass; my @splitted_pass = split /\$/, $sub_pass;

41
lib/NGCP/Panel/Utils/Subscriber.pm
Unescape View File

@ -319,9 +319,25 @@ sub prepare_resource {
} }
} }
if (exists $resource->{webpassword}
and NGCP::Panel::Utils::Auth::is_salted_hash($resource->{webpassword})) {
delete $resource->{webpassword};
}
if (exists $resource->{webpassword} and defined $resource->{webpassword}
and $item and defined $item->provisioning_voip_subscriber->webpassword
and $resource->{webpassword} eq $item->provisioning_voip_subscriber->webpassword) {
delete $resource->{webpassword};
}
if (exists $resource->{password} and defined $resource->{password}
and $item and defined $item->provisioning_voip_subscriber->password
and $resource->{password} eq $item->provisioning_voip_subscriber->password) {
delete $resource->{password};
}
foreach my $k(qw/password webpassword/) { foreach my $k(qw/password webpassword/) {
eval { eval {
if ($resource->{$k}) { if (exists $resource->{$k}) {
$resource->{$k} = NGCP::Panel::Utils::Encryption::decrypt_rsa($c,$resource->{$k}); $resource->{$k} = NGCP::Panel::Utils::Encryption::decrypt_rsa($c,$resource->{$k});
} }
}; };
@ -332,12 +348,6 @@ sub prepare_resource {
} }
} }
my $webpassword;
if (length($resource->{webpassword}) and $item and length($item->provisioning_voip_subscriber->webpassword)
and $resource->{webpassword} eq $item->provisioning_voip_subscriber->webpassword) {
$webpassword = delete $resource->{webpassword};
}
#password is mandatory field, so it cannot be absent from resource, the only reason for that being it was #password is mandatory field, so it cannot be absent from resource, the only reason for that being it was
#deleted in resource_from_item for admins without show_passwords flag; in this case, we are restoring it here #deleted in resource_from_item for admins without show_passwords flag; in this case, we are restoring it here
if (not length($resource->{password}) and $item and length($item->provisioning_voip_subscriber->password)) { if (not length($resource->{password}) and $item and length($item->provisioning_voip_subscriber->password)) {
@ -345,14 +355,6 @@ sub prepare_resource {
} }
return unless &$validate_code($resource); return unless &$validate_code($resource);
$resource->{webpassword} = $webpassword if ($webpassword);
#my ($form) = $self->get_form($c);
#return unless $self->validate_form(
# c => $c,
# resource => $resource,
# form => $form,
#);
# this format is expected by NGCP::Panel::Utils::Subscriber::create_subscriber # this format is expected by NGCP::Panel::Utils::Subscriber::create_subscriber
$resource->{alias_numbers} = [ map {{ e164 => $_ }} @{ $resource->{alias_numbers} // [] } ]; $resource->{alias_numbers} = [ map {{ e164 => $_ }} @{ $resource->{alias_numbers} // [] } ];
@ -413,7 +415,6 @@ sub prepare_resource {
} }
} }
my $preferences = {}; my $preferences = {};
my $admin = 0; my $admin = 0;
unless($customer->product->class eq 'pbxaccount') { unless($customer->product->class eq 'pbxaccount') {
@ -633,7 +634,7 @@ sub create_subscriber {
} }
my $passlen = $c->config->{security}->{password_min_length} || 8; my $passlen = $c->config->{security}->{password_min_length} || 8;
if($c->config->{security}->{password_sip_autogenerate} && !$params->{password}) { if($c->config->{security}->{password_sip_autogenerate} and not defined $params->{password}) {
$params->{password} = String::MkPasswd::mkpasswd( $params->{password} = String::MkPasswd::mkpasswd(
-length => $passlen, -length => $passlen,
-minnum => 1, -minlower => 1, -minupper => 1, -minspecial => 1, -minnum => 1, -minlower => 1, -minupper => 1, -minspecial => 1,
@ -642,7 +643,7 @@ sub create_subscriber {
#otherwise it breaks xml device configs #otherwise it breaks xml device configs
$params->{password} =~s/[<>&]/,/g; $params->{password} =~s/[<>&]/,/g;
} }
if($c->config->{security}->{password_web_autogenerate} && !$params->{webpassword}) { if($c->config->{security}->{password_web_autogenerate} and not defined $params->{webpassword}) {
$params->{webpassword} = String::MkPasswd::mkpasswd( $params->{webpassword} = String::MkPasswd::mkpasswd(
-length => $passlen, -length => $passlen,
-minnum => 1, -minlower => 1, -minupper => 1, -minspecial => 1, -minnum => 1, -minlower => 1, -minupper => 1, -minspecial => 1,
@ -692,7 +693,7 @@ sub create_subscriber {
UUID::unparse($pass_bin, $pass_str); UUID::unparse($pass_bin, $pass_str);
$params->{password} = $pass_str; $params->{password} = $pass_str;
} }
if(defined $params->{webpassword}) { if (exists $params->{webpassword} and $NGCP::Panel::Utils::Auth::ENCRYPT_SUBSCRIBER_WEBPASSWORDS) {
$params->{webpassword} = NGCP::Panel::Utils::Auth::generate_salted_hash($params->{webpassword}); $params->{webpassword} = NGCP::Panel::Utils::Auth::generate_salted_hash($params->{webpassword});
} }
my $prov_subscriber = $schema->resultset('provisioning_voip_subscribers')->create({ my $prov_subscriber = $schema->resultset('provisioning_voip_subscribers')->create({
@ -700,7 +701,7 @@ sub create_subscriber {
username => $params->{username}, username => $params->{username},
password => $params->{password}, password => $params->{password},
webusername => $params->{webusername} || $params->{username}, webusername => $params->{webusername} || $params->{username},
webpassword => $params->{webpassword}, (exists $params->{webpassword} ? (webpassword => $params->{webpassword}) : ()),
admin => $params->{administrative} // $administrative, admin => $params->{administrative} // $administrative,
account_id => $contract->id, account_id => $contract->id,
domain_id => $prov_domain->id, domain_id => $prov_domain->id,

Write Preview
Loading…
Cancel
Save