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

[spiffe-oidc-discovery-provider] Issuer issues #5719

Open
kfox1111 opened this issue Dec 15, 2024 · 11 comments
Open

[spiffe-oidc-discovery-provider] Issuer issues #5719

kfox1111 opened this issue Dec 15, 2024 · 11 comments
Labels
triage/in-progress Issue triage is in progress

Comments

@kfox1111
Copy link
Contributor

Problem
There are multiple use cases for being able to override the issuer string that the discovery provider should return.

In #5657, the issuer was made configurable, but it also uses the same config option to perform multiple other behavioral changes that make it unusable for one of those use cases.

So, I think we need to step back and reconsider the design of these features.

There seems to be two primary use cases:

  1. The discovery provider is behind a reverse proxy / load balancer. It cant properly detect what the issuer string should be automatically.
  2. The discovery provider is being contacted directly, setup on an alternate dns name, and needs spire-server issued jwt's issuer property to match the returned issuer from the discovery provider.

For use case 1, there are multiple features that could be used to implement it:

  1. The issuer returned can't be guessed by the discovery provider based on the domain / path returned by the reverse proxy / load balancer. It needs to be configurable.
  2. The discovery provider returns an absolute url to the /keys endpoint. It needs to be configurable to route the traffic back to the right endpoint
  3. Some reverse proxy / load balancers do not support rewriting the url before passing it to the discovery provider. The ability to specify a prefix to serve off of would allow the discovery provider to be used with more software successfully.

There is, arguably, an anti feature implemented in pr 5657 that rewrites the incoming url to look like it shows up as the issuer. This prevents the domain checking from working in all cases. I think it should probably be reverted outright.

For use case 2, we need:

  1. The ability to set the returned issuer explicitly
  2. The keys endpoint needs to be reachable the same way that the discovery document is. No rewriting should be done.

An example of 2 is:

  • configure the discovery provider:
    • override the jwt issuer to be the same as that spire is set to. ex: "oidc-discovery-provider.example.org"
    • listen on an alternate port. ex: 8181
    • configure /etc/hosts entry for 127.0.0.1 -> k8ssodp.example.org
    • configure the allowed domain names to be "k8ssodp.example.org"
  • configure the kubernetes apiserver

This allows the main issuer to be "oidc-discovery-provider.example.org" on port 443, but have local instances on k8s control planes for high availability / bootstrapping purposes.


Proposed Changes

  1. Change the new jwt_issuer flag override just the issuer, no other behavior.
  2. Add an advertised_url option that overrides the url returned in the discovery document for keys (advertised_url + "/keys")
  3. Add a prefix option that defaults to / that configures where the url routes will listen on.
  4. Revert the domain checking changes. It was intended to make things easier, but hasn't had that affect.
@jer8me
Copy link

jer8me commented Jan 2, 2025

@kfox1111, thanks for the proposal.

  1. Instead of configuring advertised_url what do you think about being able to configure the JWKS URI instead (advertised_urljwks_uri)?
    This would allow a user to configure any kind of proxied URL, like https://jwks.external.org/somepath/jwks.json.

  2. What is the reason to add the prefix option? I understand that the user could add a path prefix, but what use case does this solve?
    Just wondering in what case there is a need to add this prefix.

@kfox1111
Copy link
Contributor Author

kfox1111 commented Jan 2, 2025

Hi @jer8me,

  1. jwks_uri would work too... I'd be ok with that. But, if we ever added any other urls to the service, they would need to be independently configured, while advertised_url would be used as the base for them all. Thoughts?

  2. The initial use case (discussed somewhat here Make the JWT issuer configurable in the OIDC Discovery Provider #5480 and exposed more through the unit tests implemented) made it clear they wanted to be able to hang multiple independent spire instances off of the same domain name with different paths.

So, say, server-a and server-b, they may want to do something like:
https://example.org/oidc/server-a -> discovery-provider-a
https://example.org/oidc/server-b -> discovery-provider-b

as a user then, you hit: https://example.org/oidc/server-a/.well-known/openid-configuration

The routing by default in most proxies would then to just grab the path from the request and send that along, like /oidc/server-a/.well-known/openid-configuration to the backend provider. but that doesn't normally exist in the discovery provider. only /.well-known/openid-configuration today.

So, some http routers support stripping off a prefix off of the url. so, strip /oidc/server-a off the request before it goes through to the other side. That works when the http router supports it. Unfortunately, some common ones don't support rewriting, just matching. :(

k8s ingress doesn't have a native way of doing it. (some implementations like ingress-nginx support it via annotations...)
last time I looked at ingress support in azure, it didnt support it at all (was like 2 years ago. may have changed)
aws still doesn't seem to support it: kubernetes-sigs/aws-load-balancer-controller#835. The workaround is to stack a whole nother webserver/router in the middle. :/

So, having the user be able to give the prefix to listen on to the discovery provider, lets it be used by all the http routers, not just the advanced ones that support url rewriting. The code is pretty simple around it. So, the juice is worth the squeeze IMHO.

@jer8me
Copy link

jer8me commented Jan 7, 2025

  1. Regarding jwks_uri vs advertised_url , I would prefer not assuming that we have one advertised prefix that all URLs are based of. For jwks_uri, I want to be able to publish the keys to something like an S3 bucket and have jwks_uri point to that. I see value in having a fully configurable jwks_uri.
  2. @kfox1111, thank you for explaining your use case for prefix. It sounds like a reasonable feature to have. Maybe the name could be more specific since prefix could mean a lot of different things. How about path_prefix or even server_path_prefix?

@kfox1111
Copy link
Contributor Author

kfox1111 commented Jan 7, 2025

For 1, I think it would be ideal for the s3 bundle publisher to push both the well known config and jwks url to the s3 bucket directly, and skip the oidc discovery provider all together? Why have another service when s3 would be the service? Is there any other reason to have the jwks_uri configurable besides that? (Just curious)

2, sounds good to me. Naming things is hard.

@jer8me
Copy link

jer8me commented Jan 8, 2025

Maybe the S3 bucket wasn't the best example. My point was that we should be able to publish jwks_uri regardless of how it's internally hosted. For example, you could have a public API Gateway calling the private OIDC provider service.

While advertised_url would do the job, I did not see the benefit to assume that if there are more URLs down the road, they should all be exposed via the same advertised URL.

@kfox1111
Copy link
Contributor Author

kfox1111 commented Jan 8, 2025

Right. But I guess the thing I'm questioning is, I think the purpose of jwks_uri or advertised_url or whatever we call it, is to help route requests back to one (or more) document(s) hosted by the oidc-discovery provider?

IE, we're not going to have user retrieve the discovery document from the oidc-discovery-provider, but get the jwks document from a service other then the oidc-discovery-provider? If that holds true, then having the configuration be based on how to get back to the root of the oidc-discovery-provider makes sense to me.

The documents are complex to generate and so, I'm thinking either all the stuff in the oidc-discovery-provider is hosted out of the oidc-discovery-provider, or, it all is hosted by some other service, but I'm not sure the use case for splitting it.

Maybe there is a use case I'm missing?

@jer8me
Copy link

jer8me commented Jan 8, 2025

It's not so much about how the oidc-discovery provider is hosted, it's more about how it is exposed/published.
The use case that I am thinking about is multi-tenancy.
Let's say I have 2 issuers, hosted on the same example.org host (this is a valid OIDC use case):

Per OIDC standards, the well-know documents MUST be served at:

Internally, each tenant has its own oidc-discovery provider service:

This is where I would want to use jwks_uri or advertised_url to set the jwks_uri value returned in each well-known document.
And the point I was trying to make earlier is that I might want to setup other hosts to expose the keys:

@kfox1111
Copy link
Contributor Author

kfox1111 commented Jan 8, 2025

Supporting one server instance for both https://example.org/issuer1 and https://issuer1.internal would mean that you would need a prefix, per hostname coming in. Thats harder to do... :/

For my use of that, I was expecting to run multiple, differently configured oidc-discovery-providers... Is that too much?

@jer8me
Copy link

jer8me commented Jan 8, 2025

Yeah so I am not suggesting that the OIDC provider should return a different /keys endpoints based on the external or internal host. The well-known document should always return the same jwks_uri.

In the example above, I would want to return the external URL. So I would set:
advertised_url = "https://issuer1.external"
or if we go with the full URL:
jwks_uri = "https://issuer1.external/keys"

But for internal use, I still want to be able to hit the /keys endpoint directly via https://issuer1.internal/keys

@kfox1111
Copy link
Contributor Author

kfox1111 commented Jan 8, 2025

ok, in that case, we could do it with two prefixes. one implicit one always being / ? or is there a reason not to allow https://issuer1.internal/issuer1/keys when its https://issuer.external/issuer1/keys in addition to https://issuer1.internal/keys?

Though I'm still fuzzy on, how https://issuer1.internal/keys is useful when https://issuer1.internal/.well-known/openid-configuration doesn't point at it?

@jer8me
Copy link

jer8me commented Jan 8, 2025

In my case, I don't need to use a prefix with the internal host. I can point the external host to any internal URL.

To be clear, I'm fine with the original idea of having advertised_url and prefix. However I believe that using jwks_uri instead of advertised_url is clearer for the user. It explicitly indicates its purpose, and it avoids any implicit expectations on what additional URLs might be hosted on.

As far as using the internal /keys endpoint directly (without getting it from the well-known endpoint), certain JWT verification implementations do not care about the well-known document. Envoy for instance just needs to know the JWKS URI: https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/jwt_authn_filter.html#remote-jwks-config-example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/in-progress Issue triage is in progress
Projects
None yet
Development

No branches or pull requests

3 participants