Bugzilla is unable to access attachment.cgi when ssl_redirect = true and using Plack

RESOLVED FIXED in Bugzilla 6.0

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: LpSolit, Assigned: LpSolit)

Tracking

Bugzilla 6.0

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
I have configured Apache to use Plack like this:

<Location /bugzilla/>
    ProxyPass "http://localhost:5000/"
    ProxyPassReverse "http://localhost:5000/"
</Location>

But when I try to access attachment.cgi, Bugzilla enters an infinite loop because attachment.cgi calls do_ssl_redirect_if_required() which itself calls attachment.cgi again, etc.... The reason is that ssl_redirect forces to use HTTPS, but Plack is configured to use HTTP only. I'm not even sure we can configure Plack to use SSL.
(Assignee)

Comment 1

3 years ago
Another problem is that Bugzilla is unable to check the (attachment) host correctly, e.g. in url_is_attachment_base(). When it compares $cgi->url against the 'attachment_base' parameter, this check will always return false.
(Assignee)

Comment 2

3 years ago
Created attachment 8738817 [details] [diff] [review]
patch, v1

OK, I finally managed to address all bugs related to using Plack behind a reverse proxy. First of all, the configuration in httpd.conf should be:

<Location /bugzilla/>
    ProxyPreserveHost On
    ProxyPass "http://localhost:5000/"
    ProxyPassReverse "http://localhost:5000/"
    # Will pass either 'https' or 'http' (without quotes).
    RequestHeader set X-Forwarded-Proto %{REQUEST_SCHEME}s
    # Will pass the original path to the script, required to determine
    # if we are on the attachment host.
    RequestHeader set X-Forwarded-URI %{REQUEST_URI}s
</Location>

Then the patch addresses the following issues:
- If ssl_redirect = true, Bugzilla entered an infinite loop when trying to access attachment.cgi, because it was unable to determine the original protocol (HTTPS);
- Even with the first issue addresses thanks to X-Forwarded-Proto (RFC 7239), Bugzilla was unable to determine the original URL, and so entered an infinite loop again when trying to display attachments in "View" mode, because the URL never matched the attachmentbase URL.
- The ssl_redirect parameter was ignored outside attachment.cgi, because the redirection was done in Bugzilla::init_page(), which is before the CGI script is loaded by Plack, and so i_am_cgi() was always false (which means no redirection). Moving the redirection in Bugzilla::CGI->new() fixes this issue. This move is fine, because either the script is not CGI (i_am_cgi() = false) in which case the redirection was already ignored, or the script is CGI (i_am_cgi() = true) in which case Bugzilla->cgi is always called to access parameters and to set headers.
Assignee: general → LpSolit
Status: NEW → ASSIGNED
Attachment #8738817 - Flags: review?(dylan)
(Assignee)

Comment 3

3 years ago
Created attachment 8738818 [details] [diff] [review]
patch, v1

(this time, correctly formatted)
Attachment #8738817 - Attachment is obsolete: true
Attachment #8738817 - Flags: review?(dylan)
Attachment #8738818 - Flags: review?(dylan)
(Assignee)

Updated

3 years ago
Blocks: 1258157
(Assignee)

Comment 4

3 years ago
Created attachment 8739116 [details] [diff] [review]
patch, v1.1

Better patch. Only look at X-* headers if we are behind a proxy, else ignore them.
Attachment #8738818 - Attachment is obsolete: true
Attachment #8738818 - Flags: review?(dylan)
Attachment #8739116 - Flags: review?(dylan)
Comment on attachment 8739116 [details] [diff] [review]
patch, v1.1

Review of attachment 8739116 [details] [diff] [review]:
-----------------------------------------------------------------

r=dylan this works, although I have some reservations -- see below.

::: Bugzilla/CGI.pm
@@ +539,5 @@
> +    my $url = $self->url;
> +
> +    # If we are behind a reverse proxy, we need to determine the original
> +    # URL, else the comparison with the attachment_base URL will fail.
> +    if (Bugzilla->params->{'inbound_proxies'}) {

Do you think we should also compare the connection's IP against the inbound proxy setting, to protect against misconfiguration?

Actually, more broadly, perhaps if inbound proxies are configured we should fail to serve content to other ip addresses?
Attachment #8739116 - Flags: review?(dylan) → review+
(Assignee)

Comment 6

3 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   60299ec..fa360be  master -> master


(In reply to Dylan William Hardison [:dylan] from comment #5)
> Actually, more broadly, perhaps if inbound proxies are configured we should
> fail to serve content to other ip addresses?

Feel free to file a separate bug, as enhancement ;). But if you are between the proxy and Bugzilla, this check would fail. So I'm not sure it's a good idea to throw an error in that case.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.