From 043c7523acd6af5288191b15f746fc360b73ab40 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 23 Sep 2015 11:54:41 +0800 Subject: Bug 1199087 - extend 2fa protection beyond login --- Bugzilla/Auth.pm | 38 +++++++++++++++++++++++++++++++------- Bugzilla/Constants.pm | 3 +++ Bugzilla/MFA.pm | 39 +++++++++++++++++++++++++++++++++++++-- Bugzilla/MFA/TOTP.pm | 27 ++++++++++----------------- Bugzilla/Token.pm | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 127 insertions(+), 27 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index a4f2dd9a9..b39bb827b 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -39,6 +39,8 @@ use Bugzilla::Auth::Login::Stack; use Bugzilla::Auth::Verify::Stack; use Bugzilla::Auth::Persist::Cookie; use Socket; +use URI; +use URI::QueryParam; sub new { my ($class, $params) = @_; @@ -93,26 +95,48 @@ sub login { } $user->set_authorizer($self); - # trigger multi-factor auth. once verified the provider calls mfa_verified() + # trigger multi-factor auth if ($self->{_info_getter}->{successful}->requires_verification && $user->mfa && !Bugzilla->sudoer && !i_am_webservice() ) { - $user->mfa_provider->prompt({ user => $user, type => $type }); - exit; + my $params = Bugzilla->input_params; + my $cgi = Bugzilla->cgi; + my $uri = URI->new($cgi->self_url); + foreach my $param (qw( Bugzilla_remember Bugzilla_restrictlogin GoAheadAndLogIn )) { + $uri->query_param_delete($param); + } + $user->mfa_provider->verify_prompt({ + user => $user, + type => $type, + reason => 'Logging in as ' . $user->identity, + restrictlogin => $params->{Bugzilla_restrictlogin}, + remember => $params->{Bugzilla_remember}, + url => $uri->as_string, + postback => { + action => 'token.cgi', + token_field => 't', + fields => { + a => 'mfa_l', + }, + } + }); } return $self->_handle_login_result($login_info, $type); } sub mfa_verified { - my ($self, $user, $type) = @_; + my ($self, $user, $event) = @_; require Bugzilla::Auth::Login::CGI; + + my $params = Bugzilla->input_params; $self->{_info_getter}->{successful} = Bugzilla::Auth::Login::CGI->new(); - $self->_handle_login_result({ user => $user }, $type); - print Bugzilla->cgi->redirect('index.cgi'); - exit; + $params->{Bugzilla_restrictlogin} = $event->{restrictlogin}; + $params->{Bugzilla_remember} = $event->{remember}; + + $self->_handle_login_result({ user => $user }, $event->{type}); } sub successful_info_getter { diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 59796a076..2fd6a23b1 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -173,6 +173,7 @@ use Memoize; ON_ACTIVESTATE MAX_TOKEN_AGE + MAX_SHORT_TOKEN_HOURS MAX_LOGINCOOKIE_AGE MAX_SUDO_TOKEN_AGE MAX_LOGIN_ATTEMPTS @@ -469,6 +470,8 @@ use constant TIMETRACKING_FIELDS => # The maximum number of days a token will remain valid. use constant MAX_TOKEN_AGE => 3; +# The maximum number of hours a short-lived token will remain valid. +use constant MAX_SHORT_TOKEN_HOURS => 1; # How many days a logincookie will remain valid if not used. use constant MAX_LOGINCOOKIE_AGE => 7; # How many seconds (default is 6 hours) a sudo cookie remains valid. diff --git a/Bugzilla/MFA.pm b/Bugzilla/MFA.pm index 5db38e55e..21f42fbfb 100644 --- a/Bugzilla/MFA.pm +++ b/Bugzilla/MFA.pm @@ -8,6 +8,8 @@ package Bugzilla::MFA; use strict; +use Bugzilla::Token qw( issue_short_lived_session_token set_token_extra_data get_token_extra_data delete_token ); + sub new { my ($class, $user) = @_; return bless({ user => $user }, $class); @@ -27,9 +29,42 @@ sub prompt {} # throws errors if code is invalid sub check {} -# during-login verification -sub check_login {} +# verification + +sub verify_prompt { + my ($self, $event) = @_; + my $user = delete $event->{user} // Bugzilla->user; + + # generate token and attach mfa data + my $token = issue_short_lived_session_token('mfa', $user); + set_token_extra_data($token, $event); + + # trigger provider verification + my $token_field = $event->{postback}->{token_field} // 'mfa_token'; + $event->{postback}->{fields}->{$token_field} = $token; + $self->prompt($event); + exit; +} +sub verify_check { + my ($self, $token) = @_; + + # check token + my ($user_id) = Bugzilla::Token::GetTokenData($token); + my $user = Bugzilla::User->check({ id => $user_id, cache => 1 }); + + # mfa verification + $self->check(Bugzilla->input_params); + + # return event data + my $event = get_token_extra_data($token); + delete_token($token); + if (!$event) { + print Bugzilla->cgi->redirect('index.cgi'); + exit; + } + return $event; +} # helpers diff --git a/Bugzilla/MFA/TOTP.pm b/Bugzilla/MFA/TOTP.pm index 05e4e4e3b..859ca4b8d 100644 --- a/Bugzilla/MFA/TOTP.pm +++ b/Bugzilla/MFA/TOTP.pm @@ -48,32 +48,25 @@ sub enrolled { } sub prompt { - my ($self, $params) = @_; + my ($self, $vars) = @_; my $template = Bugzilla->template; - my $vars = { - user => $params->{user}, - token => scalar issue_session_token('mfa', $params->{user}), - type => $params->{type}, - }; - print Bugzilla->cgi->header(); $template->process('mfa/totp/verify.html.tmpl', $vars) || ThrowTemplateError($template->error()); } sub check { - my ($self, $code) = @_; - $self->_auth()->verify($code, 1) - || ThrowUserError('mfa_totp_bad_code'); -} - -sub check_login { - my ($self, $user) = @_; - my $cgi = Bugzilla->cgi; + my ($self, $params) = @_; + my $code = $params->{code} // ''; + return if $self->_auth()->verify($code, 1); - $self->check($cgi->param('code') // ''); - $user->authorizer->mfa_verified($user, $cgi->param('type')); + if (exists $params->{mfa_action}) { + ThrowUserError('mfa_totp_bad_enrolment_code'); + } + else { + ThrowUserError('mfa_totp_bad_code'); + } } 1; diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm index 6e1414db6..3c5261821 100644 --- a/Bugzilla/Token.pm +++ b/Bugzilla/Token.pm @@ -41,13 +41,17 @@ use Date::Parse; use File::Basename; use Digest::MD5 qw(md5_hex); use Digest::SHA qw(hmac_sha256_base64); +use Encode; +use JSON qw(encode_json decode_json); use base qw(Exporter); @Bugzilla::Token::EXPORT = qw(issue_api_token issue_session_token + issue_short_lived_session_token issue_auth_delegation_token check_auth_delegation_token check_token_data delete_token - issue_hash_token check_hash_token); + issue_hash_token check_hash_token + set_token_extra_data get_token_extra_data); # 128 bits password: # 128 * log10(2) / log10(62) = 21.49, round up to 22. @@ -227,6 +231,15 @@ sub issue_session_token { return _create_token($user->id, 'session', $data); } +sub issue_short_lived_session_token { + my ($data, $user) = @_; + # Generates a random token, adds it to the tokens table, and returns + # the token to the caller. + + $user //= Bugzilla->user; + return _create_token($user->id, 'session.short', $data); +} + sub issue_hash_token { my ($data, $time) = @_; $data ||= []; @@ -290,6 +303,9 @@ sub CleanTokenTable { $dbh->do("DELETE FROM tokens WHERE " . $dbh->sql_date_math('issuedate', '+', '?', 'HOUR') . " <= NOW()", undef, MAX_TOKEN_AGE * 24); + $dbh->do("DELETE FROM tokens WHERE tokentype = ? AND " . + $dbh->sql_date_math('issuedate', '+', '?', 'HOUR') . " <= NOW()", + undef, 'session.short', MAX_SHORT_TOKEN_HOURS); } sub GenerateUniqueToken { @@ -475,6 +491,35 @@ sub check_token_data { return 1; } +sub set_token_extra_data { + my ($token, $data) = @_; + + $data = encode_json($data) if ref($data); + + # extra_data is MEDIUMTEXT, max 16M + if (length($data) > 16_777_215) { + ThrowCodeError('token_data_too_big'); + } + + Bugzilla->dbh->do( + "INSERT INTO token_data (token, extra_data) VALUES (?, ?) ON DUPLICATE KEY UPDATE extra_data = ?", + undef, $token, $data, $data); +} + +sub get_token_extra_data { + my ($token) = @_; + trick_taint($token); + my ($data) = Bugzilla->dbh->selectrow_array( + "SELECT extra_data FROM token_data WHERE token = ?", + undef, $token); + return undef unless defined $data; + $data = encode('UTF-8', $data); + eval { + $data = decode_json($data); + }; + return $data; +} + ################################################################################ # Internal Functions ################################################################################ -- cgit v1.2.3-24-g4f1b