[
https://issues.apache.org/jira/browse/YARN-2528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14132480#comment-14132480
]
Jonathan Eagles commented on YARN-2528:
---------------------------------------
{quote}
Jonathan Eagles, no problem. I compared our CrossOriginFilter with the one in
Jetty. That one seems not to do any post-process for the string obtained from
ORIGIN header. What's the reason that we need for our CrossOriginFilter?
According to test case, you want to avoid the issue that the string contains
the other header, don't you? HttpServletResponse.getHeader doesn't handle
header splitting properly?
{quote}
[~zjshen], I haven't described yet in the JIRA how I came to make that change.
To reproduce this findbugs warning, take the above v2 patch and make the
following modifications setting the origin header response directly from the
origin header request.
{code}
- res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
+ res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, req.getHeader(ORIGIN));
{code}
and then run findbugs
{code}
mvn clean test findbugs:findbugs -DskipTests
{code}
in the hadoop-yarn-server-applicationhistoryservice directory. Here is the
findbugs error report.
{code}
<BugCollection version="1.3.9" threshold="medium" effort="max">
<file
classname="org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter">
<BugInstance type="HRS_REQUEST_PARAMETER_TO_HTTP_HEADER" priority="Normal"
category="SECURITY" message="HTTP parameter directly written to HTTP header
output in
org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter.doCrossFilter(HttpServletRequest,
HttpServletResponse)" lineNumber="128"/>
</file>
<Error/>
<Project>
<SrcDir>/Users/jeagles/hadoop/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java
</SrcDir>
</Project>
</BugCollection>
<BugInstance category="SECURITY"
instanceHash="51f813d33b6e152c981d270989331951" instanceOccurrenceNum="0"
priority="2" abbrev="HRS" type="HRS_REQUEST_PARAMETER_TO_HTTP_HEADER"
instanceOccurrenceMax="0">
<ShortMessage>HTTP Response splitting vulnerability</ShortMessage>
<LongMessage>HTTP parameter directly written to HTTP header output in
org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter.doCrossFilter(HttpServletRequest,
HttpServletResponse)</LongMessage>
<Class
classname="org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter"
primary="true">
<SourceLine start="43"
classname="org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter"
sourcepath="org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java"
sourcefile="CrossOriginFilter.java" end="240">
<Message>At CrossOriginFilter.java:[lines 43-240]</Message>
</SourceLine>
<Message>In class
org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter</Message>
</Class>
<Method isStatic="false"
classname="org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter"
name="doCrossFilter" primary="true"
signature="(Ljavax/servlet/http/HttpServletRequest;Ljavax/servlet/http/HttpServletResponse;)V">
<SourceLine endBytecode="318" startBytecode="0" start="107"
classname="org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter"
sourcepath="org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java"
sourcefile="CrossOriginFilter.java" end="133"/>
<Message>In method
org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter.doCrossFilter(HttpServletRequest,
HttpServletResponse)</Message>
</Method>
<SourceLine endBytecode="80" startBytecode="80" start="128"
classname="org.apache.hadoop.yarn.server.timeline.webapp.CrossOriginFilter"
primary="true"
sourcepath="org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java"
sourcefile="CrossOriginFilter.java" end="128">
<Message>At CrossOriginFilter.java:[line 128]</Message>
</SourceLine>
</BugInstance>
<BugCategory
category="SECURITY"><Description>Security</Description></BugCategory>
<BugPattern category="SECURITY" abbrev="HRS"
type="HRS_REQUEST_PARAMETER_TO_HTTP_HEADER">
<ShortDescription>HTTP Response splitting vulnerability</ShortDescription>
<Details>
<p>This code directly writes an HTTP parameter to an HTTP header, which
allows for a HTTP response splitting
vulnerability. See <a
href="http://en.wikipedia.org/wiki/HTTP_response_splitting">http://en.wikipedia.org/wiki/HTTP_response_splitting</a>
for more information.</p>
<p>FindBugs looks only for the most blatant, obvious cases of HTTP response
splitting.
If FindBugs found <em>any</em>, you <em>almost certainly</em> have more
vulnerabilities that FindBugs doesn't report. If you are concerned about HTTP
response splitting, you should seriously
consider using a commercial static analysis or pen-testing tool.
</p>
</Details>
</BugPattern>
</BugCategory
{code}
Unfortunately, It is too easy to trick findbugs in this scenario so perhaps
that is why jetty doesn't implement this security fix. It also could be a
difference with jetty 6 vs later versions of jetty. I have kept the protection
in the patch but have relaxed it to support the multiple origins feature
improvement you have suggested.
{quote}
BTW, it seems that ours' only allows one origin in the request header, but
Jetty's allows multiple one. And I find a specification:
http://tools.ietf.org/html/draft-abarth-origin-09, which tells that ORIGIN can
be a list. Any thought?
{quote}
Good catch, I have gone ahead and added the multiple origins feature and
support tests.
Thanks again, [~zjshen]. Let me know you thoughts on this latest patch.
> Cross Origin Filter Http response split vulnerability protection rejects
> valid origins
> --------------------------------------------------------------------------------------
>
> Key: YARN-2528
> URL: https://issues.apache.org/jira/browse/YARN-2528
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Reporter: Jonathan Eagles
> Assignee: Jonathan Eagles
> Attachments: YARN-2528-v1.patch, YARN-2528-v2.patch
>
>
> URLEncoding is too strong of a protection for HTTP Response Split
> Vulnerability protection and major browser reject the encoded Origin. An
> adequate protection is simply to remove all CRs LFs as in the case of PHP's
> header function.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)