Configurable webhook header filter

[K3das]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

Ory Network Project

No response

Describe your problem

I talked about this a bit on slack, but the header filter implemented by #4048 is somewhat arbitrary and doesn't work for some configurations or use cases (for example, where you use x-forwarded-for instead of true-client-ip to get client IP, or if you need to process injected telemetry from a header).

Describe your ideal solution

Ideally there is a config value for allowed headers, possibly defaulting to the current list to avoid making significant changes.

Workarounds or alternatives

For getting the client IP, I've updated my proxy configuration to set True-Client-IP.

Version

v1.3.1

Additional Context

No response

[robert-sisutech]

I'm on Kratos v.1.3.0.

I'm absolutely gutted that this header filtering was added without the possibility to add more headers to the allowlist in Kratos configuration. Recently updated Kratos to the latest version in our system and it broke some functionalities which relied on specific headers existing on the webhook requests as part of Kratos flows. Cannot downgrade as we need some latest Kratos features.
Requests to our system go through a reverse-proxy which decorates the request with some extra headers (ip address, ip country, plus some more implementation specific headers). I know we could transform the headers but the strict headers allowlist makes this almost impossible.

What was the benefit of strict allowlist for webhook request headers?
Ideally yes there would be a way to allow more headers than the current strict headers allowlist.

[aeneasr]

For getting the client IP, I've updated my proxy configuration to set True-Client-IP.

The workaround is a good option in my view.

@robert-sisutech is that workaround not working for you?

We have removed other headers due to security issues in our infra. At the moment there are no plans to change this

[K3das]

Is there any reason why a config option wouldn't work? This seems like a very instance-specific thing.

[aeneasr]

Not really (other than adding yet more config options), but it’s not a priority given that a workaround exists and we have no need for it!

[K3das]

@aeneasr would this be a per-webhook or instance-wide config? Doing it instance-wide would probably be a slightly easier implementation but I could imagine a scenario where someone wants to be selective about what hooks get what headers.

[robert-sisutech]

@aeneasr we have like around 5 headers we require to be on the request and that we would need to be able to read in Kratos' webhook request handler.
Your suggestion is not a workaround for our usecase. Although hacky, yeah the True-Client-IP header is great and possible to do for us but we're missing bunch of other headers which we cannot re-assign to any of the headers in the strict allowlist.

Adding strict filtering of headers for everyone using Ory Kratos due to your infra's security concerns isn't a really thoughtful decision in my opinion. Many of us still need access to other headers thus why I and I imagine some more developers would want the headers allowlist to be configurable or at least it should be possible to disable the strict headers filtering.

[aeneasr]

@aeneasr we have like around 5 headers we require to be on the request and that we would need to be able to read in Kratos' webhook request handler. Your suggestion is not a workaround for our usecase. Although hacky, yeah the True-Client-IP header is great and possible to do for us but we're missing bunch of other headers which we cannot re-assign to any of the headers in the strict allowlist.

Adding strict filtering of headers for everyone using Ory Kratos due to your infra's security concerns isn't a really thoughtful decision in my opinion. Many of us still need access to other headers thus why I and I imagine some more developers would want the headers allowlist to be configurable or at least it should be possible to disable the strict headers filtering.

Did you see my last reply?

Regarding your message pre-edit: We are listening to OSS community demands but as you know running such a large project isn’t free and our priorities are with people who pay us to maintain and advance this project (otherwise it would be long dead). It’s even more frustrating when other companies use our stuff, demand a lot, make a lot of money with their customers, but don’t give back except frustrated messages. So please don’t get frustrated but stay constructive.

By the way, if you rely on that many headers, maybe transient payloads are a better option for you.

@aeneasr would this be a per-webhook or instance-wide config? Doing it instance-wide would probably be a slightly implementation but I could imagine a scenario where someone wants to be selective about what hooks get what headers.

Global should be fine, otherwise the config gets really convoluted. The biggest challenge is probably figuring out where to put the config key.

[robert-sisutech]

@aeneasr The aforementioned headers are something we don't have much control over. We could re-assign the headers but we cannot move them to transient payload while the request is in-flight. We don't add the headers ourself at the source of the request. The headers get added while the request is in flight in one part of our reverse-proxy/firewall/gateway of our infra. Transient payload is a good option if we can allow the data to be editable by end-user. In this case we don't want the end-user to have control of the data put in these headers so transient_payload unfortunately is not an option for us in this case. Transient payload however comes in very handy in some other flows we have.

I regret the tone in my pre-edit message. Edited it for a reason. I appreciate something like Ory Kratos exists and is open-source and available for everybody to use.

What would make the situation with strict headers allowlist better for some of us would be:

• flag in configuration to disable strict header filtering altogether (easier to implement)
• option in configuration to manually add more headers to the allowlist (bit more difficult to implement)

[aeneasr]

Apology accepted, no problem :)

I feel making the headers configurable strikes a balance between security and customizability and would not need recompilation in case more headers are added to the allow list