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

[1.3.3] OpenID Connect provider fails with trailing slashes in the issuer URL #29749

Closed
hanikesn opened this issue Jul 28, 2016 · 7 comments
Closed
Labels
area/kubectl sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@hanikesn
Copy link

I like to use Azure Active Directory as IDP:
https://sts.windows.net/14929598-cff9-49f0-b20b-295e9d4dcb8b/.well-known/openid-configuration
The issuer URL has a trailing slash: https://sts.windows.net/14929598-cff9-49f0-b20b-295e9d4dcb8b/, but when I specify the issuer url with a trailing slash on the command line options for kube-apiserver the trailing slash gets trimmed before validation:

E0728 17:34:50.966425   21702 oidc.go:119] Failed to fetch provider config, trying again in 3s: "issuer" in config (https://sts.windows.net/14929598-cff9-49f0-b20b-295e9d4dcb8b/) does not match provided issuer URL (https://sts.windows.net/14929598-cff9-49f0-b20b-295e9d4dcb8b)

This means I can't use this IDP. And the apiserver will fail to start with this configuration.

Related issues:
#20476
#21128

@hanikesn
Copy link
Author

This commit 36bd693 breaks the verification.
It's also not necessary anymore as the trailing slash gets trimmed later in the code anyways: https://github.com/kubernetes/kubernetes/blob/v1.3.3/vendor/github.com/coreos/go-oidc/oidc/provider.go#L625

Paging @yifan-gu

@hanikesn
Copy link
Author

hanikesn commented Aug 1, 2016

@ericchiang
Copy link
Contributor

I believe this was fixed in our client. Feel free to assign me. I'll try to update the dependency today.

@hanikesn
Copy link
Author

hanikesn commented Aug 1, 2016

@ericchiang I think the client looks fine in 1.3.3 and it's just the old workaround that breaks things.

@ericchiang
Copy link
Contributor

Ah sorry. Yeah this line in the OpenID Connect plugin looks redundant[0]

[0] https://github.com/kubernetes/kubernetes/blob/v1.3.3/plugin/pkg/auth/authenticator/token/oidc/oidc.go#L115

@k8s-github-robot k8s-github-robot added area/kubectl sig/node Categorizes an issue or PR as relevant to SIG Node. labels Aug 3, 2016
k8s-github-robot pushed a commit that referenced this issue Aug 4, 2016
…-with-trailing-slash

Automatic merge from submit-queue

oidc authentication plugin: don't trim issuer URLs with trailing slashes

The issuer URL passed to the plugin must identically match the issuer
URL returned by OpenID Connect discovery. However, the plugin currently
trims all trailing slashes from issuer URLs, causing a mismatch. Since
the go-oidc client already handles this case correctly, don't trim the
path.

Closes #29749

cc @hanikesn @kubernetes/sig-auth
@hanikesn
Copy link
Author

hanikesn commented Aug 5, 2016

Any chance to get this backported for 1.3? I tried cherry picking it, but it didn't apply cleanly.

@erictune
Copy link
Member

erictune commented Aug 5, 2016

I created this cherry pick:
#30149

jessfraz pushed a commit to jessfraz/kubernetes that referenced this issue Aug 19, 2016
…r-dont-trim-issuer

Automatic merge from submit-queue

oidc auth provider: don't trim issuer URL

This mirrors a similar side fix for the API server authenticator.
Don't trim the issuer URL provided by the user since OpenID Connect
mandates that this URL exactly matches the URL returned by the
issuer during discovery.

This change only impacts clients attempting to connect to providers that
are non-spec compliant.

No test updates since this is already tested by the go-oidc client
package.

See: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation

Server side fix kubernetes#29860
Updates kubernetes#29749

cc @kubernetes/sig-auth @hanikesn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubectl sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests

4 participants