summaryrefslogtreecommitdiffstats
path: root/Bugzilla
diff options
context:
space:
mode:
authordkl%redhat.com <>2008-08-18 11:16:12 +0200
committerdkl%redhat.com <>2008-08-18 11:16:12 +0200
commit20d885c77680fc082640c0a7340be44cd02b2779 (patch)
treea7b20520a3f1e6648ed9dbb5bc72321007bace84 /Bugzilla
parentb3e936bf2bbc1fb1ec55732703650d9f78dfd5f0 (diff)
downloadbugzilla-20d885c77680fc082640c0a7340be44cd02b2779.tar.gz
bugzilla-20d885c77680fc082640c0a7340be44cd02b2779.tar.xz
Bug 428659 – Setting SSL param to 'authenticated sessions' only protects logins and param
doesn't protect WebService calls at all Patch by David Lawrence <dkl@redhat.com> - r/a=LpSolit/mkanat
Diffstat (limited to 'Bugzilla')
-rw-r--r--Bugzilla/Auth/Login/CGI.pm11
-rw-r--r--Bugzilla/CGI.pm41
-rw-r--r--Bugzilla/Util.pm42
-rwxr-xr-xBugzilla/WebService.pm17
4 files changed, 84 insertions, 27 deletions
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;
}