diff options
| -rw-r--r-- | Bugzilla.pm | 24 | ||||
| -rw-r--r-- | Bugzilla/Auth/Login/CGI.pm | 11 | ||||
| -rw-r--r-- | Bugzilla/Auth/Persist/Cookie.pm | 8 | ||||
| -rw-r--r-- | Bugzilla/CGI.pm | 43 | ||||
| -rw-r--r-- | Bugzilla/Config.pm | 5 | ||||
| -rw-r--r-- | Bugzilla/Config/Core.pm | 7 | ||||
| -rw-r--r-- | Bugzilla/Mailer.pm | 5 | ||||
| -rw-r--r-- | Bugzilla/Util.pm | 68 | ||||
| -rw-r--r-- | Bugzilla/WebService/Server.pm | 14 | ||||
| -rw-r--r-- | docs/en/xml/administration.xml | 8 | ||||
| -rwxr-xr-x | index.cgi | 8 | ||||
| -rw-r--r-- | template/en/default/account/auth/login-small.html.tmpl | 7 | ||||
| -rw-r--r-- | template/en/default/admin/params/core.html.tmpl | 8 | ||||
| -rw-r--r-- | template/en/default/bug/edit.html.tmpl | 3 | ||||
| -rwxr-xr-x | token.cgi | 8 |
15 files changed, 74 insertions, 153 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm index 43a9b39ae..62b1af659 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -85,7 +85,6 @@ use constant SHUTDOWNHTML_EXIT_SILENTLY => [ sub init_page { (binmode STDOUT, ':utf8') if Bugzilla->params->{'utf8'}; - if (${^TAINT}) { # Some environment variables are not taint safe delete @::ENV{'PATH', 'IFS', 'CDPATH', 'ENV', 'BASH_ENV'}; @@ -94,6 +93,12 @@ sub init_page { $ENV{'PATH'} = ''; } + # Because this function is run live from perl "use" commands of + # other scripts, we're skipping the rest of this function if we get here + # during a perl syntax check (perl -c, like we do during the + # 001compile.t test). + return if $^C; + # IIS prints out warnings to the webpage, so ignore them, or log them # to a file if the file exists. if ($ENV{SERVER_SOFTWARE} && $ENV{SERVER_SOFTWARE} =~ /microsoft-iis/i) { @@ -108,18 +113,15 @@ sub init_page { }; } + do_ssl_redirect_if_required(); + # If Bugzilla is shut down, do not allow anything to run, just display a # message to the user about the downtime and log out. Scripts listed in # SHUTDOWNHTML_EXEMPT are exempt from this message. # - # Because this is code which is run live from perl "use" commands of other - # scripts, we're skipping this part if we get here during a perl syntax - # check -- runtests.pl compiles scripts without running them, so we - # need to make sure that this check doesn't apply to 'perl -c' calls. - # # This code must go here. It cannot go anywhere in Bugzilla::CGI, because # it uses Template, and that causes various dependency loops. - if (!$^C && Bugzilla->params->{"shutdownhtml"} + if (Bugzilla->params->{"shutdownhtml"} && lsearch(SHUTDOWNHTML_EXEMPT, basename($0)) == -1) { # Allow non-cgi scripts to exit silently (without displaying any @@ -318,14 +320,6 @@ sub login { $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 5be98aa7a..a93bc3d3a 100644 --- a/Bugzilla/Auth/Login/CGI.pm +++ b/Bugzilla/Auth/Login/CGI.pm @@ -65,17 +65,6 @@ sub fail_nodata { ->faultstring('Login Required'); } - # 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/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index c533252d3..60f90925e 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -89,11 +89,9 @@ sub persist_login { # Not a session cookie, so set an infinite expiry $cookieargs{'-expires'} = 'Fri, 01-Jan-2038 00:00:00 GMT'; } - if (Bugzilla->params->{'ssl'} ne 'never' - && Bugzilla->params->{'sslbase'} ne '') - { - # Bugzilla->login will automatically redirect to https://, - # so it's safe to turn on the 'secure' bit. + if (Bugzilla->params->{'ssl_redirect'}) { + # Make these cookies only be sent to us by the browser during + # HTTPS sessions, if we're using SSL. $cookieargs{'-secure'} = 1; } diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index a00d632c3..c30e13618 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -368,22 +368,23 @@ sub remove_cookie { '-value' => 'X'); } -# Redirect to https if required -sub require_https { - 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_XMLRPC ? 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_XMLRPC ? 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); +sub redirect_to_https { + my $self = shift; + my $sslbase = Bugzilla->params->{'sslbase'}; + # If this is a POST, we don't want ?POSTDATA in the query string. + # We expect the client to re-POST, which may be a violation of + # the HTTP spec, but the only time we're expecting it often is + # in the WebService, and WebService clients usually handle this + # correctly. + $self->delete('POSTDATA'); + my $url = $sslbase . $self->url('-path_info' => 1, '-query' => 1, + '-relative' => 1); + + # XML-RPC clients (SOAP::Lite at least) require a 301 to redirect properly + # and do not work with 302. Our redirect really is permanent anyhow, so + # it doesn't hurt to make it a 301. + print $self->redirect(-location => $url, -status => 301); + # When using XML-RPC with mod_perl, we need the headers sent immediately. $self->r->rflush if $ENV{MOD_PERL}; exit; @@ -459,13 +460,13 @@ effectively removing the cookie. As its only argument, it takes the name of the cookie to expire. -=item C<require_https($baseurl)> +=item C<redirect_to_https> -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. +This routine redirects the client to the https version of the page that +they're looking at, using the C<sslbase> parameter for the redirection. -It takes an optional argument which will be used as the base URL. If $baseurl -is not provided, the current URL is used. +Generally you should use L<Bugzilla::Util/do_ssl_redirect_if_required> +instead of calling this directly. =item C<redirect_to_urlbase> diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index a20751737..f56ffd78a 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -192,6 +192,11 @@ sub update_params { $param->{'mail_delivery_method'} = $translation{$method}; } + # Convert the old "ssl" parameter to the new "ssl_redirect" parameter. + # Both "authenticated sessions" and "always" turn on "ssl_redirect" + # when upgrading. + $param->{'ssl_redirect'} = 1 if $param->{'ssl'} ne 'never'; + # --- DEFAULTS FOR NEW PARAMS --- _load_params unless %params; diff --git a/Bugzilla/Config/Core.pm b/Bugzilla/Config/Core.pm index 6d413b965..91426b30a 100644 --- a/Bugzilla/Config/Core.pm +++ b/Bugzilla/Config/Core.pm @@ -68,10 +68,9 @@ sub get_param_list { }, { - name => 'ssl', - type => 's', - choices => ['never', 'authenticated sessions', 'always'], - default => 'never' + name => 'ssl_redirect', + type => 'b', + default => 0 }, diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index 610523b8a..83ae5a600 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -82,10 +82,7 @@ sub MessageToMTA { # # We don't use correct_urlbase, because we want this URL to # *always* be the same for this Bugzilla, in every email, - # and some emails we send when we're logged out (in which case - # some emails might get urlbase while the logged-in emails might - # get sslbase). Also, we want this to stay the same even if - # the admin changes the "ssl" parameter. + # even if the admin changes the "ssl_redirect" parameter some day. $email->header_set('X-Bugzilla-URL', Bugzilla->params->{'urlbase'}); # We add this header to mark the mail as "auto-generated" and diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 55ec6dcf8..90525b9d4 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 ssl_require_redirect use_attachbase + lsearch do_ssl_redirect_if_required use_attachbase diff_arrays trim wrap_hard wrap_comment find_wrap_point format_time format_time_decimal validate_date @@ -264,60 +264,28 @@ 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')); - } +# This exists as a separate function from Bugzilla::CGI::redirect_to_https +# because we don't want to create a CGI object during XML-RPC calls +# (doing so can mess up XML-RPC). +sub do_ssl_redirect_if_required { + return if !i_am_cgi(); + return if !Bugzilla->params->{'ssl_redirect'}; - return 0; + my $sslbase = Bugzilla->params->{'sslbase'}; + + # If we're already running under SSL, never redirect. + return if uc($ENV{HTTPS} || '') eq 'ON'; + # Never redirect if there isn't an sslbase. + return if !$sslbase; + Bugzilla->cgi->redirect_to_https(); } sub correct_urlbase { - my $ssl = Bugzilla->params->{'ssl'}; - return Bugzilla->params->{'urlbase'} if $ssl eq 'never'; - + my $ssl = Bugzilla->params->{'ssl_redirect'}; + my $urlbase = Bugzilla->params->{'urlbase'}; my $sslbase = Bugzilla->params->{'sslbase'}; - if ($sslbase) { - return $sslbase if $ssl eq 'always'; - # Authenticated Sessions - return $sslbase if Bugzilla->user->id; - } - # Set to "authenticated sessions" but nobody's logged in, or - # sslbase isn't set. - return Bugzilla->params->{'urlbase'}; + return ($ssl && $sslbase) ? $sslbase : $urlbase; } sub use_attachbase { @@ -830,7 +798,7 @@ cookies) to only some addresses. =item C<correct_urlbase()> Returns either the C<sslbase> or C<urlbase> parameter, depending on the -current setting for the C<ssl> parameter. +current setting for the C<ssl_redirect> parameter. =item C<use_attachbase()> diff --git a/Bugzilla/WebService/Server.pm b/Bugzilla/WebService/Server.pm index dfb9f559a..2db182fd4 100644 --- a/Bugzilla/WebService/Server.pm +++ b/Bugzilla/WebService/Server.pm @@ -17,26 +17,12 @@ package Bugzilla::WebService::Server; use strict; -use Bugzilla::Util qw(ssl_require_redirect); sub handle_login { my ($self, $class, $method, $full_method) = @_; eval "require $class"; return if $class->login_exempt($method); 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. - Bugzilla->cgi->require_https(Bugzilla->params->{'sslbase'}) - if ssl_require_redirect($full_method); } 1; diff --git a/docs/en/xml/administration.xml b/docs/en/xml/administration.xml index 6b6ff5fda..0c9a60ce2 100644 --- a/docs/en/xml/administration.xml +++ b/docs/en/xml/administration.xml @@ -100,13 +100,13 @@ <varlistentry> <term> - ssl + ssl_redirect </term> <listitem> <para> - Determines when Bugzilla will force HTTPS (SSL) connections, using - the URL defined in <command>sslbase</command>. - Options include "always", "never", and "authenticated sessions". + If enabled, Bugzilla will force HTTPS (SSL) connections, by + automatically redirecting any users who try to use a non-SSL + connection. </para> </listitem> </varlistentry> @@ -56,14 +56,6 @@ if ($cgi->param('logout')) { # Main Body Execution ############################################################################### -# Force to use HTTPS unless Bugzilla->params->{'ssl'} equals 'never'. -# This is required because the user may want to log in from here. -if ($cgi->protocol ne 'https' && Bugzilla->params->{'sslbase'} ne '' - && Bugzilla->params->{'ssl'} ne 'never') -{ - $cgi->require_https(Bugzilla->params->{'sslbase'}); -} - # Return the appropriate HTTP response headers. print $cgi->header(); diff --git a/template/en/default/account/auth/login-small.html.tmpl b/template/en/default/account/auth/login-small.html.tmpl index 752aa64df..710d7d0a5 100644 --- a/template/en/default/account/auth/login-small.html.tmpl +++ b/template/en/default/account/auth/login-small.html.tmpl @@ -28,12 +28,7 @@ [% login_target = "index.cgi" %] [% END %] -[%# If SSL is in use, use 'sslbase', else use 'urlbase'. %] -[% IF Param("sslbase") != "" && Param("ssl") != "never" %] - [% login_target = Param("sslbase") _ login_target %] -[% ELSE %] - [% login_target = Param("urlbase") _ login_target %] -[% END %] +[% login_target = urlbase _ login_target %] <li id="mini_login_container[% qs_suffix %]"> <span class="separator">| </span> diff --git a/template/en/default/admin/params/core.html.tmpl b/template/en/default/admin/params/core.html.tmpl index d66c4a51b..b65dde233 100644 --- a/template/en/default/admin/params/core.html.tmpl +++ b/template/en/default/admin/params/core.html.tmpl @@ -42,8 +42,12 @@ sslbase => "The URL that is the common initial leading part of all HTTPS " _ "(SSL) $terms.Bugzilla URLs.", - ssl => "Controls when $terms.Bugzilla should enforce sessions to use HTTPS by " _ - "using <tt>sslbase</tt>.", + ssl_redirect => + "When this is enabled, $terms.Bugzilla will ensure that every page is" + _ " accessed over SSL, by redirecting any plain HTTP requests to HTTPS" + _ " using the <tt>sslbase</tt> parameter. Also, when this is enabled," + _ " $terms.Bugzilla will send out links using <tt>sslbase</tt> in emails" + _ " instead of <tt>urlbase</tt>.", cookiedomain => "The domain for $terms.Bugzilla cookies. Normally blank. " _ "If your website is at 'www.foo.com', setting this to " _ diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl index 76ca259e5..d5a345182 100644 --- a/template/en/default/bug/edit.html.tmpl +++ b/template/en/default/bug/edit.html.tmpl @@ -277,7 +277,8 @@ <legend>Note</legend> <p> You need to - <a href="[% IF Param('ssl') != 'never' %][% Param('sslbase') %][% END %]show_bug.cgi?id=[% bug.bug_id %]&GoAheadAndLogIn=1">log in</a> + <a href="show_bug.cgi?id= + [%- bug.bug_id %]&GoAheadAndLogIn=1">log in</a> before you can comment on or make changes to this [% terms.bug %]. </p> </fieldset> @@ -360,15 +360,7 @@ sub request_create_account { $vars->{'email'} = $login_name . Bugzilla->params->{'emailsuffix'}; $vars->{'expiration_ts'} = ctime(str2time($date) + MAX_TOKEN_AGE * 86400); - # 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'}); - } print $cgi->header(); - $template->process('account/email/confirm-new.html.tmpl', $vars) || ThrowTemplateError($template->error()); } |