You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Brian Eaton (JIRA)" <ji...@apache.org> on 2008/03/17 13:49:24 UTC
[jira] Created: (SHINDIG-133) forwarding browser headers on remote
content requests
forwarding browser headers on remote content requests
-----------------------------------------------------
Key: SHINDIG-133
URL: https://issues.apache.org/jira/browse/SHINDIG-133
Project: Shindig
Issue Type: Bug
Components: Gadgets Server - Java
Reporter: Brian Eaton
There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
As an example of various things that are likely to go wrong with the current code:
- cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
- some hop by hop headers will be forwarded
There are probably other issues.
Problem code is here:
if ("POST".equals(method)) {
....
} else {
postBody = null;
headers = new HashMap<String, List<String>>();
Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
String header = headerNames.nextElement();
headers.put(header, Collections.list(request.getHeaders(header)));
}
}
removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Created: (SHINDIG-133) forwarding browser headers on remote content requests
Posted by Kevin Brown <et...@google.com>.
On Mon, Mar 17, 2008 at 4:52 PM, Brian Eaton <be...@google.com> wrote:
> On Mon, Mar 17, 2008 at 4:27 PM, Kevin Brown <et...@google.com> wrote:
> > There's a problem here in that this is a proxy. Certain headers must be
> > forwarded so that the end user can actually understand the response,
> > especially for things like images.
>
> You have a point about the proxy. Maybe we should separate out the
> handling of the direct proxy vs the JSON proxy?
>
They were separated originally, but it got changed along the way somehow.
Do you think the greylist solution would solve our problems here though?
It's pretty easy to implement, I'm not completely sure if it's sufficient.
--
~Kevin
Re: [jira] Created: (SHINDIG-133) forwarding browser headers on remote content requests
Posted by Brian Eaton <be...@google.com>.
On Mon, Mar 17, 2008 at 4:27 PM, Kevin Brown <et...@google.com> wrote:
> There's a problem here in that this is a proxy. Certain headers must be
> forwarded so that the end user can actually understand the response,
> especially for things like images.
You have a point about the proxy. Maybe we should separate out the
handling of the direct proxy vs the JSON proxy?
Re: [jira] Created: (SHINDIG-133) forwarding browser headers on remote content requests
Posted by Kevin Brown <et...@google.com>.
On Mon, Mar 17, 2008 at 12:18 PM, Brian Eaton <be...@google.com> wrote:
> The current code is pulling headers directly out of the HTTP request
> sent from the client and forwarding them on. That's just broken.
Not quite -- it removes a few (though not nearly enough)
We should probably do this:
- Establish a whitelist for "normal" headers.
- Establish a blacklist of X- headers (X-Forwarded-For, for instance).
At some point we should worry about what HTTP headers to allow when
> gadgets intentionally ask to forward headers. In the meantime we
> should stop forwarding headers that gadget's didn't ask us to forward
> in the first place.
There's a problem here in that this is a proxy. Certain headers must be
forwarded so that the end user can actually understand the response,
especially for things like images.
>
> On Mon, Mar 17, 2008 at 11:21 AM, John Panzer <jp...@google.com> wrote:
> > Problem: Whitelisting headers will inevitably miss some (Slug:?)
> > which some gadget will need somewhere.
> >
> > Same issue that the W3C is struggling with over cross-site requests
> > for browsers. Possibly that discussion has come up with a good
> > blacklist...
> >
> > I know that black lists aren't good security, but...
> >
> >
> >
> > On Mon, Mar 17, 2008 at 5:49 AM, Brian Eaton (JIRA) <ji...@apache.org>
> wrote:
> > > forwarding browser headers on remote content requests
> > > -----------------------------------------------------
> > >
> > > Key: SHINDIG-133
> > > URL:
> https://issues.apache.org/jira/browse/SHINDIG-133
> > > Project: Shindig
> > > Issue Type: Bug
> > > Components: Gadgets Server - Java
> > > Reporter: Brian Eaton
> > >
> > >
> > > There is some fairly dodgy code in ProxyHandler.java. If a GET
> request shows up at the server, nearly all of the headers sent from the
> browser are forwarded to the backend. This should be replaced with a white
> list of headers that are OK to copy out of the request.
> > >
> > > As an example of various things that are likely to go wrong with the
> current code:
> > > - cookies will be forwarded (and yes, I know gadgets shouldn't have
> cookies, but if they do we shouldn't leak them this way.)
> > > - some hop by hop headers will be forwarded
> > >
> > > There are probably other issues.
> > >
> > > Problem code is here:
> > >
> > > if ("POST".equals(method)) {
> > > ....
> > > } else {
> > > postBody = null;
> > > headers = new HashMap<String, List<String>>();
> > > Enumeration<String> headerNames = request.getHeaderNames();
> > > while (headerNames.hasMoreElements()) {
> > > String header = headerNames.nextElement();
> > > headers.put(header, Collections.list(request.getHeaders
> (header)));
> > > }
> > > }
> > >
> > > removeUnsafeHeaders(headers);
> > >
> > >
> > > --
> > > This message is automatically generated by JIRA.
> > > -
> > > You can reply to this email to add a comment to the issue online.
> > >
> > >
> >
>
--
~Kevin
Re: [jira] Created: (SHINDIG-133) forwarding browser headers on remote content requests
Posted by Brian Eaton <be...@google.com>.
The current code is pulling headers directly out of the HTTP request
sent from the client and forwarding them on. That's just broken.
At some point we should worry about what HTTP headers to allow when
gadgets intentionally ask to forward headers. In the meantime we
should stop forwarding headers that gadget's didn't ask us to forward
in the first place.
On Mon, Mar 17, 2008 at 11:21 AM, John Panzer <jp...@google.com> wrote:
> Problem: Whitelisting headers will inevitably miss some (Slug:?)
> which some gadget will need somewhere.
>
> Same issue that the W3C is struggling with over cross-site requests
> for browsers. Possibly that discussion has come up with a good
> blacklist...
>
> I know that black lists aren't good security, but...
>
>
>
> On Mon, Mar 17, 2008 at 5:49 AM, Brian Eaton (JIRA) <ji...@apache.org> wrote:
> > forwarding browser headers on remote content requests
> > -----------------------------------------------------
> >
> > Key: SHINDIG-133
> > URL: https://issues.apache.org/jira/browse/SHINDIG-133
> > Project: Shindig
> > Issue Type: Bug
> > Components: Gadgets Server - Java
> > Reporter: Brian Eaton
> >
> >
> > There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> >
> > As an example of various things that are likely to go wrong with the current code:
> > - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> > - some hop by hop headers will be forwarded
> >
> > There are probably other issues.
> >
> > Problem code is here:
> >
> > if ("POST".equals(method)) {
> > ....
> > } else {
> > postBody = null;
> > headers = new HashMap<String, List<String>>();
> > Enumeration<String> headerNames = request.getHeaderNames();
> > while (headerNames.hasMoreElements()) {
> > String header = headerNames.nextElement();
> > headers.put(header, Collections.list(request.getHeaders(header)));
> > }
> > }
> >
> > removeUnsafeHeaders(headers);
> >
> >
> > --
> > This message is automatically generated by JIRA.
> > -
> > You can reply to this email to add a comment to the issue online.
> >
> >
>
Re: [jira] Created: (SHINDIG-133) forwarding browser headers on remote content requests
Posted by John Panzer <jp...@google.com>.
Problem: Whitelisting headers will inevitably miss some (Slug:?)
which some gadget will need somewhere.
Same issue that the W3C is struggling with over cross-site requests
for browsers. Possibly that discussion has come up with a good
blacklist...
I know that black lists aren't good security, but...
On Mon, Mar 17, 2008 at 5:49 AM, Brian Eaton (JIRA) <ji...@apache.org> wrote:
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadgets Server - Java
> Reporter: Brian Eaton
>
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
>
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
>
> There are probably other issues.
>
> Problem code is here:
>
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
>
> removeUnsafeHeaders(headers);
>
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>
[jira] Commented: (SHINDIG-133) forwarding browser headers on
remote content requests
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598048#action_12598048 ]
Paul Lindner commented on SHINDIG-133:
--------------------------------------
At hi5 we just stopped sending any client headers to the backend. This works just fine and has resolved all of our dodgy fetch problems.
Come to think of it there are hardly any cases where the headers need to be sent from the client to backend host, since you can specify the ones you want in the makeRequest() call.
Maybe this is needed for the direct proxy? Maybe if we streamed contect direct from the origin server that would make sense, but we don't. Instead we gather the results of the fetch, and send them on their way.
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (Java)
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-133) forwarding browser headers on remote
content requests
Posted by "Brian Eaton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Brian Eaton updated SHINDIG-133:
--------------------------------
Comment: was deleted
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (Java)
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-133) forwarding browser headers on
remote content requests
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579683#action_12579683 ]
Paul Lindner commented on SHINDIG-133:
--------------------------------------
To see what we're up against have a look at the squid header handling..
http://www.squid-cache.org/cgi-bin/cvsweb.cgi/squid3/src/HttpHeader.cc?rev=1.138.4.1&content-type=text/x-cvsweb-markup
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadgets Server - Java
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-133) forwarding browser headers on
remote content requests
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579680#action_12579680 ]
Paul Lindner commented on SHINDIG-133:
--------------------------------------
You have to be really really careful with headers. Odd things can cause problems like this:
http://securitytracker.com/alerts/2005/Jul/1014350.html
Also, one probably needs to correctly parse the Vary: header to insure that dynamic content is properly cached.
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadgets Server - Java
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-133) forwarding browser headers on
remote content requests
Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579685#action_12579685 ]
Kevin Brown commented on SHINDIG-133:
-------------------------------------
I don't think we have to implement everything that squid deals with, but we do at least need to handle all of the common headers that user agents pass where appropriate.
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadgets Server - Java
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-133) forwarding browser headers on
remote content requests
Posted by "Brian Eaton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12603252#action_12603252 ]
Brian Eaton commented on SHINDIG-133:
-------------------------------------
We're also leaking cookies here. Gadgets aren't supposed to be setting cookies, but leaking them is bad nonetheless.
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (Java)
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-133) forwarding browser headers on
remote content requests
Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598096#action_12598096 ]
Kevin Brown commented on SHINDIG-133:
-------------------------------------
Given the ability (and preference) to cache, I think it makes little sense to worry about browser headers, and I think your assessment is correct -- only explicitly specified headers should be sent.
> forwarding browser headers on remote content requests
> -----------------------------------------------------
>
> Key: SHINDIG-133
> URL: https://issues.apache.org/jira/browse/SHINDIG-133
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (Java)
> Reporter: Brian Eaton
>
> There is some fairly dodgy code in ProxyHandler.java. If a GET request shows up at the server, nearly all of the headers sent from the browser are forwarded to the backend. This should be replaced with a white list of headers that are OK to copy out of the request.
> As an example of various things that are likely to go wrong with the current code:
> - cookies will be forwarded (and yes, I know gadgets shouldn't have cookies, but if they do we shouldn't leak them this way.)
> - some hop by hop headers will be forwarded
> There are probably other issues.
> Problem code is here:
> if ("POST".equals(method)) {
> ....
> } else {
> postBody = null;
> headers = new HashMap<String, List<String>>();
> Enumeration<String> headerNames = request.getHeaderNames();
> while (headerNames.hasMoreElements()) {
> String header = headerNames.nextElement();
> headers.put(header, Collections.list(request.getHeaders(header)));
> }
> }
> removeUnsafeHeaders(headers);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.