From: dkl%redhat.com <> Date: Thu, 10 Jul 2008 09:51:32 +0000 (+0000) Subject: Bug 428659 – Setting SSL param to 'authenticated sessions' only protects logins... X-Git-Tag: bugzilla-3.2rc1~32 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5531e2c22d40aa21e4ff170544a289d21c59ffcd;p=thirdparty%2Fbugzilla.git Bug 428659 – Setting SSL param to 'authenticated sessions' only protects logins and param doesn't protect WebService calls at all Patch by Dave Lawrence - r/a=mkanat --- diff --git a/Bugzilla/Auth/Login/CGI.pm b/Bugzilla/Auth/Login/CGI.pm index 980e271239..0bc3ee1199 100644 --- a/Bugzilla/Auth/Login/CGI.pm +++ b/Bugzilla/Auth/Login/CGI.pm @@ -66,11 +66,9 @@ sub fail_nodata { } # Redirect to SSL if required - if (Bugzilla->params->{'sslbase'} ne '' - and Bugzilla->params->{'ssl'} ne 'never') - { - $cgi->require_https(Bugzilla->params->{'sslbase'}); - } + Bugzilla->cgi->require_https(Bugzilla->params->{'sslbase'}) + if ssl_require_redirect(); + 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 aeb8419ca7..4c62ab3ac7 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -72,9 +72,8 @@ sub new { $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()) + if (i_am_cgi() && Bugzilla->usage_mode != USAGE_MODE_WEBSERVICE + && ssl_require_redirect()) { $self->require_https(Bugzilla->params->{'sslbase'}); } @@ -297,18 +296,19 @@ 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 + my $query = Bugzilla->usage_mode == USAGE_MODE_WEBSERVICE ? 0 : 1; + # XMLRPC clients (SOAP::Lite at least) requires 301 to redirect properly + 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). "\n"; + exit; } 1; @@ -378,7 +378,7 @@ As its only argument, it takes the name of the cookie to expire. This routine checks if the current page is being served over https, and redirects to the https protocol if required, retaining QUERY_STRING. -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 e7a76e21d1..8e521c24ae 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,26 @@ sub i_am_cgi { return exists $ENV{'SERVER_SOFTWARE'} ? 1 : 0; } +sub ssl_require_redirect { + my $method = shift; + + # Redirect to SSL if required. + if (!(uc($ENV{HTTPS}) eq 'ON' || $ENV{'SERVER_PORT'} == 443) + && Bugzilla->params->{'sslbase'} ne '') + { + if (Bugzilla->params->{'ssl'} eq 'always' + || (Bugzilla->params->{'ssl'} eq 'authenticated sessions' + && Bugzilla->user->id) + || (Bugzilla->params->{'ssl'} eq 'authenticated sessions' + && !Bugzilla->user->id && $method eq 'User.login')) + { + return 1; + } + } + + 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 94dbb62177..8e7d1c21fc 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; sub fail_unimplemented { @@ -53,6 +54,15 @@ sub handle_login { return; } +sub handle_redirect { + my ($action, $uri, $method) = @_; + my $full_method = $uri . "." . $method; + + # Redirect to SSL if required. + Bugzilla->cgi->require_https(Bugzilla->params->{'sslbase'}) + if ssl_require_redirect($full_method); +} + # For some methods, we shouldn't call Bugzilla->login before we call them use constant LOGIN_EXEMPT => { }; diff --git a/index.cgi b/index.cgi index 100941765d..4426171112 100755 --- a/index.cgi +++ b/index.cgi @@ -35,6 +35,7 @@ use Bugzilla; use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Update; +use Bugzilla::Util; # Check whether or not the user is logged in my $user = Bugzilla->login(LOGIN_OPTIONAL); @@ -46,9 +47,8 @@ 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') { - $cgi->require_https(Bugzilla->params->{'sslbase'}); -} +$cgi->require_https(Bugzilla->params->{'sslbase'}) + if ssl_require_redirect(); my $template = Bugzilla->template; my $vars = {}; diff --git a/token.cgi b/token.cgi index c91c2f94ff..71996bec00 100755 --- a/token.cgi +++ b/token.cgi @@ -347,11 +347,9 @@ sub request_create_account { $vars->{'date'} = str2time($date); # We require a HTTPS connection if possible. - if (Bugzilla->params->{'sslbase'} ne '' - && Bugzilla->params->{'ssl'} ne 'never') - { - $cgi->require_https(Bugzilla->params->{'sslbase'}); - } + Bugzilla->cgi->require_https(Bugzilla->params->{'sslbase'}) + if ssl_require_redirect(); + print $cgi->header(); $template->process('account/email/confirm-new.html.tmpl', $vars) diff --git a/xmlrpc.cgi b/xmlrpc.cgi index 324382ea28..5ca40bef09 100755 --- a/xmlrpc.cgi +++ b/xmlrpc.cgi @@ -53,5 +53,9 @@ my $dispatch = { my $response = Bugzilla::WebService::XMLRPC::Transport::HTTP::CGI ->dispatch_with($dispatch) - ->on_action(sub { Bugzilla::WebService::handle_login($dispatch, @_) } ) + ->on_action(sub { + my ($action, $uri, $method) = @_; + Bugzilla::WebService::handle_login($dispatch, @_); + Bugzilla::WebService::handle_redirect(@_); + } ) ->handle;