Details
Description
Currently, CrossOriginFilter.java limits regex matching only if there is an asterisk (*) in the config.
if (allowedOrigin.contains("*")) {
This means that entries such as:
http?://foo.example.com https://[a-z][0-9].example.com
... and other patterns that succinctly limit the input space need to either be fully expanded or dramatically have their space increased by using an asterisk in order to pass through the filter.
Attachments
Attachments
- HADOOP-14908-PR279.patch
- 21 kB
- Johannes Alberti
Issue Links
Activity
GitHub user johannes-altiscale opened a pull request:
https://github.com/apache/hadoop/pull/278
(HADOOP-14908) allow for real regex patterns (and be backward compatible)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Altiscale/hadoop johannes-HADOOP-14908-allow-full-regexp
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/hadoop/pull/278.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #278
commit 90e3816f606bcccc495efa09cfb0f26c3a6d37ac
Author: Johannes Alberti <johannes@altiscale.com>
Date: 2017-09-27T01:50:10Z
allow for real regex patterns (and be backward compatible)
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 17s | Docker mode activated. |
Prechecks | |||
+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. |
trunk Compile Tests | |||
+1 | mvninstall | 14m 42s | trunk passed |
+1 | compile | 15m 14s | trunk passed |
+1 | checkstyle | 0m 37s | trunk passed |
+1 | mvnsite | 1m 3s | trunk passed |
+1 | shadedclient | 10m 39s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 30s | trunk passed |
+1 | javadoc | 0m 53s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 0m 42s | the patch passed |
+1 | compile | 11m 13s | the patch passed |
+1 | javac | 11m 13s | the patch passed |
-0 | checkstyle | 0m 37s | hadoop-common-project/hadoop-common: The patch generated 20 new + 21 unchanged - 0 fixed = 41 total (was 21) |
+1 | mvnsite | 0m 59s | the patch passed |
-1 | whitespace | 0m 0s | The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
+1 | shadedclient | 8m 43s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 31s | the patch passed |
+1 | javadoc | 0m 50s | the patch passed |
Other Tests | |||
+1 | unit | 8m 11s | hadoop-common in the patch passed. |
+1 | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
78m 20s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:71bbb86 |
JIRA Issue | |
GITHUB PR | https://github.com/apache/hadoop/pull/278 |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
uname | Linux c9584f794005 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 0da29cb |
Default Java | 1.8.0_144 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/13385/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt |
whitespace | https://builds.apache.org/job/PreCommit-HADOOP-Build/13385/artifact/patchprocess/whitespace-eol.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/13385/testReport/ |
modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/13385/console |
Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Added more tests and fixed whitespace issues, also updated documentation.
GitHub user johannes-altiscale opened a pull request:
https://github.com/apache/hadoop/pull/279
HADOOP-14908 allow regular expressions, fixes
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Altiscale/hadoop johannes-HADOOP-14908-allow-full-regexp
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/hadoop/pull/279.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #279
commit 90e3816f606bcccc495efa09cfb0f26c3a6d37ac
Author: Johannes Alberti <johannes@altiscale.com>
Date: 2017-09-27T01:50:10Z
allow for real regex patterns (and be backward compatible)
commit 6760ffa132446716471084d077511dcce81ca095
Author: Johannes Alberti <johannes@altiscale.com>
Date: 2017-09-27T16:33:24Z
(HADOOP-14908) white space issues, documentation updates, more tests
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 14s | Docker mode activated. |
Prechecks | |||
+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. |
trunk Compile Tests | |||
0 | mvndep | 1m 32s | Maven dependency ordering for branch |
+1 | mvninstall | 13m 33s | trunk passed |
+1 | compile | 14m 20s | trunk passed |
+1 | checkstyle | 2m 1s | trunk passed |
+1 | mvnsite | 1m 22s | trunk passed |
+1 | shadedclient | 14m 53s | branch has no errors when building and testing our client artifacts. |
0 | findbugs | 0m 0s | Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site |
+1 | findbugs | 1m 25s | trunk passed |
+1 | javadoc | 1m 14s | trunk passed |
Patch Compile Tests | |||
0 | mvndep | 0m 16s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 53s | the patch passed |
+1 | compile | 11m 12s | the patch passed |
+1 | javac | 11m 12s | the patch passed |
-0 | checkstyle | 2m 4s | root: The patch generated 24 new + 21 unchanged - 0 fixed = 45 total (was 21) |
+1 | mvnsite | 1m 27s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 1s | The patch has no ill-formed XML file. |
+1 | shadedclient | 10m 7s | patch has no errors when building and testing our client artifacts. |
0 | findbugs | 0m 0s | Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site |
+1 | findbugs | 1m 43s | the patch passed |
+1 | javadoc | 1m 18s | the patch passed |
Other Tests | |||
-1 | unit | 8m 49s | hadoop-common in the patch failed. |
+1 | unit | 0m 20s | hadoop-yarn-site in the patch passed. |
+1 | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
107m 29s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.security.TestRaceWhenRelogin |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:71bbb86 |
JIRA Issue | |
GITHUB PR | https://github.com/apache/hadoop/pull/279 |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux 952498d51c34 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 7c34cea |
Default Java | 1.8.0_144 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/13400/artifact/patchprocess/diff-checkstyle-root.txt |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/13400/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/13400/testReport/ |
modules | C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: . |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/13400/console |
Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
The original cross origin filter in hadoop was designed based on the apache license jetty cross origin filter (not available in the 6.x jetty line). This was done so that when jetty 9 was adopted in trunk we had an option to stop using the hadoop version and migrate to the jetty version very easily. Do we want to follow the jetty 9 capabilities for this plugin?
https://www.eclipse.org/jetty/documentation/9.4.x/cross-origin-filter.html
http://git.eclipse.org/c/jetty/org.eclipse.jetty.project.git/tree/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java
jeagles thanks for your comments, I took a quick look at the Jetty filter, and I can see, Jetty has the same issue, the support for wildcards alone can easily lead to security issues. Below code is from the Jetty filter ... and this 'greed' can lead to issues, very easily.
private String parseAllowedWildcardOriginToRegex(String allowedOrigin) { String regex = allowedOrigin.replace(".", "\\."); return regex.replace("*", ".*"); // we want to be greedy here to match multiple subdomains, thus we use .* }
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13013 (See https://builds.apache.org/job/Hadoop-trunk-Commit/13013/)
HADOOP-14908. CrossOriginFilter should trigger regex on more input (aw: rev 4d5dd75b607d25adf8b41f7408713dfcea8f5330)
- (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestCrossOriginFilter.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/TimelineServer.md
- (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/CrossOriginFilter.java
- (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
- (edit) hadoop-common-project/hadoop-common/src/site/markdown/HttpAuthentication.md
Github user johannes-altiscale commented on the issue:
https://github.com/apache/hadoop/pull/279
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13013 (See https://builds.apache.org/job/Hadoop-trunk-Commit/13013/)
There are likely a bunch of ways to solve this one. Off the top, I can think of three:
#1: always treat it as a regex
This is backwards incompatible, in the sense that periods are now wildcards and opens up the namespace on existing installations.
#2: Add additional triggers
It might simpler to just check for ? and [, but this will prevent character classes, boundary matches, and other "exotics" from being used.
#3: flag/config that says whether everything/always/etc should be used as a regex.
Personally, I'm leaning towards #1.