Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace SameFileAliasChecker #6497

Closed
joakime opened this issue Jul 7, 2021 · 6 comments · Fixed by #6681
Closed

Replace SameFileAliasChecker #6497

joakime opened this issue Jul 7, 2021 · 6 comments · Fixed by #6681
Assignees
@lachlan-roberts
Labels
Enhancement

Comments

@joakime
Copy link
Contributor

joakime commented Jul 7, 2021

Target Jetty version(s)
9.4.x+

Enhancement Description
The current SameFileAliasChecker is almost a no-op based on changes before it is called.

We should address the 2 common usages.

  1. Case Insensitive aliases - for windows requests (ala /index.html vs /INDEX.HTML)
  2. FileSystem Normalization - for OSX and ZFS based filessytems that treat UTF-8 encoding differently on request vs storage.
@lachlan-roberts
Copy link
Contributor

The documentation should be also updated to explain why you should not use SameFileAliasChecker or ApproveAliases unless you are sure of what you are doing.

@lachlan-roberts lachlan-roberts self-assigned this Jul 9, 2021
lachlan-roberts added a commit that referenced this issue Jul 9, 2021
@lachlan-roberts
Issue #6497 - Implement the AllowedResourceAliasChecker to improve Sa…
a1bb8f3 
…meFileAliasChecker

Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 13, 2021
@lachlan-roberts
Issue #6497 - add testing for AliasCheckers with symlinks
32ae130 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 13, 2021
@lachlan-roberts
Issue #6497 - AllowedResourceAliasChecker can test target of symlink
be8c6ac 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 13, 2021
@lachlan-roberts
Issue #6497 - improve testing for AliasCheck with symlinks
9d5ea7e 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 13, 2021
@lachlan-roberts
Issue #6497 - improve testing for AliasCheck with symlinks
70f36ef 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 23, 2021
@lachlan-roberts
Issue #6497 - improve testing for AliasCheck with symlinks
7c0c3c9 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts mentioned this issue Jul 26, 2021
Closed
lachlan-roberts added a commit that referenced this issue Jul 27, 2021
@lachlan-roberts
Issue #6497 - Improve comments, re-implement the hasSymbolicLink method.
68d93fb 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 30, 2021
@lachlan-roberts
Issue #6497 - Make AllowedResourceAliasChecker a LifeCycle
a7639bc 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 30, 2021
@lachlan-roberts
Issue #6497 - Fix AllowedResourceAliasChecker if protectedTarget does…
32bff77 
… not exist

Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Jul 30, 2021
@lachlan-roberts
Issue #6497 - aliasChecker with no baseResource should always denies …
bf078be 
…aliases

Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Aug 26, 2021
@lachlan-roberts
Issue #6497 - Replace the Alias checkers with new implementation.
9ef0a36 
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Aug 26, 2021
@lachlan-roberts
Issue #6497 - Replace the Alias checkers with new implementation.
c110648 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts mentioned this issue Aug 26, 2021
Closed
gregw added a commit that referenced this issue Aug 30, 2021
@gregw
Alternate fix for #6497
8cb802f 
Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Aug 30, 2021
@gregw
Alternate fix for #6497
a51e570 
Signed-off-by: Greg Wilkins <[email protected]>
@gregw gregw mentioned this issue Aug 30, 2021
Merged
gregw added a commit that referenced this issue Aug 30, 2021
@gregw
Alternate fix for #6497
c29ba3e 
Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Aug 30, 2021
@gregw
Alternate fix for #6497
497687d 
Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Aug 30, 2021
@gregw
Alternate fix for #6497
48880c6 
Signed-off-by: Greg Wilkins <[email protected]>
@gregw gregw linked a pull request Aug 31, 2021 that will close this issue
Jetty 10.0.x 6497 alias checkers alt #6681
Merged
gregw added a commit that referenced this issue Sep 21, 2021
@gregw @lachlan-roberts
Jetty 10.0.x 6497 alias checkers alt (#6681)
aa793ee 
* Issue #6497 - Replace the Alias checkers with new implementation.

Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Greg Wilkins <[email protected]>
Co-authored-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
Copy link
Contributor

I think this needs to be done in 9.4 as well.

@joakime joakime linked a pull request Sep 22, 2021 that will close this issue
Issue #6497 - Replace the Alias checkers with new implementation. (Jetty-9.4) #6540
Closed
@joakime
Copy link
Contributor Author

joakime commented Sep 22, 2021

This is fully merged in 9.4.x and 10.0.x onwards

@joakime joakime closed this as completed Sep 22, 2021
@lachlan-roberts
Copy link
Contributor

Has not been ported to 9.4.x yet.

@joakime
Copy link
Contributor Author

joakime commented Sep 22, 2021

Has not been ported to 9.4.x yet.

Then what was PR #6540 for?

@lachlan-roberts
Copy link
Contributor

That was the original PR that was for 9.4.x, however it was not merged and was closed. We instead developed a fix for the 10.0.x/11.0.x branches (#6681) and the plan was to port a minimal fix back to 9.4.x.

lachlan-roberts added a commit that referenced this issue Oct 12, 2021
@lachlan-roberts
Backport 6497 alias checkers alt (#6681)
7f94ec6 
* Issue #6497 - Replace the Alias checkers with new implementation.

Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Greg Wilkins <[email protected]>
Co-authored-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Oct 12, 2021
@lachlan-roberts
Issue #6497 - do not allow file separator chars other than / in alias…
401472f 
… uri

Signed-off-by: Lachlan Roberts <[email protected]>
lachlan-roberts added a commit that referenced this issue Oct 12, 2021
@lachlan-roberts
Issue #6497 - warn on use of AllowSymLinkAliasChecker
12ee696 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts mentioned this issue Oct 12, 2021
Merged
lachlan-roberts added a commit that referenced this issue Oct 15, 2021
@lachlan-roberts
Issue #6497 - add warnings for alias checker deprecations
4682e63 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts mentioned this issue Oct 15, 2021
Merged
lachlan-roberts added a commit that referenced this issue Oct 17, 2021
@lachlan-roberts
Merge pull request #6998 from eclipse/jetty-10.0.x-6497-AliasCheckWar…
2095fb0 
…nings

Issue #6497 - add warnings for alias checker deprecations
@joakime joakime mentioned this issue Oct 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lachlan-roberts lachlan-roberts

Labels
Enhancement
Projects
None yet
Milestone
No milestone
2 participants
@joakime @lachlan-roberts