From e364fe6e4f58af7b399e9d81a40b6ee925cba0dc Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 30 Mar 2022 11:43:24 +0900 Subject: [PATCH 1/4] All Hops Encrypted: TLS between activator and queue-Proxy Fix https://github.com/knative/serving/issues/12502 Fix https://github.com/knative/serving/issues/12503 --- .github/workflows/kind-e2e.yaml | 12 ++++ cmd/activator/main.go | 33 +++++++++++ cmd/queue/main.go | 60 +++++++++++++++++--- pkg/activator/handler/handler.go | 17 +++++- pkg/http/proxy.go | 4 +- pkg/http/proxy_test.go | 2 +- pkg/networking/constants.go | 3 + pkg/queue/constants.go | 3 + pkg/reconciler/revision/resources/deploy.go | 21 +++++++ test/config/tls/ca-volume-path.yaml | 13 +++++ test/e2e-common.sh | 14 +++++ test/generate-cert.sh | 47 +++++++++++++++ vendor/knative.dev/pkg/network/h2c.go | 13 +++++ vendor/knative.dev/pkg/network/transports.go | 48 ++++++++++++++-- 14 files changed, 273 insertions(+), 17 deletions(-) create mode 100644 test/config/tls/ca-volume-path.yaml create mode 100644 test/generate-cert.sh diff --git a/.github/workflows/kind-e2e.yaml b/.github/workflows/kind-e2e.yaml index 164d16fe20e5..321eaa584b73 100644 --- a/.github/workflows/kind-e2e.yaml +++ b/.github/workflows/kind-e2e.yaml @@ -22,6 +22,7 @@ jobs: - v1.22.2/istio - v1.23.0/contour - v1.23.0/gateway-api + - v1.23.0/kourier-tls test-suite: - ./test/conformance/runtime @@ -60,6 +61,13 @@ jobs: kingress: gateway-api test-flags: "--enable-alpha" cluster-suffix: c${{ github.run_id }}.local + - cluster: v1.23.0/kourier-tls + k8s-version: v1.23.0 + kind-version: v0.11.1 + kind-image-sha: sha256:49824ab1727c04e56a21a5d8372a402fcd32ea51ac96a2706a12af38934f81ac + kingress: kourier + test-flags: "--enable-alpha --enable-beta" + cluster-suffix: c${{ github.run_id }}.local env: GOPATH: ${{ github.workspace }} @@ -195,6 +203,10 @@ jobs: KIND=1 INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" + if [[ "${{ matrix.cluster }}" == *-tls ]]; then + echo 'Enabling TLS' + ENABLE_TLS=1 + fi knative_setup test_setup diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 1ae3c9c640d2..35d144a464a3 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -18,8 +18,11 @@ package main import ( "context" + "crypto/tls" + "crypto/x509" "errors" "fmt" + "io/ioutil" "log" "net/http" "os" @@ -68,6 +71,9 @@ const ( // The port on which autoscaler WebSocket server listens. autoscalerPort = ":8080" + + caDirectory = "/var/lib/knative/ca" + caPath = caDirectory + "/ca.crt" ) type config struct { @@ -141,6 +147,33 @@ func main() { logger.Debugf("MaxIdleProxyConns: %d, MaxIdleProxyConnsPerHost: %d", env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost) transport := pkgnet.NewProxyAutoTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost) + // Enable TLS client when activator client ca is mounted. + // At this moment activator with TLS does not disable HTTP. + // See also https://github.com/knative/serving/issues/12808. + if exists(logger, caPath) { + pool, err := x509.SystemCertPool() + if err != nil { + pool = x509.NewCertPool() + } + + caCert, err := ioutil.ReadFile(caPath) + if err != nil { + log.Fatalf("failed to append %q to RootCAs: %v", caPath, err) + } + + if ok := pool.AppendCertsFromPEM(caCert); !ok { + logger.Fatalw("failed to append ca cert to the RootCAs") + } + + tlsConf := &tls.Config{ + RootCAs: pool, + InsecureSkipVerify: false, + ServerName: "knative", + MinVersion: tls.VersionTLS12, + } + transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, tlsConf) + } + // Fetch networking configuration to determine whether EnableMeshPodAddressability // is enabled or not. networkCM, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, network.ConfigName, metav1.GetOptions{}) diff --git a/cmd/queue/main.go b/cmd/queue/main.go index 40a517d762be..b32d5a657458 100644 --- a/cmd/queue/main.go +++ b/cmd/queue/main.go @@ -62,6 +62,9 @@ const ( // This is to give networking a little bit more time to remove the pod // from its configuration and propagate that to all loadbalancers and nodes. drainSleepDuration = 30 * time.Second + + certPath = queue.CertDirectory + "/tls.crt" + keyPath = queue.CertDirectory + "/tls.key" ) type config struct { @@ -162,10 +165,10 @@ func main() { if env.ConcurrencyStateEndpoint != "" { concurrencyendpoint = queue.NewConcurrencyEndpoint(env.ConcurrencyStateEndpoint, env.ConcurrencyStateTokenPath) } - mainServer, drain := buildServer(ctx, env, probe, stats, logger, concurrencyendpoint) + mainServer, drain := buildServer(ctx, env, probe, stats, logger, concurrencyendpoint, false) servers := map[string]*http.Server{ "main": mainServer, - "admin": buildAdminServer(logger, drain), + "admin": buildAdminServer(logger, drain, false), "metrics": buildMetricsServer(promStatReporter, protoStatReporter), } if env.EnableProfiling { @@ -182,6 +185,25 @@ func main() { }(name, server) } + // Enable TLS server when activator server certs are mounted. + // At this moment activator with TLS does not disable HTTP. + // See also https://github.com/knative/serving/issues/12808. + if exists(logger, certPath) && exists(logger, keyPath) { + mainTLSServer, drain := buildServer(ctx, env, probe, stats, logger, concurrencyendpoint, true) + tlsServers := map[string]*http.Server{ + "tlsMain": mainTLSServer, + "tlsAdmin": buildAdminServer(logger, drain, true), + } + for name, server := range tlsServers { + go func(name string, s *http.Server) { + // Don't forward ErrServerClosed as that indicates we're already shutting down. + if err := s.ListenAndServeTLS(certPath, keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) { + errCh <- fmt.Errorf("%s server failed to serve: %w", name, err) + } + }(name, server) + } + } + // Blocks until we actually receive a TERM signal or one of the servers // exits unexpectedly. We fold both signals together because we only want // to act on the first of those to reach here. @@ -212,6 +234,14 @@ func main() { } } +func exists(logger *zap.SugaredLogger, filename string) bool { + _, err := os.Stat(filename) + if err != nil && !os.IsNotExist(err) { + logger.Fatalw(fmt.Sprintf("Failed to verify the file path %q", filename), zap.Error(err)) + } + return err == nil +} + func buildProbe(logger *zap.SugaredLogger, encodedProbe string, autodetectHTTP2 bool) *readiness.Probe { coreProbe, err := readiness.DecodeProbe(encodedProbe) if err != nil { @@ -224,18 +254,18 @@ func buildProbe(logger *zap.SugaredLogger, encodedProbe string, autodetectHTTP2 } func buildServer(ctx context.Context, env config, probeContainer func() bool, stats *network.RequestStats, logger *zap.SugaredLogger, - ce *queue.ConcurrencyEndpoint) (server *http.Server, drain func()) { + ce *queue.ConcurrencyEndpoint, tls bool) (server *http.Server, drain func()) { target := net.JoinHostPort("127.0.0.1", env.UserPort) - httpProxy := pkghttp.NewHeaderPruningReverseProxy(target, pkghttp.NoHostOverride, activator.RevisionHeaders) + httpProxy := pkghttp.NewHeaderPruningReverseProxy(target, pkghttp.NoHostOverride, activator.RevisionHeaders, "http" /* use http to the target*/) httpProxy.Transport = buildTransport(env, logger) httpProxy.ErrorHandler = pkghandler.Error(logger) httpProxy.BufferPool = network.NewBufferPool() httpProxy.FlushInterval = network.FlushInterval breaker := buildBreaker(logger, env) - metricsSupported := supportsMetrics(ctx, logger, env) + metricsSupported := supportsMetrics(ctx, logger, env, tls) tracingEnabled := env.TracingConfigBackend != tracingconfig.None concurrencyStateEnabled := env.ConcurrencyStateEndpoint != "" firstByteTimeout := time.Duration(env.RevisionTimeoutSeconds) * time.Second @@ -287,6 +317,11 @@ func buildServer(ctx context.Context, env config, probeContainer func() bool, st composedHandler = requestLogHandler(logger, composedHandler, env) } + if tls { + // Open +100 (8112) port. + return pkgnet.NewServer(":8112", composedHandler), drainer.Drain + } + return pkgnet.NewServer(":"+env.QueueServingPort, composedHandler), drainer.Drain } @@ -333,9 +368,10 @@ func buildBreaker(logger *zap.SugaredLogger, env config) *queue.Breaker { return queue.NewBreaker(params) } -func supportsMetrics(ctx context.Context, logger *zap.SugaredLogger, env config) bool { +func supportsMetrics(ctx context.Context, logger *zap.SugaredLogger, env config, tls bool) bool { // Setup request metrics reporting for end-user metrics. - if env.ServingRequestMetricsBackend == "" { + // TODO: enable Metrics for TLS enabled. + if env.ServingRequestMetricsBackend == "" || tls { return false } @@ -347,13 +383,21 @@ func supportsMetrics(ctx context.Context, logger *zap.SugaredLogger, env config) return true } -func buildAdminServer(logger *zap.SugaredLogger, drain func()) *http.Server { +func buildAdminServer(logger *zap.SugaredLogger, drain func(), tls bool) *http.Server { adminMux := http.NewServeMux() adminMux.HandleFunc(queue.RequestQueueDrainPath, func(w http.ResponseWriter, r *http.Request) { logger.Info("Attached drain handler from user-container") drain() }) + if tls { + return &http.Server{ + // Open 8122 port. + Addr: ":" + strconv.Itoa(networking.QueueAdminPort+100), + Handler: adminMux, + } + } + return &http.Server{ Addr: ":" + strconv.Itoa(networking.QueueAdminPort), Handler: adminMux, diff --git a/pkg/activator/handler/handler.go b/pkg/activator/handler/handler.go index 85e72261e178..beb311006748 100644 --- a/pkg/activator/handler/handler.go +++ b/pkg/activator/handler/handler.go @@ -21,6 +21,8 @@ import ( "errors" "net/http" "net/http/httputil" + "strconv" + "strings" "go.opencensus.io/plugin/ochttp" "go.opencensus.io/trace" @@ -35,6 +37,7 @@ import ( "knative.dev/serving/pkg/activator" activatorconfig "knative.dev/serving/pkg/activator/config" pkghttp "knative.dev/serving/pkg/http" + "knative.dev/serving/pkg/networking" "knative.dev/serving/pkg/queue" "knative.dev/serving/pkg/reconciler/serverlessservice/resources/names" ) @@ -116,7 +119,12 @@ func (a *activationHandler) proxyRequest(revID types.NamespacedName, w http.Resp if usePassthroughLb { hostOverride = names.PrivateService(revID.Name) + "." + revID.Namespace } - proxy := pkghttp.NewHeaderPruningReverseProxy(target, hostOverride, activator.RevisionHeaders) + proxy := pkghttp.NewHeaderPruningReverseProxy(target, hostOverride, activator.RevisionHeaders, "http" /* use https to the target */) + + // TODO: Use configmap or annotation to enable secure proxy. + if true { + proxy = pkghttp.NewHeaderPruningReverseProxy(useSecurePort(target), hostOverride, activator.RevisionHeaders, "https" /* use https to the target */) + } proxy.BufferPool = a.bufferPool proxy.Transport = a.transport if tracingEnabled { @@ -129,3 +137,10 @@ func (a *activationHandler) proxyRequest(revID types.NamespacedName, w http.Resp proxy.ServeHTTP(w, r) } + +// useSecurePort replaces the default port with HTTPS port (8112). +// TODO: endpointsToDests() should support HTTPS instead of this hack but it needs metadata request to be encrypted. +func useSecurePort(target string) string { + target = strings.Split(target, ":")[0] + return target + ":" + strconv.Itoa(networking.BackendHTTPSPort) +} diff --git a/pkg/http/proxy.go b/pkg/http/proxy.go index 81805cedbde5..6054a20b2b83 100644 --- a/pkg/http/proxy.go +++ b/pkg/http/proxy.go @@ -33,10 +33,10 @@ const NoHostOverride = "" // If hostOverride is not an empty string, the outgoing request's Host header will be // replaced with that explicit value and the passthrough loadbalancing header will be // set to enable pod-addressability. -func NewHeaderPruningReverseProxy(target, hostOverride string, headersToRemove []string) *httputil.ReverseProxy { +func NewHeaderPruningReverseProxy(target, hostOverride string, headersToRemove []string, scheme string) *httputil.ReverseProxy { return &httputil.ReverseProxy{ Director: func(req *http.Request) { - req.URL.Scheme = "http" + req.URL.Scheme = scheme req.URL.Host = target if hostOverride != NoHostOverride { diff --git a/pkg/http/proxy_test.go b/pkg/http/proxy_test.go index a5e40bd3afac..636248926b5f 100644 --- a/pkg/http/proxy_test.go +++ b/pkg/http/proxy_test.go @@ -89,7 +89,7 @@ func TestNewHeaderPruningProxy(t *testing.T) { proxy := NewHeaderPruningReverseProxy(serverURL.Host, test.host, []string{ "header-to-remove-1", "header-to-remove-2", - }) + }, "http") resp := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, test.url, nil) diff --git a/pkg/networking/constants.go b/pkg/networking/constants.go index e85118bf939b..0fe1e5ea1048 100644 --- a/pkg/networking/constants.go +++ b/pkg/networking/constants.go @@ -26,6 +26,9 @@ const ( // BackendHTTP2Port is the backend, i.e. `targetPort` that we setup for HTTP/2 services. BackendHTTP2Port = 8013 + // BackendHTTPSPort is the backend. i.e. `targetPort` that we setup for HTTPS services. + BackendHTTPSPort = 8112 + // QueueAdminPort specifies the port number for // health check and lifecycle hooks for queue-proxy. QueueAdminPort = 8022 diff --git a/pkg/queue/constants.go b/pkg/queue/constants.go index 4d6b6f0573b0..4d08db5c1a86 100644 --- a/pkg/queue/constants.go +++ b/pkg/queue/constants.go @@ -26,4 +26,7 @@ const ( // Main usage is to delay the termination of user-container until all // accepted requests have been processed. RequestQueueDrainPath = "/wait-for-drain" + + // CertDirectory is the name of the directory path where certificates are stored. + CertDirectory = "/var/lib/knative/certs" ) diff --git a/pkg/reconciler/revision/resources/deploy.go b/pkg/reconciler/revision/resources/deploy.go index 997fc9715ad4..c4ce2282890b 100644 --- a/pkg/reconciler/revision/resources/deploy.go +++ b/pkg/reconciler/revision/resources/deploy.go @@ -70,6 +70,21 @@ var ( }, } + certVolumeMount = corev1.VolumeMount{ + MountPath: queue.CertDirectory, + Name: "server-certs", + ReadOnly: true, + } + + certVolume = corev1.Volume{ + Name: "server-certs", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "server-certs", + }, + }, + } + varTokenVolumeMount = corev1.VolumeMount{ Name: varTokenVolume.Name, MountPath: concurrencyStateTokenVolumeMountPath, @@ -122,6 +137,12 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error) extraVolumes = append(extraVolumes, varTokenVolume) } + // TODO: Use configmap to decide if mount cert volumes or not. + if true { + queueContainer.VolumeMounts = append(queueContainer.VolumeMounts, certVolumeMount) + extraVolumes = append(extraVolumes, certVolume) + } + podSpec := BuildPodSpec(rev, append(BuildUserContainers(rev), *queueContainer), cfg) podSpec.Volumes = append(podSpec.Volumes, extraVolumes...) diff --git a/test/config/tls/ca-volume-path.yaml b/test/config/tls/ca-volume-path.yaml new file mode 100644 index 000000000000..97673d419e2b --- /dev/null +++ b/test/config/tls/ca-volume-path.yaml @@ -0,0 +1,13 @@ +spec: + template: + spec: + volumes: + - name: client-ca + secret: + secretName: serving-ca + containers: + - name: activator + volumeMounts: + - name: client-ca + mountPath: /var/lib/knative/ca + readOnly: true diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 9546d0131424..bfc78f63d0fa 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -34,6 +34,7 @@ export RUN_HTTP01_AUTO_TLS_TESTS=0 export HTTPS=0 export SHORT=0 export ENABLE_HA=0 +export ENABLE_TLS=0 export MESH=0 export PERF=0 export KIND=0 @@ -351,6 +352,19 @@ function install() { # kubectl -n ${SYSTEM_NAMESPACE} delete leases --all wait_for_leader_controller || return 1 fi + + if (( ENABLE_TLS )); then + echo "Generate certificates" + bash ${REPO_ROOT_DIR}/test/generate-cert.sh + + # echo "Patch to activator to serve TLS" + # kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/volume-patch.yaml)" + # kubectl patch service -n ${SYSTEM_NAMESPACE} activator-service --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/service-patch.yaml)" + # kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml + + echo "Patch to activator to mount Client CA" + kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/ca-volume-path.yaml)" + fi } # Check if we should use --resolvabledomain. In case the ingress only has diff --git a/test/generate-cert.sh b/test/generate-cert.sh new file mode 100644 index 000000000000..5569f166ad32 --- /dev/null +++ b/test/generate-cert.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash + +# Copyright 2022 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. + +SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-knative-serving}" +TEST_NAMESPACE=serving-tests +TEST_NAMESPACE_ALT=serving-tests-alt +out_dir="$(mktemp -d /tmp/certs-XXX)" +san="knative" + +# Generate Root key and cert. +openssl req -x509 -sha256 -nodes -days 365 -newkey rsa:2048 -subj '/O=Example/CN=Example' -keyout "${out_dir}"/root.key -out "${out_dir}"/root.crt + +# Create server key +openssl req -out "${out_dir}"/tls.csr -newkey rsa:2048 -nodes -keyout "${out_dir}"/tls.key -subj "/CN=Example/O=Example" -addext "subjectAltName = DNS:$san" + +# Create server certs +openssl x509 -req -extfile <(printf "subjectAltName=DNS:$san") -days 365 -in "${out_dir}"/tls.csr -CA "${out_dir}"/root.crt -CAkey "${out_dir}"/root.key -CAcreateserial -out "${out_dir}"/tls.crt + +# Create secret +kubectl create -n ${SYSTEM_NAMESPACE} secret generic serving-ca \ + --from-file=ca.crt="${out_dir}"/root.crt --dry-run=client -o yaml | kubectl apply -f - + +kubectl create -n ${SYSTEM_NAMESPACE} secret tls server-certs \ + --key="${out_dir}"/tls.key \ + --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - + +# Create secrets for test namespaces +kubectl create -n ${TEST_NAMESPACE} secret tls server-certs \ + --key="${out_dir}"/tls.key \ + --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - + +kubectl create -n ${TEST_NAMESPACE_ALT} secret tls server-certs \ + --key="${out_dir}"/tls.key \ + --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - diff --git a/vendor/knative.dev/pkg/network/h2c.go b/vendor/knative.dev/pkg/network/h2c.go index f950b9c34fc2..6cc0fa733d95 100644 --- a/vendor/knative.dev/pkg/network/h2c.go +++ b/vendor/knative.dev/pkg/network/h2c.go @@ -54,3 +54,16 @@ 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 { + return &http2.Transport{ + DisableCompression: disableCompression, + DialTLS: func(netw, addr string, tlsConf *tls.Config) (net.Conn, error) { + return DialTLSWithBackOff(context.Background(), + netw, addr, tlsConf) + }, + TLSClientConfig: tlsConf, + } +} diff --git a/vendor/knative.dev/pkg/network/transports.go b/vendor/knative.dev/pkg/network/transports.go index 26ce82395760..da60ae803f2d 100644 --- a/vendor/knative.dev/pkg/network/transports.go +++ b/vendor/knative.dev/pkg/network/transports.go @@ -18,6 +18,7 @@ package network import ( "context" + "crypto/tls" "errors" "fmt" "net" @@ -54,8 +55,7 @@ var backOffTemplate = wait.Backoff{ Steps: 15, } -// DialWithBackOff executes `net.Dialer.DialContext()` with exponentially increasing -// dial timeouts. In addition it sleeps with random jitter between tries. +// DialWithBackOff executes `net.Dialer.DialContext()` with exponentially increasing // dial timeouts. In addition it sleeps with random jitter between tries. var DialWithBackOff = NewBackoffDialer(backOffTemplate) // NewBackoffDialer returns a dialer that executes `net.Dialer.DialContext()` with @@ -63,11 +63,21 @@ var DialWithBackOff = NewBackoffDialer(backOffTemplate) // between tries. func NewBackoffDialer(backoffConfig wait.Backoff) func(context.Context, string, string) (net.Conn, error) { return func(ctx context.Context, network, address string) (net.Conn, error) { - return dialBackOffHelper(ctx, network, address, backoffConfig, sleepTO) + return dialBackOffHelper(ctx, network, address, backoffConfig, sleepTO, nil) } } -func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Backoff, sleep time.Duration) (net.Conn, error) { +// DialTLSWithBackOff is same with DialWithBackOff but takes tls config. +var DialTLSWithBackOff = NewTLSBackoffDialer(backOffTemplate) + +// NewTLSBackoffDialer is same with NewBackoffDialer but takes tls config. +func NewTLSBackoffDialer(backoffConfig wait.Backoff) func(context.Context, string, string, *tls.Config) (net.Conn, error) { + return func(ctx context.Context, network, address string, tlsConf *tls.Config) (net.Conn, error) { + return dialBackOffHelper(ctx, network, address, backoffConfig, sleepTO, tlsConf) + } +} + +func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Backoff, sleep time.Duration, tlsConf *tls.Config) (net.Conn, error) { dialer := &net.Dialer{ Timeout: bo.Duration, // Initial duration. KeepAlive: 5 * time.Second, @@ -75,7 +85,15 @@ func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Bac } start := time.Now() for { - c, err := dialer.DialContext(ctx, network, address) + var ( + c net.Conn + err error + ) + if tlsConf == nil { + c, err = dialer.DialContext(ctx, network, address) + } else { + c, err = tls.DialWithDialer(dialer, network, address, tlsConf) + } if err != nil { var errNet net.Error if errors.As(err, &errNet) && errNet.Timeout() { @@ -105,6 +123,19 @@ func newHTTPTransport(disableKeepAlives, disableCompression bool, maxIdle, maxId return transport } +func newHTTPSTransport(disableKeepAlives, disableCompression bool, maxIdle, maxIdlePerHost int, tlsConf *tls.Config) 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.TLSClientConfig = tlsConf + return transport +} + // NewProberTransport creates a RoundTripper that is useful for probing, // since it will not cache connections. func NewProberTransport() http.RoundTripper { @@ -113,6 +144,13 @@ 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 { + return newAutoTransport( + newHTTPSTransport(false /*disable keep-alives*/, true /*disable auto-compression*/, maxIdle, maxIdlePerHost, tlsConf), + newH2Transport(true /*disable auto-compression*/, tlsConf)) +} + // NewAutoTransport creates a RoundTripper that can use appropriate transport // based on the request's HTTP version. func NewAutoTransport(maxIdle, maxIdlePerHost int) http.RoundTripper { From 065c10a6806b43efb6a46c4f71f93e877b45ebac Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 5 Apr 2022 19:06:11 +0900 Subject: [PATCH 2/4] Add queue-proxy server cert config --- .github/workflows/kind-e2e.yaml | 1 + cmd/activator/main.go | 46 +++++++++----------- cmd/queue/main.go | 39 ++++++++--------- pkg/activator/handler/handler.go | 6 ++- pkg/activator/handler/handler_test.go | 10 ++--- pkg/activator/handler/main_test.go | 2 +- pkg/reconciler/revision/resources/deploy.go | 25 ++++++----- pkg/reconciler/revision/resources/queue.go | 19 ++++++-- test/config/tls/ca-volume-path.yaml | 13 ------ test/config/tls/config-network.yaml | 25 +++++++++++ test/e2e-common.sh | 5 ++- vendor/knative.dev/networking/pkg/network.go | 45 +++++++++++++++++++ 12 files changed, 152 insertions(+), 84 deletions(-) delete mode 100644 test/config/tls/ca-volume-path.yaml create mode 100644 test/config/tls/config-network.yaml diff --git a/.github/workflows/kind-e2e.yaml b/.github/workflows/kind-e2e.yaml index 321eaa584b73..41f8edc030c6 100644 --- a/.github/workflows/kind-e2e.yaml +++ b/.github/workflows/kind-e2e.yaml @@ -203,6 +203,7 @@ jobs: KIND=1 INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" + if [[ "${{ matrix.cluster }}" == *-tls ]]; then echo 'Enabling TLS' ENABLE_TLS=1 diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 35d144a464a3..1c23cb6833af 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -22,7 +22,6 @@ import ( "crypto/x509" "errors" "fmt" - "io/ioutil" "log" "net/http" "os" @@ -71,9 +70,6 @@ const ( // The port on which autoscaler WebSocket server listens. autoscalerPort = ":8080" - - caDirectory = "/var/lib/knative/ca" - caPath = caDirectory + "/ca.crt" ) type config struct { @@ -147,44 +143,43 @@ func main() { logger.Debugf("MaxIdleProxyConns: %d, MaxIdleProxyConnsPerHost: %d", env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost) transport := pkgnet.NewProxyAutoTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost) - // Enable TLS client when activator client ca is mounted. + // Fetch networking configuration to determine whether EnableMeshPodAddressability + // is enabled or not. + networkCM, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, network.ConfigName, metav1.GetOptions{}) + if err != nil { + logger.Fatalw("Failed to fetch network config", zap.Error(err)) + } + networkConfig, err := network.NewConfigFromConfigMap(networkCM) + if err != nil { + logger.Fatalw("Failed to construct network config", zap.Error(err)) + } + + // Enable TLS client when queue-proxy-ca is specified. // At this moment activator with TLS does not disable HTTP. // See also https://github.com/knative/serving/issues/12808. - if exists(logger, caPath) { - pool, err := x509.SystemCertPool() + if networkConfig.QueueProxyCA != "" && networkConfig.QueueProxySAN != "" { + caSecret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, networkConfig.QueueProxyCA, metav1.GetOptions{}) if err != nil { - pool = x509.NewCertPool() + logger.Fatalw("failed to get secret", zap.Error(err)) } - - caCert, err := ioutil.ReadFile(caPath) + pool, err := x509.SystemCertPool() if err != nil { - log.Fatalf("failed to append %q to RootCAs: %v", caPath, err) + pool = x509.NewCertPool() } - if ok := pool.AppendCertsFromPEM(caCert); !ok { + if ok := pool.AppendCertsFromPEM(caSecret.Data["ca.crt"]); !ok { logger.Fatalw("failed to append ca cert to the RootCAs") } tlsConf := &tls.Config{ RootCAs: pool, InsecureSkipVerify: false, - ServerName: "knative", + ServerName: networkConfig.QueueProxySAN, MinVersion: tls.VersionTLS12, } transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, tlsConf) } - // Fetch networking configuration to determine whether EnableMeshPodAddressability - // is enabled or not. - networkCM, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, network.ConfigName, metav1.GetOptions{}) - if err != nil { - logger.Fatalw("Failed to fetch network config", zap.Error(err)) - } - networkConfig, err := network.NewConfigFromConfigMap(networkCM) - if err != nil { - logger.Fatalw("Failed to construct network config", zap.Error(err)) - } - // Start throttler. throttler := activatornet.NewThrottler(ctx, env.PodIP) go throttler.Run(ctx, transport, networkConfig.EnableMeshPodAddressability, networkConfig.MeshCompatibilityMode) @@ -220,7 +215,8 @@ func main() { // Create activation handler chain // Note: innermost handlers are specified first, ie. the last handler in the chain will be executed first - ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger) + tlsEnabled := networkConfig.QueueProxyCA != "" && networkConfig.QueueProxySAN != "" // TODO: + ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger, tlsEnabled) ah = concurrencyReporter.Handler(ah) ah = activatorhandler.NewTracingHandler(ah) reqLogHandler, err := pkghttp.NewRequestLogHandler(ah, logging.NewSyncFileWriter(os.Stdout), "", diff --git a/cmd/queue/main.go b/cmd/queue/main.go index b32d5a657458..cf976e270914 100644 --- a/cmd/queue/main.go +++ b/cmd/queue/main.go @@ -70,6 +70,7 @@ const ( type config struct { ContainerConcurrency int `split_words:"true" required:"true"` QueueServingPort string `split_words:"true" required:"true"` + QueueServingTLSPort string `split_words:"true" required:"true"` UserPort string `split_words:"true" required:"true"` RevisionTimeoutSeconds int `split_words:"true" required:"true"` MaxDurationSeconds int `split_words:"true"` // optional @@ -168,13 +169,16 @@ func main() { mainServer, drain := buildServer(ctx, env, probe, stats, logger, concurrencyendpoint, false) servers := map[string]*http.Server{ "main": mainServer, - "admin": buildAdminServer(logger, drain, false), "metrics": buildMetricsServer(promStatReporter, protoStatReporter), } if env.EnableProfiling { servers["profile"] = profiling.NewServer(profiling.NewHandler(logger, true)) } + if !exists(logger, certPath) || !exists(logger, keyPath) { + servers["admin"] = buildAdminServer(logger, drain) + } + errCh := make(chan error) for name, server := range servers { go func(name string, s *http.Server) { @@ -189,10 +193,10 @@ func main() { // At this moment activator with TLS does not disable HTTP. // See also https://github.com/knative/serving/issues/12808. if exists(logger, certPath) && exists(logger, keyPath) { - mainTLSServer, drain := buildServer(ctx, env, probe, stats, logger, concurrencyendpoint, true) + mainTLSServer, drain := buildServer(ctx, env, probe, stats, logger, concurrencyendpoint, true /* enable TLS */) tlsServers := map[string]*http.Server{ "tlsMain": mainTLSServer, - "tlsAdmin": buildAdminServer(logger, drain, true), + "tlsAdmin": buildAdminServer(logger, drain), } for name, server := range tlsServers { go func(name string, s *http.Server) { @@ -254,7 +258,7 @@ func buildProbe(logger *zap.SugaredLogger, encodedProbe string, autodetectHTTP2 } func buildServer(ctx context.Context, env config, probeContainer func() bool, stats *network.RequestStats, logger *zap.SugaredLogger, - ce *queue.ConcurrencyEndpoint, tls bool) (server *http.Server, drain func()) { + ce *queue.ConcurrencyEndpoint, enableTLS bool) (server *http.Server, drain func()) { target := net.JoinHostPort("127.0.0.1", env.UserPort) @@ -265,7 +269,11 @@ func buildServer(ctx context.Context, env config, probeContainer func() bool, st httpProxy.FlushInterval = network.FlushInterval breaker := buildBreaker(logger, env) - metricsSupported := supportsMetrics(ctx, logger, env, tls) + metricsSupported := supportsMetrics(ctx, logger, env) + // Metrics HTTP and non-HTTPS + if enableTLS { + metricsSupported = false + } tracingEnabled := env.TracingConfigBackend != tracingconfig.None concurrencyStateEnabled := env.ConcurrencyStateEndpoint != "" firstByteTimeout := time.Duration(env.RevisionTimeoutSeconds) * time.Second @@ -317,9 +325,8 @@ func buildServer(ctx context.Context, env config, probeContainer func() bool, st composedHandler = requestLogHandler(logger, composedHandler, env) } - if tls { - // Open +100 (8112) port. - return pkgnet.NewServer(":8112", composedHandler), drainer.Drain + if enableTLS { + return pkgnet.NewServer(":"+env.QueueServingTLSPort, composedHandler), drainer.Drain } return pkgnet.NewServer(":"+env.QueueServingPort, composedHandler), drainer.Drain @@ -368,13 +375,11 @@ func buildBreaker(logger *zap.SugaredLogger, env config) *queue.Breaker { return queue.NewBreaker(params) } -func supportsMetrics(ctx context.Context, logger *zap.SugaredLogger, env config, tls bool) bool { +func supportsMetrics(ctx context.Context, logger *zap.SugaredLogger, env config) bool { // Setup request metrics reporting for end-user metrics. - // TODO: enable Metrics for TLS enabled. - if env.ServingRequestMetricsBackend == "" || tls { + if env.ServingRequestMetricsBackend == "" { return false } - if err := setupMetricsExporter(ctx, logger, env.ServingRequestMetricsBackend, env.MetricsCollectorAddress); err != nil { logger.Errorw("Error setting up request metrics exporter. Request metrics will be unavailable.", zap.Error(err)) return false @@ -383,21 +388,13 @@ func supportsMetrics(ctx context.Context, logger *zap.SugaredLogger, env config, return true } -func buildAdminServer(logger *zap.SugaredLogger, drain func(), tls bool) *http.Server { +func buildAdminServer(logger *zap.SugaredLogger, drain func()) *http.Server { adminMux := http.NewServeMux() adminMux.HandleFunc(queue.RequestQueueDrainPath, func(w http.ResponseWriter, r *http.Request) { logger.Info("Attached drain handler from user-container") drain() }) - if tls { - return &http.Server{ - // Open 8122 port. - Addr: ":" + strconv.Itoa(networking.QueueAdminPort+100), - Handler: adminMux, - } - } - return &http.Server{ Addr: ":" + strconv.Itoa(networking.QueueAdminPort), Handler: adminMux, diff --git a/pkg/activator/handler/handler.go b/pkg/activator/handler/handler.go index beb311006748..7cd88c732d20 100644 --- a/pkg/activator/handler/handler.go +++ b/pkg/activator/handler/handler.go @@ -56,10 +56,11 @@ type activationHandler struct { throttler Throttler bufferPool httputil.BufferPool logger *zap.SugaredLogger + tls bool } // New constructs a new http.Handler that deals with revision activation. -func New(_ context.Context, t Throttler, transport http.RoundTripper, usePassthroughLb bool, logger *zap.SugaredLogger) http.Handler { +func New(_ context.Context, t Throttler, transport http.RoundTripper, usePassthroughLb bool, logger *zap.SugaredLogger, tlsEnabled bool) http.Handler { return &activationHandler{ transport: transport, tracingTransport: &ochttp.Transport{ @@ -70,6 +71,7 @@ func New(_ context.Context, t Throttler, transport http.RoundTripper, usePassthr throttler: t, bufferPool: network.NewBufferPool(), logger: logger, + tls: tlsEnabled, } } @@ -122,7 +124,7 @@ func (a *activationHandler) proxyRequest(revID types.NamespacedName, w http.Resp proxy := pkghttp.NewHeaderPruningReverseProxy(target, hostOverride, activator.RevisionHeaders, "http" /* use https to the target */) // TODO: Use configmap or annotation to enable secure proxy. - if true { + if a.tls { proxy = pkghttp.NewHeaderPruningReverseProxy(useSecurePort(target), hostOverride, activator.RevisionHeaders, "https" /* use https to the target */) } proxy.BufferPool = a.bufferPool diff --git a/pkg/activator/handler/handler_test.go b/pkg/activator/handler/handler_test.go index 33ef6a571d6a..2481675391d6 100644 --- a/pkg/activator/handler/handler_test.go +++ b/pkg/activator/handler/handler_test.go @@ -123,7 +123,7 @@ func TestActivationHandler(t *testing.T) { ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(t) defer cancel() - handler := New(ctx, test.throttler, rt, false /*usePassthroughLb*/, logging.FromContext(ctx)) + handler := New(ctx, test.throttler, rt, false /*usePassthroughLb*/, logging.FromContext(ctx), false /* TLS */) resp := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) @@ -162,7 +162,7 @@ func TestActivationHandlerProxyHeader(t *testing.T) { ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(t) defer cancel() - handler := New(ctx, fakeThrottler{}, rt, false /*usePassthroughLb*/, logging.FromContext(ctx)) + handler := New(ctx, fakeThrottler{}, rt, false /*usePassthroughLb*/, logging.FromContext(ctx), false /* TLS */) writer := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) @@ -195,7 +195,7 @@ func TestActivationHandlerPassthroughLb(t *testing.T) { ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(t) defer cancel() - handler := New(ctx, fakeThrottler{}, rt, true /*usePassthroughLb*/, logging.FromContext(ctx)) + handler := New(ctx, fakeThrottler{}, rt, true /*usePassthroughLb*/, logging.FromContext(ctx), false /* TLS */) writer := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) @@ -276,7 +276,7 @@ func TestActivationHandlerTraceSpans(t *testing.T) { oct.Finish() }() - handler := New(ctx, fakeThrottler{}, rt, false /*usePassthroughLb*/, logging.FromContext(ctx)) + handler := New(ctx, fakeThrottler{}, rt, false /*usePassthroughLb*/, logging.FromContext(ctx), false /* TLS */) // Set up config store to populate context. configStore := setupConfigStore(t, logging.FromContext(ctx)) @@ -345,7 +345,7 @@ func BenchmarkHandler(b *testing.B) { }, nil }) - handler := New(ctx, fakeThrottler{}, rt, false /*usePassthroughLb*/, logging.FromContext(ctx)) + handler := New(ctx, fakeThrottler{}, rt, false /*usePassthroughLb*/, logging.FromContext(ctx), false /* TLS */) request := func() *http.Request { req := httptest.NewRequest(http.MethodGet, "http://example.com", nil) diff --git a/pkg/activator/handler/main_test.go b/pkg/activator/handler/main_test.go index f8fe4edfea75..98b44b244413 100644 --- a/pkg/activator/handler/main_test.go +++ b/pkg/activator/handler/main_test.go @@ -69,7 +69,7 @@ func BenchmarkHandlerChain(b *testing.B) { }) // Make sure to update this if the activator's main file changes. - ah := New(ctx, fakeThrottler{}, rt, false, logger) + ah := New(ctx, fakeThrottler{}, rt, false, logger, false /* TLS */) ah = concurrencyReporter.Handler(ah) ah = NewTracingHandler(ah) ah, _ = pkghttp.NewRequestLogHandler(ah, io.Discard, "", nil, false) diff --git a/pkg/reconciler/revision/resources/deploy.go b/pkg/reconciler/revision/resources/deploy.go index c4ce2282890b..8a60e351cf23 100644 --- a/pkg/reconciler/revision/resources/deploy.go +++ b/pkg/reconciler/revision/resources/deploy.go @@ -76,15 +76,6 @@ var ( ReadOnly: true, } - certVolume = corev1.Volume{ - Name: "server-certs", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "server-certs", - }, - }, - } - varTokenVolumeMount = corev1.VolumeMount{ Name: varTokenVolume.Name, MountPath: concurrencyStateTokenVolumeMountPath, @@ -104,6 +95,17 @@ var ( } ) +func certVolume(secret string) corev1.Volume { + return corev1.Volume{ + Name: "server-certs", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secret, + }, + }, + } +} + func rewriteUserProbe(p *corev1.Probe, userPort int) { if p == nil { return @@ -137,10 +139,9 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error) extraVolumes = append(extraVolumes, varTokenVolume) } - // TODO: Use configmap to decide if mount cert volumes or not. - if true { + if cfg.Network.QueueProxyServerCert != "" { queueContainer.VolumeMounts = append(queueContainer.VolumeMounts, certVolumeMount) - extraVolumes = append(extraVolumes, certVolume) + extraVolumes = append(extraVolumes, certVolume(cfg.Network.QueueProxyServerCert)) } podSpec := BuildPodSpec(rev, append(BuildUserContainers(rev), *queueContainer), cfg) diff --git a/pkg/reconciler/revision/resources/queue.go b/pkg/reconciler/revision/resources/queue.go index d5fe68df1918..44ee93771450 100644 --- a/pkg/reconciler/revision/resources/queue.go +++ b/pkg/reconciler/revision/resources/queue.go @@ -44,9 +44,10 @@ import ( ) const ( - localAddress = "127.0.0.1" - requestQueueHTTPPortName = "queue-port" - profilingPortName = "profiling-port" + localAddress = "127.0.0.1" + requestQueueHTTPPortName = "queue-port" + requestQueueHTTPSPortName = "https-port" // must be no more than 15 characters. + profilingPortName = "profiling-port" ) var ( @@ -58,6 +59,10 @@ var ( Name: requestQueueHTTPPortName, ContainerPort: networking.BackendHTTP2Port, } + queueHTTPSPort = corev1.ContainerPort{ + Name: requestQueueHTTPSPortName, + ContainerPort: networking.BackendHTTPSPort, + } queueNonServingPorts = []corev1.ContainerPort{{ // Provides health checks and lifecycle hooks. Name: v1.QueueAdminPortName, @@ -204,6 +209,11 @@ func makeQueueContainer(rev *v1.Revision, cfg *config.Config) (*corev1.Container } ports = append(ports, servingPort) + // TODO: use configmap + if true { + ports = append(ports, queueHTTPSPort) + } + container := rev.Spec.GetContainer() var httpProbe, execProbe *corev1.Probe @@ -269,6 +279,9 @@ func makeQueueContainer(rev *v1.Revision, cfg *config.Config) (*corev1.Container }, { Name: "QUEUE_SERVING_PORT", Value: strconv.Itoa(int(servingPort.ContainerPort)), + }, { + Name: "QUEUE_SERVING_TLS_PORT", + Value: strconv.Itoa(int(queueHTTPSPort.ContainerPort)), }, { Name: "CONTAINER_CONCURRENCY", Value: strconv.Itoa(int(rev.Spec.GetContainerConcurrency())), diff --git a/test/config/tls/ca-volume-path.yaml b/test/config/tls/ca-volume-path.yaml deleted file mode 100644 index 97673d419e2b..000000000000 --- a/test/config/tls/ca-volume-path.yaml +++ /dev/null @@ -1,13 +0,0 @@ -spec: - template: - spec: - volumes: - - name: client-ca - secret: - secretName: serving-ca - containers: - - name: activator - volumeMounts: - - name: client-ca - mountPath: /var/lib/knative/ca - readOnly: true diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml new file mode 100644 index 000000000000..5a2cad59d9db --- /dev/null +++ b/test/config/tls/config-network.yaml @@ -0,0 +1,25 @@ +# Copyright 2022 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 +# +# https://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. +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-network + labels: + app.kubernetes.io/name: knative-serving + app.kubernetes.io/version: devel + serving.knative.dev/release: devel +data: + queue-proxy-ca: "serving-ca" + queue-proxy-san: "knative" + queue-proxy-server-certs: "server-certs" diff --git a/test/e2e-common.sh b/test/e2e-common.sh index bfc78f63d0fa..17325ba9d947 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -356,14 +356,15 @@ function install() { if (( ENABLE_TLS )); then echo "Generate certificates" bash ${REPO_ROOT_DIR}/test/generate-cert.sh + kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml # echo "Patch to activator to serve TLS" # kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/volume-patch.yaml)" # kubectl patch service -n ${SYSTEM_NAMESPACE} activator-service --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/service-patch.yaml)" # kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml - echo "Patch to activator to mount Client CA" - kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/ca-volume-path.yaml)" + # echo "Patch to activator to mount Client CA" + # kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/ca-volume-path.yaml)" fi } diff --git a/vendor/knative.dev/networking/pkg/network.go b/vendor/knative.dev/networking/pkg/network.go index 26a2633ca1bf..89aa84abcb74 100644 --- a/vendor/knative.dev/networking/pkg/network.go +++ b/vendor/knative.dev/networking/pkg/network.go @@ -196,6 +196,21 @@ const ( // ActivatorSANKey is the config for the SAN used to validate the activator TLS certificate. ActivatorSANKey = "activator-san" + + // ActivatorServerCertKey is the config for the secret name, which stores certificates + // to serve the TLS traffic from ingress to activator. + ActivatorServerCertKey = "activator-server-certs" + + // QueueProxyCAKey is the config for the secret name, which stores CA public certificate used + // to sign the queue-proxy TLS certificate. + QueueProxyCAKey = "queue-proxy-ca" + + // QueueProxySANKey is the config for the SAN used to validate the queue-proxy TLS certificate. + QueueProxySANKey = "queue-proxy-san" + + // QueueProxyServerCertKey is the config for the secret name, which stores certificates + // to serve the TLS traffic from activator to queue-proxy. + QueueProxyServerCertKey = "queue-proxy-server-certs" ) // DomainTemplateValues are the available properties people can choose from @@ -302,6 +317,20 @@ type Config struct { // ActivatorSAN defines the SAN (Subject Alt Name) used to validate the activator TLS certificate. // It is used only when ActivatorCA is specified. ActivatorSAN string + + // ActivatorSererCert defines the secret name of the server certificates to serve the TLS traffic from ingress to activator. + ActivatorServerCert string + + // QueueProxyCA defines the secret name of the CA public certificate used to sign the queue-proxy TLS certificate. + // The traffic to queue-proxy is not encrypted if QueueProxyCA is empty. + QueueProxyCA string + + // QueueProxySAN defines the SAN (Subject Alt Name) used to validate the queue-proxy TLS certificate. + // It is used only when QueueProxyCA is specified. + QueueProxySAN string + + // QueueProxyServerCert defines the secret name of the server certificates to serve the TLS traffic from activator to queue-proxy. + QueueProxyServerCert string } // HTTPProtocol indicates a type of HTTP endpoint behavior @@ -359,6 +388,10 @@ func defaultConfig() *Config { MeshCompatibilityMode: MeshCompatibilityModeAuto, ActivatorCA: "", ActivatorSAN: "", + ActivatorServerCert: "", + QueueProxyCA: "", + QueueProxySAN: "", + QueueProxyServerCert: "", } } @@ -392,6 +425,10 @@ func NewConfigFromMap(data map[string]string) (*Config, error) { cm.AsString(DefaultExternalSchemeKey, &nc.DefaultExternalScheme), cm.AsString(ActivatorCAKey, &nc.ActivatorCA), cm.AsString(ActivatorSANKey, &nc.ActivatorSAN), + cm.AsString(ActivatorServerCertKey, &nc.ActivatorServerCert), + cm.AsString(QueueProxyCAKey, &nc.QueueProxyCA), + cm.AsString(QueueProxySANKey, &nc.QueueProxySAN), + cm.AsString(QueueProxyServerCertKey, &nc.QueueProxyServerCert), asMode(MeshCompatibilityModeKey, &nc.MeshCompatibilityMode), asLabelSelector(NamespaceWildcardCertSelectorKey, &nc.NamespaceWildcardCertSelector), ); err != nil { @@ -456,6 +493,14 @@ func NewConfigFromMap(data map[string]string) (*Config, error) { return nil, fmt.Errorf("%q must be set when %q was set", ActivatorCAKey, ActivatorSANKey) } + if nc.QueueProxyCA != "" && nc.QueueProxySAN == "" { + return nil, fmt.Errorf("%q must be set when %q was set", QueueProxySANKey, QueueProxyCAKey) + } + + if nc.QueueProxyCA == "" && nc.QueueProxySAN != "" { + return nil, fmt.Errorf("%q must be set when %q was set", QueueProxyCAKey, QueueProxySANKey) + } + return nc, nil } From 7f1c83f8e360e2bc9e2c0a31a68fc252db09e3a8 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 7 Apr 2022 14:12:26 +0900 Subject: [PATCH 3/4] Revert activator's TLS change. --- cmd/activator/main.go | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 1c23cb6833af..1ae3c9c640d2 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -18,8 +18,6 @@ package main import ( "context" - "crypto/tls" - "crypto/x509" "errors" "fmt" "log" @@ -154,32 +152,6 @@ func main() { logger.Fatalw("Failed to construct network config", zap.Error(err)) } - // Enable TLS client when queue-proxy-ca is specified. - // At this moment activator with TLS does not disable HTTP. - // See also https://github.com/knative/serving/issues/12808. - if networkConfig.QueueProxyCA != "" && networkConfig.QueueProxySAN != "" { - caSecret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, networkConfig.QueueProxyCA, metav1.GetOptions{}) - if err != nil { - logger.Fatalw("failed to get secret", zap.Error(err)) - } - pool, err := x509.SystemCertPool() - if err != nil { - pool = x509.NewCertPool() - } - - if ok := pool.AppendCertsFromPEM(caSecret.Data["ca.crt"]); !ok { - logger.Fatalw("failed to append ca cert to the RootCAs") - } - - tlsConf := &tls.Config{ - RootCAs: pool, - InsecureSkipVerify: false, - ServerName: networkConfig.QueueProxySAN, - MinVersion: tls.VersionTLS12, - } - transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, tlsConf) - } - // Start throttler. throttler := activatornet.NewThrottler(ctx, env.PodIP) go throttler.Run(ctx, transport, networkConfig.EnableMeshPodAddressability, networkConfig.MeshCompatibilityMode) @@ -215,8 +187,7 @@ func main() { // Create activation handler chain // Note: innermost handlers are specified first, ie. the last handler in the chain will be executed first - tlsEnabled := networkConfig.QueueProxyCA != "" && networkConfig.QueueProxySAN != "" // TODO: - ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger, tlsEnabled) + ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger) ah = concurrencyReporter.Handler(ah) ah = activatorhandler.NewTracingHandler(ah) reqLogHandler, err := pkghttp.NewRequestLogHandler(ah, logging.NewSyncFileWriter(os.Stdout), "", From 9e79b2ff048765242e253d6334f4403ca6f7c915 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 7 Apr 2022 14:35:23 +0900 Subject: [PATCH 4/4] Disable activator's TLS client --- cmd/activator/main.go | 5 ++++- test/config/tls/config-network.yaml | 2 -- test/e2e-common.sh | 8 -------- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 1ae3c9c640d2..9860e864c2b3 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -187,7 +187,10 @@ func main() { // Create activation handler chain // Note: innermost handlers are specified first, ie. the last handler in the chain will be executed first - ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger) + + // Disable TLS for now. + tlsEnabled := false + ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger, tlsEnabled) ah = concurrencyReporter.Handler(ah) ah = activatorhandler.NewTracingHandler(ah) reqLogHandler, err := pkghttp.NewRequestLogHandler(ah, logging.NewSyncFileWriter(os.Stdout), "", diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml index 5a2cad59d9db..97e5ef78c2ae 100644 --- a/test/config/tls/config-network.yaml +++ b/test/config/tls/config-network.yaml @@ -20,6 +20,4 @@ metadata: app.kubernetes.io/version: devel serving.knative.dev/release: devel data: - queue-proxy-ca: "serving-ca" - queue-proxy-san: "knative" queue-proxy-server-certs: "server-certs" diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 17325ba9d947..16b0fd301630 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -357,14 +357,6 @@ function install() { echo "Generate certificates" bash ${REPO_ROOT_DIR}/test/generate-cert.sh kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml - - # echo "Patch to activator to serve TLS" - # kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/volume-patch.yaml)" - # kubectl patch service -n ${SYSTEM_NAMESPACE} activator-service --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/service-patch.yaml)" - # kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml - - # echo "Patch to activator to mount Client CA" - # kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/ca-volume-path.yaml)" fi }