feat: customizable token prefix

[mattslocum]

matching Hydra PR: https://github.com/ory/hydra/pull/3407

https://github.com/ory/fosite/pull/675 added ory_at|pt|ac prefixes to HMAC tokens. This is a good feature for detecting tokens in places they shouldn't exist. However, for security minded self-hosted implementations, it is often preferred to avoid underlying tooling providers. In this case, it is preferrable to be able to customize the ory name and replace it with another key.

This PR adds ability to customize the token prefix before at|pt|ac.

Related Issue or Design Document

Related to https://github.com/ory/fosite/pull/675 and https://github.com/ory/hydra/issues/2845.

Checklist

• [x] I have read the contributing guidelines and signed the CLA.
• [x] I have referenced an issue containing the design document if my change introduces a new feature.
• [x] I have read the security policy.
• [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
• [x] I have added tests that prove my fix is effective or that my feature works.
• [x] I have added the necessary documentation within the code base (if appropriate).

Further comments

For backwards compatibility for integrations that already upgraded, it continues to remove the ory specific prefix when trimming.

[james-d-elliott]

Wouldn't this be easier via a newup function which sets the prefix field then provide the result as the core strategy?

[mattslocum]

Wouldn't this be easier via a newup function which sets the prefix field then provide the result as the core strategy?

Yeah, I agree that it seems like it should be easier. I was trying to match what I perceived as the pattern I found in https://github.com/ory/fosite/blob/master/config.go and https://github.com/ory/fosite/blob/master/config_default.go, and then I chased a couple of those examples and tried to mimic them. Can you expound on the better way or is there a similar example? If you are referring to the strategy_hmacsha.go change only, then I can see how that is odd. @aeneas made the original change that added the private field, and I was struggling to figure out the full purpose. https://github.com/ory/fosite/commit/b652335c965d5cc523faebad9c9792c4135cfb75#diff-180026ce292f76d8c418fa1d6c09ef637ffd461c5772ec9441b1131642965f6f But I'm happy to refactor that field to be a different style.

[james-d-elliott]

Yeah it makes sense why you did it. I think the intention in the pattern was primarily for hot reloading, whereas this probably isn't something that would benefit much from it. We'll see what Mr Hackerman thinks and if he has a preference.

[aeneasr]

Hi, thank you for the PR and inititative!

Unfortunately, we don't plan on changing this. The point of token prefixes is to allow security scanners to easily scan code for leaked credentials. If everyone does their own token prefix, it makes the whole software ecosystem more vulnerable. Any token leaks will stay undetected if every site decides to use their own token prefixes. Currently, there is no standard for the token prefix format agreed upon (e.g. in an RFC). What I'm open to consider is adding a suffix to the prefix to allow big sites (assuming e.g. github wants to use this) to uniquely identify the token source, which would make it easier for scanners to tell you the token source ory_at_github_...

[aeneasr]

Whoops, pressed enter a bit too soon :) Also wanted to say thank you for the PR and contribution! It always sucks when a feature is not accepted upstream, but I hope the explanation makes sense!

[vivshankar]

@aeneasr I know you have previously stated that Fosite is only meant to be used with Hydra but that wasn't my understanding when we adopted it for our own implementation. I believe there are others in the same boat.

Would you accept a change where the prefix can be set globally - maybe just a change from setPrefix to SetPrefix to let it be set over a public function? It doesn't make sense for our tokens to have a "ory_" prefix.

The alternative is for us to effectively clone the HMACStrategy and write our own without the prefix and that was my plan until this PR was submitted.

[mattslocum]

@aeneasr Thank you for responding. A few thoughts

1. "If everyone does their own token prefix, it makes the whole software ecosystem more vulnerable." I know you pointed out the absence of an RFC standard on prefixes. But if Google, Azure, Okta, JumpCloud, Ory, Github are all doing their own prefix isn't this the same thing and there is already a scan-ability problem? There may be hundreds or thousands of oauth vendors already. So how does putting ory_ in front help those other providers since they will be doing a different prefix?
2. I'm in the self hosted space with Hydra, so we consider our tokens to be our concern with our own scanners. I wouldn't expect this prefix to be customizable on your enterprise hosted offering, but it would be nice on the Apache2.0 licensing side.
3. We also have a concern about sharing underlying tech in an event of a CVE or any extra info for attack vectors. We try to prevent disclosure of internal tooling and our use of Hydra/Fosite is behind the scenes. Adding ory_ to our tokens might limit our ability to upgrade to Hydra2 because our Security team might have issues with it.

I understand that I might not change your mind on this. Thank you for your time either way.

[K3das]

Though I don't think it's feasible to try to hide your usage of hydra, I do agree with @mattslocum, especially considering that services reporting secret leaks will only report to ory, which isn't at all helpful in the case of a self-hosted hydra, where you'd want those services reporting to you...

[mitar]

I also opened #789 for this. Missed this PR. I am also +1 for having this be configurable. There are also +9 upvotes on this PR. @aeneasr please reconsider this.

[OffensiveBias-08-145]

@aeneasr

@K3das and @mattslocum both make valid points.

This has been a point of contention within our organization in order to adopt Ory fully. Limiting the breadth of information providing potential attack vectors is just good security posture.

It is not clear to me how having different prefixes makes the software ecosystem more vulnerable? Organizations with good security posture are already doing their own internal scans for secret leaks.

From an attackers standpoint, the ory specific prefix is a gift. I now know that I do not need to waste time determining which Auth/Authz service is being used since information is handed to me in a pretty bow.

At a minimum an optional config should be available. This would ease the concerns from Security Teams and allow further adoption. If not, I foresee this will be a limiting factor for orgs to adopt Ory.