From 39ee6f7bdd4b98611b4b3ed3bba5ea134d18dfe8 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 17 Oct 2023 11:40:44 +0900 Subject: [PATCH] Trust DataPlaneUserSAN from Activator to Queue-Proxy (#14452) * Trust DataPlaneUserSAN from Activator to Queue-Proxy * Fix lint * Fix plate * Remove * Use read lock * bump pkg * Use DataPlaneUserSAN instead of DataPlaneUserName --- cmd/activator/main.go | 2 +- go.mod | 2 +- go.sum | 4 +- pkg/activator/certificate/tls_context.go | 68 ++++++++++++++++++ pkg/activator/certificate/tls_context_test.go | 72 +++++++++++++++++++ .../apiextensions/storageversion/migrator.go | 5 ++ .../pkg/environment/client_config.go | 8 +-- vendor/knative.dev/pkg/network/h2c.go | 8 +-- vendor/knative.dev/pkg/network/transports.go | 15 ++-- .../pkg/test/upgrade/shell/executor.go | 4 +- vendor/modules.txt | 2 +- 11 files changed, 166 insertions(+), 24 deletions(-) create mode 100644 pkg/activator/certificate/tls_context.go create mode 100644 pkg/activator/certificate/tls_context_test.go diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 8f0095a090d9..9756e4d43bce 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -169,7 +169,7 @@ func main() { if tlsEnabled { logger.Info("Knative Internal TLS is enabled") certCache = certificate.NewCertCache(ctx) - transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, &certCache.TLSConf) + transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, certCache.TLSContext()) } // Start throttler. diff --git a/go.mod b/go.mod index 97b011f07c67..29a258763079 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( knative.dev/caching v0.0.0-20231012110827-8551914fdf65 knative.dev/hack v0.0.0-20231010131532-fc76874b28c6 knative.dev/networking v0.0.0-20231012062439-c0863403c83b - knative.dev/pkg v0.0.0-20231016092905-cf06733aa47b + knative.dev/pkg v0.0.0-20231016185203-283df0be0668 sigs.k8s.io/yaml v1.3.0 ) diff --git a/go.sum b/go.sum index 35255db2854d..aae45c7ae8bd 100644 --- a/go.sum +++ b/go.sum @@ -931,8 +931,8 @@ knative.dev/hack v0.0.0-20231010131532-fc76874b28c6 h1:K9saPnpWTK1xH/Dpx1aE4CA+4 knative.dev/hack v0.0.0-20231010131532-fc76874b28c6/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q= knative.dev/networking v0.0.0-20231012062439-c0863403c83b h1:yGtVPNHek3rmKb50k7G9fG/NuuC4FRzESVrWmPFU9AM= knative.dev/networking v0.0.0-20231012062439-c0863403c83b/go.mod h1:uEvP4spV82HGB8loxo8nH/LGmwsd9jUGWvDVC+tH4O4= -knative.dev/pkg v0.0.0-20231016092905-cf06733aa47b h1:ELB4gahWrQ+C1flcudcX3v7ThYYOD3Tnm8+ISod2bwQ= -knative.dev/pkg v0.0.0-20231016092905-cf06733aa47b/go.mod h1:khuxKBM4WqjcCIeCIm+4VDNBmzMsl0ZspXGMm5i/fFA= +knative.dev/pkg v0.0.0-20231016185203-283df0be0668 h1:rYlTKNUZbMsSHQID0A7sZbrtXlD+REKN6F94ceMnA5c= +knative.dev/pkg v0.0.0-20231016185203-283df0be0668/go.mod h1:khuxKBM4WqjcCIeCIm+4VDNBmzMsl0ZspXGMm5i/fFA= pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= diff --git a/pkg/activator/certificate/tls_context.go b/pkg/activator/certificate/tls_context.go new file mode 100644 index 000000000000..a0453f574f2e --- /dev/null +++ b/pkg/activator/certificate/tls_context.go @@ -0,0 +1,68 @@ +/* +Copyright 2023 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package certificate + +import ( + "context" + "crypto/tls" + "errors" + "fmt" + "net" + + "knative.dev/networking/pkg/certificates" + pkgnet "knative.dev/pkg/network" + "knative.dev/serving/pkg/activator/handler" +) + +// TLSContext returns DialTLSContextFunc. +func (cr *CertCache) TLSContext() pkgnet.DialTLSContextFunc { + return cr.dialTLSContext +} + +// dialTLSContext handles TLS dialer +func (cr *CertCache) dialTLSContext(ctx context.Context, network, addr string) (net.Conn, error) { + return dialTLSContext(ctx, network, addr, cr) +} + +// dialTLSContext handles verify SAN before calling DialTLSWithBackOff. +func dialTLSContext(ctx context.Context, network, addr string, cr *CertCache) (net.Conn, error) { + cr.certificatesMux.RLock() + // Clone the certificate Pool such that the one used by the client will be different from the one that will get updated is CA is replaced. + tlsConf := cr.TLSConf.Clone() + tlsConf.RootCAs = tlsConf.RootCAs.Clone() + cr.certificatesMux.RUnlock() + + revID := handler.RevIDFrom(ctx) + san := certificates.DataPlaneUserSAN(revID.Namespace) + + tlsConf.VerifyConnection = verifySAN(san) + return pkgnet.DialTLSWithBackOff(ctx, network, addr, tlsConf) +} + +func verifySAN(san string) func(tls.ConnectionState) error { + return func(cs tls.ConnectionState) error { + if len(cs.PeerCertificates) == 0 { + return errors.New("no PeerCertificates provided") + } + for _, name := range cs.PeerCertificates[0].DNSNames { + if name == san { + return nil + } + } + return fmt.Errorf("san %q does not have a matching name in %v", san, cs.PeerCertificates[0].DNSNames) + } +} diff --git a/pkg/activator/certificate/tls_context_test.go b/pkg/activator/certificate/tls_context_test.go new file mode 100644 index 000000000000..3ea4e0226c94 --- /dev/null +++ b/pkg/activator/certificate/tls_context_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2023 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package certificate + +import ( + "crypto/tls" + "crypto/x509" + "encoding/pem" + "testing" +) + +// TestVerifySAN tests verifySAN. +func TestVerifySAN(t *testing.T) { + tests := []struct { + name string + san string + expErr bool + }{{ + name: "first SAN", + san: "knative-knative-serving", + expErr: false, + }, { + name: "second SAN", + san: "data-plane.knative.dev", + expErr: false, + }, { + name: "non existent SAN", + san: "foo", + expErr: true, + }} + + // tlsCrt contains two SANs knative-knative-serving and data-plane.knative.dev. + block, _ := pem.Decode(tlsCrt) + if block == nil { + t.Fatal("failed to parse certificate PEM") + } + + serverCert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("failed to parse certificate: %v", err) + } + + tlsConnectionState := tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{serverCert}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := verifySAN(test.san)(tlsConnectionState) + if test.expErr && err == nil { + t.Fatalf("failed to verify SAN") + } + if !test.expErr && err != nil { + t.Fatalf("failed to verify SAN: %v", err) + } + }) + } +} diff --git a/vendor/knative.dev/pkg/apiextensions/storageversion/migrator.go b/vendor/knative.dev/pkg/apiextensions/storageversion/migrator.go index 7ac1cc932687..469031251e52 100644 --- a/vendor/knative.dev/pkg/apiextensions/storageversion/migrator.go +++ b/vendor/knative.dev/pkg/apiextensions/storageversion/migrator.go @@ -68,6 +68,11 @@ func (m *Migrator) Migrate(ctx context.Context, gr schema.GroupResource) error { return fmt.Errorf("unable to determine storage version for %s", gr) } + // don't migrate storage version if CRD has a single valid storage in its status + if len(crd.Status.StoredVersions) == 1 && crd.Status.StoredVersions[0] == version { + return nil + } + if err := m.migrateResources(ctx, gr.WithVersion(version)); err != nil { return err } diff --git a/vendor/knative.dev/pkg/environment/client_config.go b/vendor/knative.dev/pkg/environment/client_config.go index 0b51857aa143..aef33927ef72 100644 --- a/vendor/knative.dev/pkg/environment/client_config.go +++ b/vendor/knative.dev/pkg/environment/client_config.go @@ -44,12 +44,8 @@ func (c *ClientConfig) InitFlags(fs *flag.FlagSet) { fs.StringVar(&c.ServerURL, "server", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") - if f := fs.Lookup("kubeconfig"); f != nil { - c.Kubeconfig = f.Value.String() - } else { - fs.StringVar(&c.Kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), - "Path to a kubeconfig. Only required if out-of-cluster.") - } + fs.StringVar(&c.Kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), + "Path to a kubeconfig. Only required if out-of-cluster.") fs.IntVar(&c.Burst, "kube-api-burst", int(envVarOrDefault("KUBE_API_BURST", 0)), "Maximum burst for throttle.") diff --git a/vendor/knative.dev/pkg/network/h2c.go b/vendor/knative.dev/pkg/network/h2c.go index ebf1ee8a2ac4..e1671233dcbd 100644 --- a/vendor/knative.dev/pkg/network/h2c.go +++ b/vendor/knative.dev/pkg/network/h2c.go @@ -59,13 +59,11 @@ func newH2CTransport(disableCompression bool) http.RoundTripper { // newH2Transport constructs a neew H2 transport. That transport will handles HTTPS traffic // with TLS config. -func newH2Transport(disableCompression bool, tlsConf *tls.Config) http.RoundTripper { +func newH2Transport(disableCompression bool, tlsContext DialTLSContextFunc) http.RoundTripper { return &http2.Transport{ DisableCompression: disableCompression, - DialTLS: func(netw, addr string, tlsConf *tls.Config) (net.Conn, error) { - return DialTLSWithBackOff(context.Background(), - netw, addr, tlsConf) + DialTLSContext: func(ctx context.Context, network, addr string, cfg *tls.Config) (net.Conn, error) { + return tlsContext(ctx, network, addr) }, - TLSClientConfig: tlsConf, } } diff --git a/vendor/knative.dev/pkg/network/transports.go b/vendor/knative.dev/pkg/network/transports.go index d48cd62151ef..1e9c6c219865 100644 --- a/vendor/knative.dev/pkg/network/transports.go +++ b/vendor/knative.dev/pkg/network/transports.go @@ -127,16 +127,17 @@ func newHTTPTransport(disableKeepAlives, disableCompression bool, maxIdle, maxId return transport } -func newHTTPSTransport(disableKeepAlives, disableCompression bool, maxIdle, maxIdlePerHost int, tlsConf *tls.Config) http.RoundTripper { +type DialTLSContextFunc func(ctx context.Context, network, addr string) (net.Conn, error) + +func newHTTPSTransport(disableKeepAlives, disableCompression bool, maxIdle, maxIdlePerHost int, tlsContext DialTLSContextFunc) http.RoundTripper { transport := http.DefaultTransport.(*http.Transport).Clone() - transport.DialContext = DialWithBackOff transport.DisableKeepAlives = disableKeepAlives transport.MaxIdleConns = maxIdle transport.MaxIdleConnsPerHost = maxIdlePerHost transport.ForceAttemptHTTP2 = false transport.DisableCompression = disableCompression + transport.DialTLSContext = tlsContext - transport.TLSClientConfig = tlsConf return transport } @@ -148,11 +149,11 @@ func NewProberTransport() http.RoundTripper { NewH2CTransport()) } -// NewProxyAutoTLSTransport is same with NewProxyAutoTransport but it has tls.Config to create HTTPS request. -func NewProxyAutoTLSTransport(maxIdle, maxIdlePerHost int, tlsConf *tls.Config) http.RoundTripper { +// NewProxyAutoTLSTransport is same with NewProxyAutoTransport but it has DialTLSContextFunc to create HTTPS request. +func NewProxyAutoTLSTransport(maxIdle, maxIdlePerHost int, tlsContext DialTLSContextFunc) http.RoundTripper { return newAutoTransport( - newHTTPSTransport(false /*disable keep-alives*/, true /*disable auto-compression*/, maxIdle, maxIdlePerHost, tlsConf), - newH2Transport(true /*disable auto-compression*/, tlsConf)) + newHTTPSTransport(false /*disable keep-alives*/, true /*disable auto-compression*/, maxIdle, maxIdlePerHost, tlsContext), + newH2Transport(true /*disable auto-compression*/, tlsContext)) } // NewAutoTransport creates a RoundTripper that can use appropriate transport diff --git a/vendor/knative.dev/pkg/test/upgrade/shell/executor.go b/vendor/knative.dev/pkg/test/upgrade/shell/executor.go index d41ccc765a77..020f0d3eddf5 100644 --- a/vendor/knative.dev/pkg/test/upgrade/shell/executor.go +++ b/vendor/knative.dev/pkg/test/upgrade/shell/executor.go @@ -185,7 +185,9 @@ func (w testingWriter) Write(p []byte) (n int, err error) { // Strip trailing newline because t.Log always adds one. p = bytes.TrimRight(p, "\n") - w.t.Logf("%s", p) + for _, line := range strings.Split(string(p), "\n") { + w.t.Logf(line) + } return n, nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index cd51dc17c40d..821f8f43e923 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1340,7 +1340,7 @@ knative.dev/networking/pkg/http/stats knative.dev/networking/pkg/ingress knative.dev/networking/pkg/k8s knative.dev/networking/pkg/prober -# knative.dev/pkg v0.0.0-20231016092905-cf06733aa47b +# knative.dev/pkg v0.0.0-20231016185203-283df0be0668 ## explicit; go 1.18 knative.dev/pkg/apiextensions/storageversion knative.dev/pkg/apiextensions/storageversion/cmd/migrate