diff options
| -rw-r--r-- | Bugzilla.pm | 8 | ||||
| -rw-r--r-- | Bugzilla/Auth/Login/CGI.pm | 11 | ||||
| -rw-r--r-- | Bugzilla/CGI.pm | 41 | ||||
| -rw-r--r-- | Bugzilla/Util.pm | 42 | ||||
| -rwxr-xr-x | Bugzilla/WebService.pm | 17 | ||||
| -rwxr-xr-x | index.cgi | 4 | ||||
| -rwxr-xr-x | token.cgi | 5 |
7 files changed, 98 insertions, 30 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm index a20aa0f6b..abba18924 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -270,6 +270,14 @@ sub login { else { $class->set_user($authenticated_user); } + + # We run after the login has completed since + # some of the checks in ssl_require_redirect + # look for Bugzilla->user->id to determine + # if redirection is required. + if (i_am_cgi() && ssl_require_redirect()) { + $class->cgi->require_https($class->params->{'sslbase'}); + } return $class->user; } diff --git a/Bugzilla/Auth/Login/CGI.pm b/Bugzilla/Auth/Login/CGI.pm index 980e27123..9e008be82 100644 --- a/Bugzilla/Auth/Login/CGI.pm +++ b/Bugzilla/Auth/Login/CGI.pm @@ -65,12 +65,17 @@ sub fail_nodata { ->faultstring('Login Required'); } - # Redirect to SSL if required - if (Bugzilla->params->{'sslbase'} ne '' - and Bugzilla->params->{'ssl'} ne 'never') + # If system is not configured to never require SSL connections + # we want to always redirect to SSL since passing usernames and + # passwords over an unprotected connection is a bad idea. If we + # get here then a login form will be provided to the user so we + # want this to be protected if possible. + if ($cgi->protocol ne 'https' && Bugzilla->params->{'sslbase'} ne '' + && Bugzilla->params->{'ssl'} ne 'never') { $cgi->require_https(Bugzilla->params->{'sslbase'}); } + print $cgi->header(); $template->process("account/auth/login.html.tmpl", { 'target' => $cgi->url(-relative=>1) }) diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index aeb8419ca..e6238f334 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -71,14 +71,6 @@ sub new { # Send appropriate charset $self->charset(Bugzilla->params->{'utf8'} ? 'UTF-8' : ''); - # Redirect to SSL if required - if (Bugzilla->params->{'sslbase'} ne '' - && Bugzilla->params->{'ssl'} eq 'always' - && i_am_cgi()) - { - $self->require_https(Bugzilla->params->{'sslbase'}); - } - # Check for errors # All of the Bugzilla code wants to do this, so do it here instead of # in each script @@ -297,18 +289,23 @@ sub remove_cookie { # Redirect to https if required sub require_https { - my $self = shift; - if ($self->protocol ne 'https') { - my $url = shift; - if (defined $url) { - $url .= $self->url('-path_info' => 1, '-query' => 1, '-relative' => 1); - } else { - $url = $self->self_url; - $url =~ s/^http:/https:/i; - } - print $self->redirect(-location => $url); - exit; + my ($self, $url) = @_; + # Do not create query string if data submitted via XMLRPC + # since we want the data to be resubmitted over POST method. + my $query = Bugzilla->usage_mode == USAGE_MODE_WEBSERVICE ? 0 : 1; + # XMLRPC clients (SOAP::Lite at least) requires 301 to redirect properly + # and do not work with 302. + my $status = Bugzilla->usage_mode == USAGE_MODE_WEBSERVICE ? 301 : 302; + if (defined $url) { + $url .= $self->url('-path_info' => 1, '-query' => $query, '-relative' => 1); + } else { + $url = $self->self_url; + $url =~ s/^http:/https:/i; } + print $self->redirect(-location => $url, -status => $status); + # When using XML-RPC with mod_perl, we need the headers sent immediately. + $self->r->rflush if $ENV{MOD_PERL}; + exit; } 1; @@ -375,10 +372,10 @@ As its only argument, it takes the name of the cookie to expire. =item C<require_https($baseurl)> -This routine checks if the current page is being served over https, and -redirects to the https protocol if required, retaining QUERY_STRING. +This routine redirects the client to a different location using the https protocol. +If the client is using XMLRPC, it will not retain the QUERY_STRING since XMLRPC uses POST. -It takes an option argument which will be used as the base URL. If $baseurl +It takes an optional argument which will be used as the base URL. If $baseurl is not provided, the current URL is used. =back diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index e7a76e21d..1e7dbf8d1 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -36,7 +36,7 @@ use base qw(Exporter); html_quote url_quote xml_quote css_class_quote html_light_quote url_decode i_am_cgi get_netaddr correct_urlbase - lsearch + lsearch ssl_require_redirect diff_arrays diff_strings trim wrap_hard wrap_comment find_wrap_point format_time format_time_decimal validate_date @@ -218,6 +218,46 @@ sub i_am_cgi { return exists $ENV{'SERVER_SOFTWARE'} ? 1 : 0; } +sub ssl_require_redirect { + my $method = shift; + + # If currently not in a protected SSL + # connection, determine if a redirection is + # needed based on value in Bugzilla->params->{ssl}. + # If we are already in a protected connection or + # sslbase is not set then no action is required. + if (uc($ENV{'HTTPS'}) ne 'ON' + && $ENV{'SERVER_PORT'} != 443 + && Bugzilla->params->{'sslbase'} ne '') + { + # System is configured to never require SSL + # so no redirection is needed. + return 0 + if Bugzilla->params->{'ssl'} eq 'never'; + + # System is configured to always require a SSL + # connection so we need to redirect. + return 1 + if Bugzilla->params->{'ssl'} eq 'always'; + + # System is configured such that if we are inside + # of an authenticated session, then we need to make + # sure that all of the connections are over SSL. Non + # authenticated sessions SSL is not mandatory. + # For XMLRPC requests, if the method is User.login + # then we always want the connection to be over SSL + # if the system is configured for authenticated + # sessions since the user's username and password + # will be passed before the user is logged in. + return 1 + if Bugzilla->params->{'ssl'} eq 'authenticated sessions' + && (Bugzilla->user->id + || (defined $method && $method eq 'User.login')); + } + + return 0; +} + sub correct_urlbase { my $ssl = Bugzilla->params->{'ssl'}; return Bugzilla->params->{'urlbase'} if $ssl eq 'never'; diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index 7812a237b..d1502468d 100755 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -19,6 +19,7 @@ package Bugzilla::WebService; use strict; use Bugzilla::WebService::Constants; +use Bugzilla::Util; use Date::Parse; use XMLRPC::Lite; @@ -49,7 +50,21 @@ sub handle_login { eval "require $class"; return if $class->login_exempt($method); - Bugzilla->login; + Bugzilla->login(); + + # Even though we check for the need to redirect in + # Bugzilla->login() we check here again since Bugzilla->login() + # does not know what the current XMLRPC method is. Therefore + # ssl_require_redirect in Bugzilla->login() will have returned + # false if system was configured to redirect for authenticated + # sessions and the user was not yet logged in. + # So here we pass in the method name to ssl_require_redirect so + # it can then check for the extra case where the method equals + # User.login, which we would then need to redirect if not + # over a secure connection. + my $full_method = $uri . "." . $method; + Bugzilla->cgi->require_https(Bugzilla->params->{'sslbase'}) + if ssl_require_redirect($full_method); return; } @@ -46,7 +46,9 @@ my $user = Bugzilla->login(LOGIN_OPTIONAL); my $cgi = Bugzilla->cgi; # Force to use HTTPS unless Bugzilla->params->{'ssl'} equals 'never'. # This is required because the user may want to log in from here. -if (Bugzilla->params->{'sslbase'} ne '' and Bugzilla->params->{'ssl'} ne 'never') { +if ($cgi->protocol ne 'https' && Bugzilla->params->{'sslbase'} ne '' + && Bugzilla->params->{'ssl'} ne 'never') +{ $cgi->require_https(Bugzilla->params->{'sslbase'}); } @@ -346,8 +346,9 @@ sub request_create_account { $vars->{'email'} = $login_name . Bugzilla->params->{'emailsuffix'}; $vars->{'date'} = str2time($date); - # We require a HTTPS connection if possible. - if (Bugzilla->params->{'sslbase'} ne '' + # When 'ssl' equals 'always' or 'authenticated sessions', + # we want this form to always be over SSL. + if ($cgi->protocol ne 'https' && Bugzilla->params->{'sslbase'} ne '' && Bugzilla->params->{'ssl'} ne 'never') { $cgi->require_https(Bugzilla->params->{'sslbase'}); |