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

Google OIDC ID tokens unexpectedly lose their audience when refreshing w/ kubectl #56063

Closed
jchv opened this issue Nov 20, 2017 · 7 comments
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/auth Categorizes an issue or PR as relevant to SIG Auth.

Comments

@jchv
Copy link

jchv commented Nov 20, 2017

Is this a BUG REPORT or FEATURE REQUEST?:

/kind bug
/sig auth

What happened:

I am using Google as an OIDC provider. In order to use web-based authorization alongside kubectl/the CLI, I have set up the recommended configuration with two separate sets of OAuth2 client credentials, with the web client ID being the client ID set on the API server. This does work and we've been able to authenticate both Kubernetes Dashboard and kubectl with the same user.

However, when the ID token expires and a refresh is attempted, the refresh does unexpected things to the token.

The original token looks like this:

...
  "azp": "CLI_CLIENT_ID",
  "aud": "WEB_CLIENT_ID",
...

After refresh, it looks like this:

...
  "azp": "CLI_CLIENT_ID",
  "aud": "CLI_CLIENT_ID",
...

And then the kubectl command fails:

error: You must be logged in to the server (the server has asked for the client to provide credentials)

What you expected to happen:

I expected the audience and AZP to remain the same throughout the refresh. Maybe my (custom) authorization is incorrect?

I am not confident that this has anything to do with Kubernetes, but after a month of trying to get this to work I'm at wits end and browsing through endless issue pages and Google Groups has gotten me nowhere. If there's no bug in Kubernetes, I hope to learn what I'm doing wrong at the very least so we can stop manually getting new tokens every hour or so.

How to reproduce it (as minimally and precisely as possible):

  1. Create two sets of credentials on Google Cloud API Credentials manager, one with Web and one with Other, under the same Project.

  2. Assign the API server to the Web set of credentials.

  3. Perform authorization using CLI_CLIENT_ID, then when getting tokens, add a scope for the audience claim: audience:server:client_id:WEB_CLIENT_ID.

  4. Add information to ~/.kube/config. Verify that it works.

  5. After the ID token expires, run any authenticated kubectl command (i.e. kubectl get pods)

  6. It should fail, and now the ID token will contain the wrong audience.

Anything else we need to know?:

As mentioned, the setup does work other than this issue. I am using a custom oidc-helper utility to do the authentication, and a custom oidc-proxy for Kubernetes dashboard. Even Kubernetes aside, I wish there was more documentation around audiences and authorized parties with regards to Google's OIDC provider :(

Environment:

  • Kubernetes version: Client Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.1", GitCommit:"f38e43b221d08850172a9a4ea785a86a3ffa3b3a", GitTreeState:"clean", BuildDate:"2017-10-11T23:27:35Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"windows/amd64"}

(Details about the Kubernetes cluster are irrelevant; this problem can be reproduced even without being able to reach the API server, so it is purely client-side.)

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 20, 2017
@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Nov 20, 2017
@k8s-ci-robot k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Nov 20, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Nov 20, 2017
@jchv
Copy link
Author

jchv commented Nov 20, 2017

OK, update: it looks like I'm also losing other scopes. I think the change that removes extra-scopes needs to be reconsidered, it looks like at least for Google's OpenID implementation, you do not implicitly get your scopes back in a refresh. Any thoughts on this? I can make a PR for this if it sounds acceptable. If some providers do actually refresh with the original scopes, then it would make sense to support both sending scopes and not sending scopes.

@jchv
Copy link
Author

jchv commented Nov 20, 2017

Ah dang, this is starting to look worse. So I reimplemented extra-scopes in the most logical way possible:

diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go
index 1fe52c5241..2be5e6a6a2 100644
--- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go
+++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go
@@ -28,6 +28,7 @@ import (
        "sync"
        "time"

+       "github.com/coreos/go-oidc/oidc"
        "github.com/golang/glog"
        "golang.org/x/oauth2"
        "k8s.io/apimachinery/pkg/util/net"
@@ -40,11 +41,9 @@ const (
        cfgClientSecret             = "client-secret"
        cfgCertificateAuthority     = "idp-certificate-authority"
        cfgCertificateAuthorityData = "idp-certificate-authority-data"
+       cfgExtraScopes              = "extra-scopes"
        cfgIDToken                  = "id-token"
        cfgRefreshToken             = "refresh-token"
-
-       // Unused. Scopes aren't sent during refreshing.
-       cfgExtraScopes = "extra-scopes"
 )

 func init() {
@@ -123,11 +122,6 @@ func newOIDCAuthProvider(_ string, cfg map[string]string, persister restclient.A
                return provider, nil
        }

-       if len(cfg[cfgExtraScopes]) > 0 {
-               glog.V(2).Infof("%s auth provider field depricated, refresh request don't send scopes",
-                       cfgExtraScopes)
-       }
-
        var certAuthData []byte
        var err error
        if cfg[cfgCertificateAuthorityData] != "" {
@@ -244,10 +238,13 @@ func (p *oidcAuthProvider) idToken() (string, error) {
                return "", err
        }

+       scopes := strings.Split(p.cfg[cfgExtraScopes], ",")
+
        config := oauth2.Config{
                ClientID:     p.cfg[cfgClientID],
                ClientSecret: p.cfg[cfgClientSecret],
                Endpoint:     oauth2.Endpoint{TokenURL: tokenURL},
+               Scopes:       append(scopes, oidc.DefaultScope...),
        }

        ctx := context.WithValue(context.Background(), oauth2.HTTPClient, p.client)

On first glance, this was working - my scopes appeared to be maintained. Except one, the audience scope! It turns out you have to set the audience URL parameter in your POST to the token endpoint.

As far as I can tell, the golang.org/x/oauth2 library offers no way of doing this, putting this in an even more awkward position. So I edited it in the vendor folder:

diff --git a/vendor/golang.org/x/oauth2/oauth2.go b/vendor/golang.org/x/oauth2/oauth2.go
index bb35f277c2..8a89c2938b 100644
--- a/vendor/golang.org/x/oauth2/oauth2.go
+++ b/vendor/golang.org/x/oauth2/oauth2.go
@@ -227,6 +227,7 @@ func (tf *tokenRefresher) Token() (*Token, error) {
        }

        tk, err := retrieveToken(tf.ctx, tf.conf, url.Values{
+               "audience":      {"WEB_CLIENT_ID"},
                "grant_type":    {"refresh_token"},
                "refresh_token": {tf.refreshToken},
        })

This worked, but obviously is not going to be acceptable :)

I deeply hope that I'm incorrect and something I'm doing is just horribly wrong, but given that this actually worked I'm suspecting that in fact all of this would be necessary if I wanted to continue using Google as an authentication provider, which I do. But there's a lot of layers to bust through here. Seems like one of the more sane workarounds would be calling the tokens endpoint directly bypassing the OAuth2 library entirely, and I am sure that will be about as popular as hardcoding a change in the vendor folder. I would also be willing to do that, though.

On the other hand, the more orthodox approach would be to modify golang.org/x/oauth2, but I have no idea how likely that is. Seems like it'd need to be an API break to me...

I'm beginning to wonder if anyone's gone through this before and I'm just failing very badly at searching for it. Am I the first person to try this setup?

@jchv
Copy link
Author

jchv commented Nov 20, 2017

OK, final update for the day (I think:) I am now questioning whether Scopes even comes into play during a refresh. It looks like the golang.org/x/oauth2 library doesn't even touch it, so I don't know how it'd be possible to begin with. That calls into question whether this has anything at all to do with scopes... (they actually get lost either way sometimes, which I assume would be prevented if the underlying library sent it. There's an issue in for that too, though.)

However, I was able to verify very directly that setting the audience POST parameter works as suspected, and not setting it results in a broken refresh. So it's seeming more like I am going to need to move my attention to the golang.org/x/oauth2 library first to solve this problem.

@liggitt
Copy link
Member

liggitt commented Nov 20, 2017

to be clear, you are submitting a refresh token, and getting back an id token that is missing the aud claim of the original (pre-refresh) id token?

that seems contrary to the oidc spec for refresh token responses - http://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse

If an ID Token is returned as a result of a token refresh request, the following requirements apply:

  • its iss Claim Value MUST be the same as in the ID Token issued when the original authentication occurred,
  • its sub Claim Value MUST be the same as in the ID Token issued when the original authentication occurred,
  • its iat Claim MUST represent the time that the new ID Token is issued,
  • its aud Claim Value MUST be the same as in the ID Token issued when the original authentication occurred,
  • if the ID Token contains an auth_time Claim, its value MUST represent the time of the original authentication - not the time that the new ID token is issued,
  • its azp Claim Value MUST be the same as in the ID Token issued when the original authentication occurred; if no azp Claim was present in the original ID Token, one MUST NOT be present in the new ID Token, and

@jchv
Copy link
Author

jchv commented Nov 20, 2017

Yes, that is exactly the case. The aud comes back up as being the same as the azp. I don't think that's the only way the Google OIDC implemention violates the spec, either.

@liggitt
Copy link
Member

liggitt commented Nov 23, 2017

Maybe I'm just missing it, but I'm not seeing anything in the oidc spec about submitting an audience parameter with a refresh token. As such, this doesn't seem like a kubernetes bug.

@liggitt liggitt closed this as completed Nov 23, 2017
@jchv
Copy link
Author

jchv commented Nov 23, 2017

@liggitt Please consider re-opening. I am very much aware that Google is violating the spec, but unfortunately this is the case for many OIDC providers.

OAuth2 and OIDC providers are notoriously buggy. If they weren't, this list wouldn't exist: https://github.com/golang/oauth2/blob/master/internal/token.go#L92

If there is no intention in fixing this, then it might be worth documenting that the Google OAuth provider is not a good idea to use with Kubernetes, because we're now in production with a cluster we can only use with a modified kubectl :( I guess I can try to contact Google about this, but I have a feeling I've got a better chance at winning the lottery than getting a response of any kind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Development

No branches or pull requests

4 participants