Note: There are a few cases of duplicates in user autocompletion which are being worked on.

xmlrpc.cgi doesn't send any security-related headers

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
WebService
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: reed, Assigned: dkl)

Tracking

4.3.2
Bugzilla 4.0
Bug Flags:
approval +
approval4.4 +
approval4.2 +
approval4.0 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
xmlrpc.cgi doesn't seem to send any of the security-related headers that are sent for every other page. The one that worries me the most is the lack of X-Frame-Options...

$ curl -I https://landfill.bugzilla.org/bugzilla-tip/xmlrpc.cgi
HTTP/1.1 200 OK
Date: Fri, 31 Aug 2012 08:27:47 GMT
Server: Apache/2.2.3 (CentOS)
Connection: close
Content-Type: text/plain; charset=UTF-8

$ curl -I https://landfill.bugzilla.org/bugzilla-tip/jsonrpc.cgi
HTTP/1.1 403 Forbidden
Date: Fri, 31 Aug 2012 08:27:55 GMT
Server: Apache/2.2.3 (CentOS)
X-xss-protection: 1; mode=block
Strict-transport-security: max-age=604800
X-frame-options: SAMEORIGIN
X-content-type-options: nosniff
Connection: close
Content-Type: application/json; charset=UTF-8
Created attachment 657196 [details] [diff] [review]
patch v1

this patch copies across all X- headers that Bugzilla:CGI generates into xmlrpc responses.

i'm not totally happy with this patch, as i suspect there's an easier way to achieve this, but this does the job.
Assignee: webservice → glob
Status: NEW → ASSIGNED
Attachment #657196 - Flags: review?(reed)
(Assignee)

Comment 2

5 years ago
Comment on attachment 657196 [details] [diff] [review]
patch v1

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

I don't see another easy way to do it either as SOAP::Transport::HTTP.pm doesnt use CGI.pm and use its
own instance of HTTP::Response. Tested this on my local instance and works well.

r=dkl
Attachment #657196 - Flags: review+
(Reporter)

Comment 3

5 years ago
Comment on attachment 657196 [details] [diff] [review]
patch v1

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

This will miss the 'Strict-Transport-Security' header. Note that it's very likely that some of the other headers will be losing the 'X-' prefix shortly, as the various standards groups are deprecating it. Maybe instead of only sending the 'X-' prefixed headers, we send those that don't match the standard ones (whatever they might be)? That may be easier to handle, and it's less likely that a new header will be added that causes problems there vs. custom headers added in Bugzilla/CGI.pm.
Attachment #657196 - Flags: review?(reed) → review-

Comment 4

5 years ago
Why is this bug filed with the security flag set? We are not talking about a security hole here, but only about some headers not returned and which are of no used when using XML-RPC. About JSON-RPC, it's a different story because it's fine to call jsonrpc.cgi from a web browser.
(Assignee)

Comment 5

5 years ago
Maybe this instead? Works for me in my testing:

    # Copy across security related headers from Bugzilla::CGI
    foreach my $header (split(/[\r\n]+/, Bugzilla->cgi->header)) {
        my ($name, $value) = $header =~ /^([^:]+): (.*)/;
        if (!$self->response->headers->header($name)) {
           $self->response->headers->header($name => $value);
        }
    }

It makes it more difficult due to CGI.pm not using HTTP::Headers so cannot use header_field_names to loop through the headers one at a time.

dkl
(Reporter)

Comment 6

5 years ago
(In reply to Frédéric Buclin from comment #4)
> Why is this bug filed with the security flag set? We are not talking about a
> security hole here, but only about some headers not returned and which are
> of no used when using XML-RPC. About JSON-RPC, it's a different story
> because it's fine to call jsonrpc.cgi from a web browser.

You can call XML-RPC from a web browser, just not as easily as JSON-RPC (as in, you're likely to have to use JavaScript to do it).

Comment 7

5 years ago
(In reply to Reed Loden [:reed] from comment #6)
> You can call XML-RPC from a web browser, just not as easily as JSON-RPC (as
> in, you're likely to have to use JavaScript to do it).

You cannot call *our* XML-RPC API from a web browser, even with JavaScript. We explicitly blocked simple cross-site requests in bug 725663. And if you try to alter the Content-Type, your browser won't get anything back anyway.
(Reporter)

Comment 8

5 years ago
dkl, can you attach an actual patch with what you said in comment #5?
(Assignee)

Comment 9

5 years ago
Created attachment 659613 [details] [diff] [review]
patch v2
Attachment #657196 - Attachment is obsolete: true
Attachment #659613 - Flags: review?(reed)
(Assignee)

Updated

5 years ago
Attachment #659613 - Flags: review?(glob)
(Reporter)

Comment 10

5 years ago
Comment on attachment 659613 [details] [diff] [review]
patch v2

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

This doesn't work with my testing...

$ curl -I .../jsonrpc.cgi
HTTP/1.1 403 Forbidden
Date: Mon, 10 Sep 2012 04:40:31 GMT
Server: Apache/2.2.3 (CentOS)
X-xss-protection: 1; mode=block
X-frame-options: SAMEORIGIN
X-content-type-options: nosniff
Connection: close
Content-Type: application/json; charset=UTF-8

$ curl -I .../xmlrpc.cgi
HTTP/1.1 411 Length Required
Date: Mon, 10 Sep 2012 04:40:41 GMT
Server: Apache/2.2.3 (CentOS)
Connection: close
Content-Type: text/plain; charset=UTF-8
Attachment #659613 - Flags: review?(reed) → review-
(In reply to Reed Loden [:reed] from comment #10)
> This doesn't work with my testing...

please test with a successful request; iirc failures take a different path in the code.
(Reporter)

Comment 12

5 years ago
(In reply to Byron Jones ‹:glob› from comment #11)
> (In reply to Reed Loden [:reed] from comment #10)
> > This doesn't work with my testing...
> 
> please test with a successful request; iirc failures take a different path
> in the code.

Will test, but can we fix the failure path as well so it matches everything else?
(In reply to Reed Loden [:reed] from comment #12)
> Will test, but can we fix the failure path as well so it matches everything
> else?

unfortunately not - it's deep in SOAP::Transport::HTTP
Comment on attachment 659613 [details] [diff] [review]
patch v2

this works when you send a valid request:

T 127.0.0.1:45903 -> 127.0.0.1:80 [AP]
  POST /787328/xmlrpc.cgi HTTP/1.1..TE: deflate,gzip;q=0.3..Connection: TE, close..Accept: text/xml..Accept: multipart/*..Accept: 
  application/soap..Host: fedora..User-Agent: SOAP::Lite/Perl/0.712..Content-Length: 114..Content-Type: text/xml....<?xml version=
  "1.0" encoding="UTF-8"?><methodCall><methodName>Bugzilla.version</methodName><params /></methodCall>                            
##
T 127.0.0.1:80 -> 127.0.0.1:45903 [AP]
  HTTP/1.1 200 OK..Date: Mon, 12 Nov 2012 15:03:11 GMT..Server: Apache/2.2.22 (Fedora)..SOAPServer: SOAP::Lite/Perl/0.712..X-Conte
  nt-Type-Options: nosniff..X-Frame-Options: SAMEORIGIN..X-Xss-Protection: 1; mode=block..Content-Length: 207..Connection: close..
  Content-Type: text/xml....<?xml version="1.0" encoding="UTF-8"?><methodResponse><params><param><value><struct><member><name>vers
  ion</name><value><string>4.5</string></value></member></struct></value></param></params></methodResponse>
Attachment #659613 - Flags: review?(glob)
Attachment #659613 - Flags: review-
Attachment #659613 - Flags: review+
Assignee: glob → dkl
Flags: approval?
Flags: approval4.4?
Flags: approval4.2?
(Reporter)

Comment 15

5 years ago
*poke* --- can we get this into a release?

Comment 16

5 years ago
(In reply to Reed Loden [:reed] from comment #15)
> *poke* --- can we get this into a release?

No, not until someone replies to comment 7.
(Reporter)

Comment 17

5 years ago
(In reply to Frédéric Buclin from comment #16)
> (In reply to Reed Loden [:reed] from comment #15)
> > *poke* --- can we get this into a release?
> 
> No, not until someone replies to comment 7.

What part of comment #7 needs a reply? Even if this turns out to not be directly exploitable, it's still a bug, and we have a reviewed fix. Let's take it and get it into a release.

Comment 18

5 years ago
(In reply to Reed Loden [:reed] from comment #17)
> What part of comment #7 needs a reply? Even if this turns out to not be
> directly exploitable, it's still a bug

The part which needs a reply is "You cannot call *our* XML-RPC API from a web browser, even with JavaScript." So your comment 6 doesn't stand. The security headers are for the web browser, and so it's not a security bug to not send them if you cannot use your browser anyway.

If it's a security bug, the patch must land in all supported branches and need a security advisory. If it's simply a security enhancement, it should land in trunk + 4.4 only and doesn't need a security advisory. This makes a big difference. And from the comments above, I see no evidence that it's a security bug.
(Reporter)

Comment 19

5 years ago
Do we allow GETs to xmlrpc.cgi to affect anything of consequence?
(Reporter)

Comment 20

5 years ago
(In reply to Frédéric Buclin from comment #18)
> If it's a security bug, the patch must land in all supported branches and
> need a security advisory. If it's simply a security enhancement, it should
> land in trunk + 4.4 only and doesn't need a security advisory. This makes a
> big difference. And from the comments above, I see no evidence that it's a
> security bug.

Then let's treat it as a (non-security) bug, and land it on 4.0 and higher (since that's when we started sending X-Frame-Options and HSTS), yet not put it into a security advisory.
I can go with comment 20's proposal.  Looks like we have enough stuff already on the 4.0 branch since the last release to be worth doing another release of it next time we have an excuse to push one (so we don't need to worry about doing a release just for this).
Group: bugzilla-security
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 22

4 years ago
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/trunk
modified Bugzilla/WebService/Server/XMLRPC.pm
Committed revision 8647.

Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/4.4
modified Bugzilla/WebService/Server/XMLRPC.pm
Committed revision 8576.

Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/4.2
modified Bugzilla/WebService/Server/XMLRPC.pm
Committed revision 8218.

Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/4.0
modified Bugzilla/WebService/Server/XMLRPC.pm
Committed revision 7752.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Target Milestone: --- → Bugzilla 4.0
You need to log in before you can comment on or make changes to this bug.