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

Don't use non-standard auth scheme #247

Closed
shirhatti opened this issue Apr 27, 2021 · 1 comment · Fixed by #463
Closed

Don't use non-standard auth scheme #247

shirhatti opened this issue Apr 27, 2021 · 1 comment · Fixed by #463
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@shirhatti
Copy link
Contributor

Originally posted by @idg10

This seems to be planning to use a non-standard scheme, MonitorApiToken for the Authorization HTTP header. According to the HTTP standard for authentication (RFC7235) the scheme is meant to be one of those registered in IANA's Hypertext Transfer Protocol (HTTP) Authentication Scheme Registry. (The alternative mechanism you mention, Negotiate, is registered with IANA. So the problem is just with the novel MonitorApiToken you've proposed.)

The first obvious question is: why aren't you just using Bearer? RFC6750 defines this scheme, and the operation of your proposed MonitorApiToken seems to be a reinvention of the existing Bearer token scheme but by another name.

There's a really good reason for using the existing standard Bearer scheme: bearer tokens have certain risks attached to them. Because there's no proof of ownership mechanism, it's really important to ensure they are suitably protected. Since Bearer is a well-defined standard scheme, HTTP infrastructure (proxies, caching, APIs) can be built with a baked-in understanding of the risks, making it possible to impose policies that prevent accidental leaking of data. (E.g., an egress point could detect the use of a Bearer header in an unencrypted request and block it.) But by defining your own non-standard bearer token scheme, you will lose any such protection. This use of a non-standard security scheme introduces security risks, and for no obvious benefit.

Second, if you were to attempt to get IANA to register your new scheme, it would almost certainly be rejected because it violates the third bullet point in RFC7235's 'Considerations for New Authentication Schemes'. You are in effect using the token68 production in your header here. This is exactly what Bearer does, but the Bearer scheme is allowed to get away with this because, as that section of RFC7235 says:

The "token68" notation was introduced for compatibility with existing authentication schemes

I.e., Bearer already worked this way, and it's regrettable, but too late to change, so the current spec makes a special case for it. But as it says:

new schemes ought to use the auth-param syntax instead

Your proposed MonitorApiToken scheme is definitely "new" so it should use the auth-param syntax defined in RFC7235's Challenge and Response section.

@shirhatti shirhatti added the enhancement New feature or request label Apr 27, 2021
@kelltrick kelltrick added this to the Preview 6 milestone Apr 27, 2021
@kelltrick kelltrick self-assigned this Jun 7, 2021
@kelltrick
Copy link
Contributor

I suggest we switch to using Bearer over MonitorApiKey.

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

Successfully merging a pull request may close this issue.

3 participants