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