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.

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

case sensitive in jwtauthn match prefix #1633

Closed
Ruil1n opened this issue Apr 21, 2021 · 5 comments
Closed

case sensitive in jwtauthn match prefix #1633

Ruil1n opened this issue Apr 21, 2021 · 5 comments
Assignees

Comments

@Ruil1n
Copy link

Ruil1n commented Apr 21, 2021

Describe the bug

When using JWT authorization, it is case-sensitive to the prefix that the URL matches, which can lead to authentication bypassing in some scenarios.

It is recommended to apply URL normalization and employ case-insensitive comparison in order to eliminate the risk of potential access control list bypasses.

Expected behavior

Be intercepted by jwtauthn

Actual behavior

Bypass the intercept

Steps to reproduce

config.js

......
"rules": [
             {
                     "match": {
                     "prefix": "/index"
             },
......

NodeJS with Express

curl http://127.0.0.1:2046/index -s -o /dev/null  -H "Authorization: Bearer token" -w "%{http_code}\n" 
401
curl http://127.0.0.1:2046/indeX -s -o /dev/null  -H "Authorization: Bearer token" -w "%{http_code}\n"
200

Minimal yet complete reproducer code (or GitHub URL to code)

Environment

  • MOSN Version
@antJack
Copy link
Contributor

antJack commented Apr 23, 2021

According to RFC3986, only the host, sheme, and hexadecimal digits should be case-insensitive. The other generic syntax components are assumed to be case-sensitive.

So, by default, the prefix "/index" should not match with "/indeX"

@Ruil1n
Copy link
Author

Ruil1n commented Apr 23, 2021

Yes, that's true for RFC3986, but the conventions generally use all lowercase paths.

As MOSN is a general network proxy, I think its function of authenticating the matching path needs to support the differences of most applications to the greatest extent.

Personally, I suggest that case-insensitive matching be enabled by default, and users can configure it by themselves.

If the default is case-sensitive, it is best to remind users of possible risks.

@antJack
Copy link
Contributor

antJack commented Apr 23, 2021

Is there any documentation about your mentioned conventions?

I referred to the Envoy JWT Authentication module, but found that its match is also case-sensitive by default. Or am I wrong with some places?

@antJack
Copy link
Contributor

antJack commented Apr 23, 2021

And we do need to provide the ability to configure whether case-sensitive or not by users.

That's what @GLYASAI is doing right now in #1637

@Ruil1n
Copy link
Author

Ruil1n commented Apr 23, 2021

There may not be an explicit rule on this, I see in dapr

dapr/dapr#2768 https://github.com/dapr/dapr/blob/e3846f5f4126cc1e9fa782c638e10b23dbdf6fba/pkg/config/configuration.go#L558

They appear to be case insensitive after a security audit.

But now I think it's better to be consistent with the RFC and Envoy as you do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants