From 0b3b99c096fec15d698c6e2df417f9458566f721 Mon Sep 17 00:00:00 2001 From: Michael Taufen Date: Wed, 28 Apr 2021 09:24:54 -0700 Subject: [PATCH] Try both in-cluster and external discovery The conformance test for ServiceAccountIssuerDiscovery is currently configured with --in-cluster-discovery, which only supports token validation against in-cluster endpoints. Many cloud providers provide their own, external endpoints for OIDC discovery, and because the iss claim in tokens will point to these endpoints, but the client in this test only trusts the Cluster CA, it will fail to connect to the external discovery endpoints when validating the token. To ensure that the conformance test at least supports scenario where both the discovery doc endpoint and JWKS endpoint are cluster-local and the scenario where both endpoints are cluster-external, this PR has the test try both and requires at least one to pass. Caveat: The test still won't support a configuration where one endpoint is cluster-local and the other is external. We don't yet have evidence that this is a configuration that is used in practice, so this initial hotfix will at least fix the conformance test for the "both external" configuration we know providers already use. Note that if one endpoint is cluster-local, and the other is cluster-external, tokens can still only be validated in-cluster, because both endpoints must be accessible to Relying Parties that validate tokens. --- build/dependencies.yaml | 2 +- test/images/agnhost/VERSION | 2 +- test/images/agnhost/agnhost.go | 2 +- .../agnhost/openidmetadata/openidmetadata.go | 113 +++++++++++------- 4 files changed, 70 insertions(+), 49 deletions(-) diff --git a/build/dependencies.yaml b/build/dependencies.yaml index e723717b078..23ff7b66390 100644 --- a/build/dependencies.yaml +++ b/build/dependencies.yaml @@ -18,7 +18,7 @@ dependencies: # agnhost: bump this one first - name: "agnhost" - version: "2.31" + version: "2.32" refPaths: - path: test/images/agnhost/VERSION match: \d.\d diff --git a/test/images/agnhost/VERSION b/test/images/agnhost/VERSION index 3125d734075..0f34dc7f370 100644 --- a/test/images/agnhost/VERSION +++ b/test/images/agnhost/VERSION @@ -1 +1 @@ -2.31 +2.32 diff --git a/test/images/agnhost/agnhost.go b/test/images/agnhost/agnhost.go index be59fce49d9..3118cc1bcae 100644 --- a/test/images/agnhost/agnhost.go +++ b/test/images/agnhost/agnhost.go @@ -51,7 +51,7 @@ import ( func main() { rootCmd := &cobra.Command{ Use: "app", - Version: "2.31", + Version: "2.32", } rootCmd.AddCommand(auditproxy.CmdAuditProxy) diff --git a/test/images/agnhost/openidmetadata/openidmetadata.go b/test/images/agnhost/openidmetadata/openidmetadata.go index 90fa9297c1c..fe33656a4c2 100644 --- a/test/images/agnhost/openidmetadata/openidmetadata.go +++ b/test/images/agnhost/openidmetadata/openidmetadata.go @@ -26,6 +26,7 @@ import ( "net" "net/http" "net/url" + "os" "runtime" "time" @@ -48,32 +49,58 @@ var CmdTestServiceAccountIssuerDiscovery = &cobra.Command{ } var ( - tokenPath string - audience string - inClusterDiscovery bool + tokenPath string + audience string ) func init() { fs := CmdTestServiceAccountIssuerDiscovery.Flags() fs.StringVar(&tokenPath, "token-path", "", "Path to read service account token from.") fs.StringVar(&audience, "audience", "", "Audience to check on received token.") - fs.BoolVar(&inClusterDiscovery, "in-cluster-discovery", false, - "Includes the in-cluster bearer token in request headers. "+ - "Use when validating against API server's discovery endpoints, "+ - "which require authentication.") } func main(cmd *cobra.Command, args []string) { - ctx, err := withOAuth2Client(context.Background()) - if err != nil { - log.Fatal(err) - } - raw, err := gettoken() if err != nil { log.Fatal(err) } log.Print("OK: Got token") + + /* + To support both in-cluster discovery and external (non kube-apiserver) + discovery: + 1. Attempt with in-cluster discovery. Only trust Cluster CA. + If pass, exit early, successfully. This attempt includes the bearer + token, so we only trust the Cluster CA to avoid sending tokens to + some external endpoint by accident. + 2. If in-cluster discovery doesn't pass, then try again assuming both + discovery doc and JWKS endpoints are external rather than being + served from kube-apiserver. This attempt does not pass the bearer + token at all. + */ + + log.Print("validating with in-cluster discovery") + inClusterCtx, err := withInClusterOauth2Client(context.Background()) + if err != nil { + log.Fatal(err) + } + if err := validate(inClusterCtx, raw); err == nil { + os.Exit(0) + } else { + log.Print("failed to validate with in-cluster discovery: ", err) + } + + log.Print("falling back to validating with external discovery") + externalCtx, err := withExternalOAuth2Client(context.Background()) + if err != nil { + log.Fatal(err) + } + if err := validate(externalCtx, raw); err != nil { + log.Fatal(err) + } +} + +func validate(ctx context.Context, raw string) error { tok, err := jwt.ParseSigned(raw) if err != nil { log.Fatal(err) @@ -93,7 +120,7 @@ func main(cmd *cobra.Command, args []string) { iss, err := oidc.NewProvider(ctx, unsafeClaims.Issuer) if err != nil { - log.Fatal(err) + return err } log.Printf("OK: Constructed OIDC provider for issuer %v", unsafeClaims.Issuer) @@ -102,16 +129,17 @@ func main(cmd *cobra.Command, args []string) { SupportedSigningAlgs: []string{oidc.RS256, oidc.ES256}, }).Verify(ctx, raw) if err != nil { - log.Fatal(err) + return err } log.Print("OK: Validated signature on JWT") var safeClaims claims if err := validTok.Claims(&safeClaims); err != nil { - log.Fatal(err) + return err } log.Print("OK: Got valid claims from token!") log.Printf("Full, validated claims: \n%#v", &safeClaims) + return nil } type kubeName struct { @@ -139,42 +167,35 @@ func gettoken() (string, error) { return string(b), err } -// withOAuth2Client returns a context that includes an HTTP Client, under the -// oauth2.HTTPClient key. If --in-cluster-discovery is true, the client will -// use the Kubernetes InClusterConfig. Otherwise it will use -// http.DefaultTransport. -// The `oidc` library respects the oauth2.HTTPClient context key; if it is set, -// the library will use the provided http.Client rather than the default -// HTTP client. -// This allows us to ensure requests get routed to the API server for -// --in-cluster-discovery, in a client configured with the appropriate CA. -func withOAuth2Client(context.Context) (context.Context, error) { - // TODO(mtaufen): Someday, might want to change this so that we can test - // TokenProjection with an API audience set to the external provider with - // requests against external endpoints (in which case we'd send - // a different token with a non-Kubernetes audience). - - // By default, use the default http transport with the system root bundle, +func withExternalOAuth2Client(ctx context.Context) (context.Context, error) { + // Use the default http transport with the system root bundle, // since it's validating against the external internet. - rt := http.DefaultTransport - if inClusterDiscovery { - // If in-cluster discovery, then use the in-cluster config so we can - // authenticate with the API server. - cfg, err := rest.InClusterConfig() - if err != nil { - return nil, err - } - rt, err = rest.TransportFor(cfg) - if err != nil { - return nil, fmt.Errorf("could not get roundtripper: %v", err) - } + return context.WithValue(ctx, + // The `oidc` library respects the oauth2.HTTPClient context key; if it is set, + // the library will use the provided http.Client rather than the default HTTP client. + oauth2.HTTPClient, &http.Client{ + Transport: http.DefaultTransport, + }), nil +} + +func withInClusterOauth2Client(ctx context.Context) (context.Context, error) { + // Use the in-cluster config so we can trust and authenticate with kube-apiserver + cfg, err := rest.InClusterConfig() + if err != nil { + return nil, err } - ctx := context.WithValue(context.Background(), + rt, err := rest.TransportFor(cfg) + if err != nil { + return nil, fmt.Errorf("could not get roundtripper: %v", err) + } + + return context.WithValue(ctx, + // The `oidc` library respects the oauth2.HTTPClient context key; if it is set, + // the library will use the provided http.Client rather than the default HTTP client. oauth2.HTTPClient, &http.Client{ Transport: rt, - }) - return ctx, nil + }), nil } // DNS can be available sometime after the container starts due to the way