[ 
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)

Reply via email to