Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: security
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Cross Frame Scripting (XFS) prevention for UIs can be provided through a common servlet filter. This filter will set the X-Frame-Options HTTP header to DENY unless configured to another valid setting.

      There are a number of UIs that could just add this to their filters as well as the Yarn webapp proxy which could add it for all it's proxied UIs - if appropriate.

      1. HADOOP-13008-001.patch
        14 kB
        Larry McCay
      2. HADOOP-13008-002.patch
        14 kB
        Larry McCay
      3. HADOOP-13008-003.patch
        16 kB
        Larry McCay
      4. HADOOP-13008-004.patch
        24 kB
        Larry McCay

        Issue Links

          Activity

          Hide
          lmccay Larry McCay added a comment -

          Like the RestCsrfPreventionFilter config, I plan to enable individual integration points/webapps to configure the specific value that they want to set as the X-Frame-Options header. It may be that some webapps intend some pages to be embedded in a frame that is served from the same origin. In which case, they could set the configuration property component.prefix.xframe-options to SAMEORIGIN rather than accept the default/global setting.

          In order to do this we should probably check for configuration for the value with two separate prefixes. One for the global setting/prefix and one for the integration specific prefix and override the global value with the component specific value.

          Current thinking is to block the headers from being set by the component itself. Perhaps, this should be config driven. Something like allow.component.overrides?

          Show
          lmccay Larry McCay added a comment - Like the RestCsrfPreventionFilter config, I plan to enable individual integration points/webapps to configure the specific value that they want to set as the X-Frame-Options header. It may be that some webapps intend some pages to be embedded in a frame that is served from the same origin. In which case, they could set the configuration property component.prefix.xframe-options to SAMEORIGIN rather than accept the default/global setting. In order to do this we should probably check for configuration for the value with two separate prefixes. One for the global setting/prefix and one for the integration specific prefix and override the global value with the component specific value. Current thinking is to block the headers from being set by the component itself. Perhaps, this should be config driven. Something like allow.component.overrides?
          Hide
          lmccay Larry McCay added a comment -

          So the following config would turn on XFS protection and set the global value to DENY - not setting the value would also since it is the default:

          hadoop.security.xframe-options-enabled=true
          hadoop.security.xframe-options=DENY
          

          The following additional config would override the value:

          dfs.security.namenode.xframe-options=SAMEORIGIN
          

          The filter initializer for HDFS would need to check whether it was enabled and if so what the global value is.
          Then check and see whether it is overridden by a dfs specific property.

          No configured value for either would result in DENY whenever the filter is enabled.

          Show
          lmccay Larry McCay added a comment - So the following config would turn on XFS protection and set the global value to DENY - not setting the value would also since it is the default: hadoop.security.xframe-options-enabled= true hadoop.security.xframe-options=DENY The following additional config would override the value: dfs.security.namenode.xframe-options=SAMEORIGIN The filter initializer for HDFS would need to check whether it was enabled and if so what the global value is. Then check and see whether it is overridden by a dfs specific property. No configured value for either would result in DENY whenever the filter is enabled.
          Hide
          lmccay Larry McCay added a comment -

          Quick status: I am trying to write tests for this filter without Servlet 3 additions to HttpServletResponse for getting headers, headernames, etc. I should have a patch at some point this week.

          Show
          lmccay Larry McCay added a comment - Quick status: I am trying to write tests for this filter without Servlet 3 additions to HttpServletResponse for getting headers, headernames, etc. I should have a patch at some point this week.
          Hide
          lmccay Larry McCay added a comment -

          initial patch

          Show
          lmccay Larry McCay added a comment - initial patch
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 37s trunk passed
          +1 compile 9m 7s trunk passed with JDK v1.8.0_77
          +1 compile 8m 10s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 1m 5s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 1m 45s trunk passed
          +1 javadoc 1m 6s trunk passed with JDK v1.8.0_77
          +1 javadoc 1m 14s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 45s the patch passed
          +1 compile 9m 6s the patch passed with JDK v1.8.0_77
          +1 javac 9m 6s the patch passed
          +1 compile 8m 0s the patch passed with JDK v1.7.0_95
          +1 javac 8m 0s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 1m 1s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 5s the patch passed
          +1 javadoc 1m 8s the patch passed with JDK v1.8.0_77
          +1 javadoc 1m 11s the patch passed with JDK v1.7.0_95
          +1 unit 10m 30s hadoop-common in the patch passed with JDK v1.8.0_77.
          +1 unit 10m 17s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 25s Patch does not generate ASF License warnings.
          77m 22s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799880/HADOOP-13008-001.patch
          JIRA Issue HADOOP-13008
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c03032922922 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 33fd95a
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9136/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9136/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 37s trunk passed +1 compile 9m 7s trunk passed with JDK v1.8.0_77 +1 compile 8m 10s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 25s trunk passed +1 mvnsite 1m 5s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 45s trunk passed +1 javadoc 1m 6s trunk passed with JDK v1.8.0_77 +1 javadoc 1m 14s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 45s the patch passed +1 compile 9m 6s the patch passed with JDK v1.8.0_77 +1 javac 9m 6s the patch passed +1 compile 8m 0s the patch passed with JDK v1.7.0_95 +1 javac 8m 0s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 1m 1s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 5s the patch passed +1 javadoc 1m 8s the patch passed with JDK v1.8.0_77 +1 javadoc 1m 11s the patch passed with JDK v1.7.0_95 +1 unit 10m 30s hadoop-common in the patch passed with JDK v1.8.0_77. +1 unit 10m 17s hadoop-common in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 77m 22s Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799880/HADOOP-13008-001.patch JIRA Issue HADOOP-13008 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c03032922922 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 33fd95a Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9136/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9136/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          Chris Nauroth - would you be able to give this patch a review when you have a chance?
          It is a similar filter to the RestCsrfPreventionFilter added earlier in that it utilizes config prefixes to allow individual component UIs to override global settings.

          Show
          lmccay Larry McCay added a comment - Chris Nauroth - would you be able to give this patch a review when you have a chance? It is a similar filter to the RestCsrfPreventionFilter added earlier in that it utilizes config prefixes to allow individual component UIs to override global settings.
          Hide
          cnauroth Chris Nauroth added a comment -

          Hello Larry McCay. This looks good. Here are just a few comments:

          1. I think for completeness, there are a few other relevant methods that XFrameOptionsResponseWrapper needs to override: addDateHeader, addIntHeader, setDateHeader and setIntHeader. All of those should disallow altering X-Frame-Options.
          2. Check indentation level on the super call here.
                public XFrameOptionsResponseWrapper(HttpServletResponse response) {
                    super(response);
                }
            
          3. I generally prefer that tests just let exceptions propagate instead of catching and calling fail, unless the test specifically covers an error case and needs to verify the right kind of exception was thrown. If there is a test failure, letting the exception propagate will show the full stack trace in the JUnit report, and that's often helpful for diagnosis.
          Show
          cnauroth Chris Nauroth added a comment - Hello Larry McCay . This looks good. Here are just a few comments: I think for completeness, there are a few other relevant methods that XFrameOptionsResponseWrapper needs to override: addDateHeader , addIntHeader , setDateHeader and setIntHeader . All of those should disallow altering X-Frame-Options. Check indentation level on the super call here. public XFrameOptionsResponseWrapper(HttpServletResponse response) { super (response); } I generally prefer that tests just let exceptions propagate instead of catching and calling fail , unless the test specifically covers an error case and needs to verify the right kind of exception was thrown. If there is a test failure, letting the exception propagate will show the full stack trace in the JUnit report, and that's often helpful for diagnosis.
          Hide
          lmccay Larry McCay added a comment -

          Thanks, Chris Nauroth!
          I'll take care of those and provide a new revision.

          Show
          lmccay Larry McCay added a comment - Thanks, Chris Nauroth ! I'll take care of those and provide a new revision.
          Hide
          lmccay Larry McCay added a comment -

          Hi Appy - I'd really like to make sure that this patch addresses the usecase/s that you were targeting for HADOOP-12234. If you have a chance can you please take a look?

          I'll be providing a new version to address review comments but it will be functionally and configurationally the same.

          Show
          lmccay Larry McCay added a comment - Hi Appy - I'd really like to make sure that this patch addresses the usecase/s that you were targeting for HADOOP-12234 . If you have a chance can you please take a look? I'll be providing a new version to address review comments but it will be functionally and configurationally the same.
          Hide
          lmccay Larry McCay added a comment -

          v002 addresses Chris Nauroth's review comments.

          Show
          lmccay Larry McCay added a comment - v002 addresses Chris Nauroth 's review comments.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for patch v002, pending another pre-commit run. Larry McCay, thank you for the patch.

          Show
          cnauroth Chris Nauroth added a comment - +1 for patch v002, pending another pre-commit run. Larry McCay , thank you for the patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 8s trunk passed
          +1 compile 6m 31s trunk passed with JDK v1.8.0_91
          +1 compile 7m 16s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 1m 2s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 44s trunk passed
          +1 javadoc 0m 57s trunk passed with JDK v1.8.0_91
          +1 javadoc 1m 10s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 48s the patch passed
          +1 compile 6m 15s the patch passed with JDK v1.8.0_91
          +1 javac 6m 15s the patch passed
          +1 compile 6m 35s the patch passed with JDK v1.7.0_95
          +1 javac 6m 35s the patch passed
          -1 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 1 new + 173 unchanged - 0 fixed = 174 total (was 173)
          +1 mvnsite 0m 56s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 49s the patch passed
          +1 javadoc 0m 53s the patch passed with JDK v1.8.0_91
          +1 javadoc 1m 4s the patch passed with JDK v1.7.0_95
          +1 unit 7m 39s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 7m 54s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          62m 40s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802695/HADOOP-13008-002.patch
          JIRA Issue HADOOP-13008
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 29a78fefb4a9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 2835f14
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9307/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9307/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9307/console
          Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 8s trunk passed +1 compile 6m 31s trunk passed with JDK v1.8.0_91 +1 compile 7m 16s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 23s trunk passed +1 mvnsite 1m 2s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 44s trunk passed +1 javadoc 0m 57s trunk passed with JDK v1.8.0_91 +1 javadoc 1m 10s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 48s the patch passed +1 compile 6m 15s the patch passed with JDK v1.8.0_91 +1 javac 6m 15s the patch passed +1 compile 6m 35s the patch passed with JDK v1.7.0_95 +1 javac 6m 35s the patch passed -1 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 1 new + 173 unchanged - 0 fixed = 174 total (was 173) +1 mvnsite 0m 56s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 49s the patch passed +1 javadoc 0m 53s the patch passed with JDK v1.8.0_91 +1 javadoc 1m 4s the patch passed with JDK v1.7.0_95 +1 unit 7m 39s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 7m 54s hadoop-common in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 62m 40s Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802695/HADOOP-13008-002.patch JIRA Issue HADOOP-13008 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 29a78fefb4a9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2835f14 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9307/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9307/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9307/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          Hello Larry McCay. There are a few more nitpicky things I forgot in my last review:

          1. Please add the visibility annotations @InterfaceAudience.Public and @InterfaceStability.Evolving to XFrameOptionsFilter.
          2. To satisfy Checkstyle, let's add a basic package-info.java file. This isn't a problem introduced by the current patch, but there are public classes in there, so it would be nice to have the package-info.java.
          Show
          cnauroth Chris Nauroth added a comment - Hello Larry McCay . There are a few more nitpicky things I forgot in my last review: Please add the visibility annotations @InterfaceAudience.Public and @InterfaceStability.Evolving to XFrameOptionsFilter . To satisfy Checkstyle, let's add a basic package-info.java file. This isn't a problem introduced by the current patch, but there are public classes in there, so it would be nice to have the package-info.java.
          Hide
          lmccay Larry McCay added a comment -

          Will do - thanks, Chris Nauroth.

          Show
          lmccay Larry McCay added a comment - Will do - thanks, Chris Nauroth .
          Hide
          lmccay Larry McCay added a comment -

          v003 addresses Chris Nauroth's review comments.

          Show
          lmccay Larry McCay added a comment - v003 addresses Chris Nauroth 's review comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 25s trunk passed
          +1 compile 9m 8s trunk passed with JDK v1.8.0_91
          +1 compile 8m 48s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 1m 13s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 1m 56s trunk passed
          +1 javadoc 1m 11s trunk passed with JDK v1.8.0_91
          +1 javadoc 1m 20s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 52s the patch passed
          +1 compile 9m 3s the patch passed with JDK v1.8.0_91
          +1 javac 9m 3s the patch passed
          +1 compile 8m 50s the patch passed with JDK v1.7.0_95
          +1 javac 8m 50s the patch passed
          -1 checkstyle 0m 34s hadoop-common-project/hadoop-common: The patch generated 11 new + 173 unchanged - 0 fixed = 184 total (was 173)
          +1 mvnsite 1m 16s the patch passed
          +1 mvneclipse 0m 17s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 38s the patch passed
          +1 javadoc 1m 10s the patch passed with JDK v1.8.0_91
          +1 javadoc 1m 16s the patch passed with JDK v1.7.0_95
          +1 unit 10m 33s hadoop-common in the patch passed with JDK v1.8.0_91.
          -1 unit 19m 5s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 28s The patch does not generate ASF License warnings.
          90m 22s



          Reason Tests
          JDK v1.7.0_95 Failed junit tests hadoop.ipc.TestRPCWaitForProxy
            hadoop.ipc.TestRPC
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803040/HADOOP-13008-003.patch
          JIRA Issue HADOOP-13008
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 49550f125cc3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 411fb4b
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/console
          Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 25s trunk passed +1 compile 9m 8s trunk passed with JDK v1.8.0_91 +1 compile 8m 48s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 13s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 1m 56s trunk passed +1 javadoc 1m 11s trunk passed with JDK v1.8.0_91 +1 javadoc 1m 20s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 52s the patch passed +1 compile 9m 3s the patch passed with JDK v1.8.0_91 +1 javac 9m 3s the patch passed +1 compile 8m 50s the patch passed with JDK v1.7.0_95 +1 javac 8m 50s the patch passed -1 checkstyle 0m 34s hadoop-common-project/hadoop-common: The patch generated 11 new + 173 unchanged - 0 fixed = 184 total (was 173) +1 mvnsite 1m 16s the patch passed +1 mvneclipse 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 38s the patch passed +1 javadoc 1m 10s the patch passed with JDK v1.8.0_91 +1 javadoc 1m 16s the patch passed with JDK v1.7.0_95 +1 unit 10m 33s hadoop-common in the patch passed with JDK v1.8.0_91. -1 unit 19m 5s hadoop-common in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 28s The patch does not generate ASF License warnings. 90m 22s Reason Tests JDK v1.7.0_95 Failed junit tests hadoop.ipc.TestRPCWaitForProxy   hadoop.ipc.TestRPC JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803040/HADOOP-13008-003.patch JIRA Issue HADOOP-13008 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 49550f125cc3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 411fb4b Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9341/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          Patch v003 looks good. I think we'll be all set after one more revision to fix the last round of Checkstyle crankiness.

          Show
          cnauroth Chris Nauroth added a comment - Patch v003 looks good. I think we'll be all set after one more revision to fix the last round of Checkstyle crankiness.
          Hide
          lmccay Larry McCay added a comment -

          Are the rules changing out from under me?
          I ran checkstyle and those didn't show up.

          Show
          lmccay Larry McCay added a comment - Are the rules changing out from under me? I ran checkstyle and those didn't show up.
          Hide
          cnauroth Chris Nauroth added a comment -

          Are the rules changing out from under me?

          Hmmm... I don't see any recent changes in the Checkstyle rule set. I tried applying patch v003 locally, running mvn -o clean checkstyle:checkstyle, and then looking at target/site/checkstyle.html. I saw the same results as reported by pre-commit for the files in this patch.

          I don't know what happened. Certainly there are times that I would take great joy in dropkicking Checkstyle, but it seems fine for me this time.

          Show
          cnauroth Chris Nauroth added a comment - Are the rules changing out from under me? Hmmm... I don't see any recent changes in the Checkstyle rule set. I tried applying patch v003 locally, running mvn -o clean checkstyle:checkstyle , and then looking at target/site/checkstyle.html. I saw the same results as reported by pre-commit for the files in this patch. I don't know what happened. Certainly there are times that I would take great joy in dropkicking Checkstyle, but it seems fine for me this time.
          Hide
          lmccay Larry McCay added a comment -

          v004 fixes the checkstyle issues that are now being reported as well as those that weren't reported for the TestRestCsrfPreventionFilter during it's precommit checks.

          Show
          lmccay Larry McCay added a comment - v004 fixes the checkstyle issues that are now being reported as well as those that weren't reported for the TestRestCsrfPreventionFilter during it's precommit checks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 45s trunk passed
          +1 compile 6m 13s trunk passed with JDK v1.8.0_91
          +1 compile 7m 1s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 1m 3s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 41s trunk passed
          +1 javadoc 0m 58s trunk passed with JDK v1.8.0_91
          +1 javadoc 1m 6s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 43s the patch passed
          +1 compile 6m 22s the patch passed with JDK v1.8.0_91
          +1 javac 6m 22s the patch passed
          +1 compile 6m 53s the patch passed with JDK v1.7.0_95
          +1 javac 6m 53s the patch passed
          -1 checkstyle 0m 29s hadoop-common-project/hadoop-common: The patch generated 1 new + 174 unchanged - 19 fixed = 175 total (was 193)
          +1 mvnsite 0m 58s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 48s the patch passed
          +1 javadoc 0m 54s the patch passed with JDK v1.8.0_91
          +1 javadoc 1m 3s the patch passed with JDK v1.7.0_95
          +1 unit 7m 36s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 7m 42s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          61m 51s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803089/HADOOP-13008-004.patch
          JIRA Issue HADOOP-13008
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 72ae71f9dfcb 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 996a210
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9345/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9345/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9345/console
          Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 45s trunk passed +1 compile 6m 13s trunk passed with JDK v1.8.0_91 +1 compile 7m 1s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 27s trunk passed +1 mvnsite 1m 3s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 41s trunk passed +1 javadoc 0m 58s trunk passed with JDK v1.8.0_91 +1 javadoc 1m 6s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 43s the patch passed +1 compile 6m 22s the patch passed with JDK v1.8.0_91 +1 javac 6m 22s the patch passed +1 compile 6m 53s the patch passed with JDK v1.7.0_95 +1 javac 6m 53s the patch passed -1 checkstyle 0m 29s hadoop-common-project/hadoop-common: The patch generated 1 new + 174 unchanged - 19 fixed = 175 total (was 193) +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 54s the patch passed with JDK v1.8.0_91 +1 javadoc 1m 3s the patch passed with JDK v1.7.0_95 +1 unit 7m 36s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 7m 42s hadoop-common in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 61m 51s Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803089/HADOOP-13008-004.patch JIRA Issue HADOOP-13008 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 72ae71f9dfcb 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 996a210 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9345/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9345/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9345/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          The 1 remaining checkstyle issue is for the package-info.java class not having a javadoc comment which is the same as all (or at least most) of the other package-info.java classes that I can see.

          Show
          lmccay Larry McCay added a comment - The 1 remaining checkstyle issue is for the package-info.java class not having a javadoc comment which is the same as all (or at least most) of the other package-info.java classes that I can see.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for patch v004. I have committed this to trunk, branch-2 and branch-2.8. Larry, thank you for the patch.

          Show
          cnauroth Chris Nauroth added a comment - +1 for patch v004. I have committed this to trunk, branch-2 and branch-2.8. Larry, thank you for the patch.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9746 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9746/)
          HADOOP-13008. Add XFS Filter for UIs to Hadoop Common. Contributed by (cnauroth: rev dee279b532e7286362518b531c9daea9ae8606f4)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestXFrameOptionsFilter.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/XFrameOptionsFilter.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/package-info.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9746 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9746/ ) HADOOP-13008 . Add XFS Filter for UIs to Hadoop Common. Contributed by (cnauroth: rev dee279b532e7286362518b531c9daea9ae8606f4) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestXFrameOptionsFilter.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/XFrameOptionsFilter.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/package-info.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java
          Hide
          vvasudev Varun Vasudev added a comment -

          Larry McCay, Chris Nauroth - HADOOP-12964 has also added XFS support. There seems to some amount of duplicate code between the two issues. Can you please let me know if this is intended?

          Show
          vvasudev Varun Vasudev added a comment - Larry McCay , Chris Nauroth - HADOOP-12964 has also added XFS support. There seems to some amount of duplicate code between the two issues. Can you please let me know if this is intended?
          Hide
          lmccay Larry McCay added a comment -

          Varun Vasudev - Thank you for bring this to my attention!

          This effort was certainly not intended to duplicate any other work - in fact, I went to some length to make sure that I didn't do so with HADOOP-12234.

          I was unaware of the inner QuotingInputFilter class within HttpServer2 or the fact that it also adds X-Frame-Options.

          The fact that it is baked into the HttpServer2 class rather than commonly available for anyone to use and that it doesn't separate the responsibility for XFS make that filter less reusable by the overall ecosystem.

          My inclination is to refactor the functionality in QuotingIinputFilter out into a generic XSS filter that can be reused by others and to integrate with it and the common XFS filter rather than relying on HttpServer2 specific filters.

          Thoughts?

          Show
          lmccay Larry McCay added a comment - Varun Vasudev - Thank you for bring this to my attention! This effort was certainly not intended to duplicate any other work - in fact, I went to some length to make sure that I didn't do so with HADOOP-12234 . I was unaware of the inner QuotingInputFilter class within HttpServer2 or the fact that it also adds X-Frame-Options. The fact that it is baked into the HttpServer2 class rather than commonly available for anyone to use and that it doesn't separate the responsibility for XFS make that filter less reusable by the overall ecosystem. My inclination is to refactor the functionality in QuotingIinputFilter out into a generic XSS filter that can be reused by others and to integrate with it and the common XFS filter rather than relying on HttpServer2 specific filters. Thoughts?
          Hide
          vvasudev Varun Vasudev added a comment - - edited

          I prefer the generic XFS filter based approach to the QuotingInputFilter - it's more flexible. The only reason I realized this is that I was testing a patch for YARN integration and noticed that the header was being set for all responses and I wasn't sure why. I defer to Chris and you on what to do going forward.

          Show
          vvasudev Varun Vasudev added a comment - - edited I prefer the generic XFS filter based approach to the QuotingInputFilter - it's more flexible. The only reason I realized this is that I was testing a patch for YARN integration and noticed that the header was being set for all responses and I wasn't sure why. I defer to Chris and you on what to do going forward.
          Hide
          cnauroth Chris Nauroth added a comment -

          Larry McCay, the proposed refactoring sounds good to me. Varun Vasudev, thank you for pointing out the duplication.

          Show
          cnauroth Chris Nauroth added a comment - Larry McCay , the proposed refactoring sounds good to me. Varun Vasudev , thank you for pointing out the duplication.
          Hide
          lmccay Larry McCay added a comment -

          Varun Vasudev - have you filed a JIRA for this refactoring and do you intend to take it on?

          Show
          lmccay Larry McCay added a comment - Varun Vasudev - have you filed a JIRA for this refactoring and do you intend to take it on?
          Hide
          vvasudev Varun Vasudev added a comment -

          Larry McCay - sorry I haven't filed a JIRA; I wasn't planning to work on the issue.

          Show
          vvasudev Varun Vasudev added a comment - Larry McCay - sorry I haven't filed a JIRA; I wasn't planning to work on the issue.
          Hide
          lmccay Larry McCay added a comment -

          Chris Nauroth - I don't think that this was ever committed to branch-2.8.
          Is there some reason to not do so?

          Show
          lmccay Larry McCay added a comment - Chris Nauroth - I don't think that this was ever committed to branch-2.8. Is there some reason to not do so?
          Hide
          cnauroth Chris Nauroth added a comment -

          Hello Larry McCay. I see this in branch-2.8 as commit e9ee258d04705f71c62c4d585686ad0eec47d8f4.

          commit e9ee258d04705f71c62c4d585686ad0eec47d8f4
          Author: Chris Nauroth <cnauroth@apache.org>
          Date:   Wed May 11 10:58:32 2016 -0700
          
              HADOOP-13008. Add XFS Filter for UIs to Hadoop Common. Contributed by Larry McCay.
              
              (cherry picked from commit dee279b532e7286362518b531c9daea9ae8606f4)
              (cherry picked from commit 31279ae45eeb74d0955014ceffacb5c40d2f3ee5)
          
          Show
          cnauroth Chris Nauroth added a comment - Hello Larry McCay . I see this in branch-2.8 as commit e9ee258d04705f71c62c4d585686ad0eec47d8f4. commit e9ee258d04705f71c62c4d585686ad0eec47d8f4 Author: Chris Nauroth <cnauroth@apache.org> Date: Wed May 11 10:58:32 2016 -0700 HADOOP-13008. Add XFS Filter for UIs to Hadoop Common. Contributed by Larry McCay. (cherry picked from commit dee279b532e7286362518b531c9daea9ae8606f4) (cherry picked from commit 31279ae45eeb74d0955014ceffacb5c40d2f3ee5)
          Hide
          lmccay Larry McCay added a comment -

          Oh, thanks, Chris Nauroth!
          I couldn't find it for some reason.

          Show
          lmccay Larry McCay added a comment - Oh, thanks, Chris Nauroth ! I couldn't find it for some reason.

            People

            • Assignee:
              lmccay Larry McCay
              Reporter:
              lmccay Larry McCay
            • Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development