Skip to content
Closed
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
2c7446e
initial changes
davidhadas May 3, 2023
cd59288
poc for tls and mtls
davidhadas May 9, 2023
667b62f
cleanup
davidhadas May 9, 2023
3c78734
add support for dialer
davidhadas May 9, 2023
57554c8
ensure support of all ingresses
davidhadas May 9, 2023
c00adf1
add non tls support, get most tests to pass
davidhadas May 11, 2023
5fc0409
http2 support
davidhadas May 13, 2023
ae63ea2
fix lint item
davidhadas May 13, 2023
0a1305a
fix lint item
davidhadas May 13, 2023
0722ef3
reorg
davidhadas May 13, 2023
d666360
back to RoundTripper interface - solve tests
davidhadas May 14, 2023
68c65f6
Merge branch 'knative:main' into dataplane-trust
davidhadas May 14, 2023
83a30a7
nits
davidhadas May 14, 2023
4f40a36
nits
davidhadas May 14, 2023
22f1d06
nits
davidhadas May 14, 2023
6fc785c
fix bug added on prev commit
davidhadas May 14, 2023
a69c439
merge
davidhadas May 14, 2023
d6e22e7
merge
davidhadas May 14, 2023
96644d9
merge
davidhadas May 14, 2023
31f5a38
fix non-tls http2 dial
davidhadas May 14, 2023
449dc96
support reload of activator server-side client rootcas
davidhadas May 14, 2023
4908795
cleanups
davidhadas May 14, 2023
eca4205
fix bug in updateCache + nits
davidhadas May 23, 2023
70f977f
Merge branch 'knative:main' into dataplane-trust
davidhadas May 31, 2023
250b31c
improved errors and logs, removed coe constants when networking const…
davidhadas May 31, 2023
61286df
fix sever side CA update
davidhadas Jun 3, 2023
f786633
merge
davidhadas Jun 3, 2023
ccd7045
avoid adding CAs already included
davidhadas Jun 3, 2023
b886b11
fix san of user certificate
davidhadas Jun 3, 2023
b789f23
moved to AppendCertsFromPEM
davidhadas Jun 13, 2023
516b899
Merge branch 'knative:main' into dataplane-trust
davidhadas Jun 13, 2023
fc23dd6
update certificates dependency
davidhadas Jun 13, 2023
bbbfbb5
tests
davidhadas Jun 13, 2023
be5c882
tests
davidhadas Jun 13, 2023
ccaaf6e
assume ingresses already send updated dataplane certificates
davidhadas Jun 16, 2023
f8be5dc
assume ingresses already send updated dataplane certificates
davidhadas Jun 16, 2023
01804ac
lock certiricate pool
davidhadas Jun 16, 2023
64683a7
lock certiricate pool
davidhadas Jun 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions cmd/activator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package main

import (
"context"
"crypto/tls"
"errors"
"fmt"
"log"
Expand All @@ -32,6 +31,7 @@ import (
"go.uber.org/zap"

// Injection related imports.

Comment thread
dprotaso marked this conversation as resolved.
Outdated
kubeclient "knative.dev/pkg/client/injection/kube/client"
"knative.dev/pkg/injection"
"knative.dev/serving/pkg/activator"
Expand Down Expand Up @@ -145,7 +145,6 @@ func main() {
// (via keep-alive) to send real requests, avoiding needing an extra
// reconnect for the first request after the probe succeeds.
logger.Debugf("MaxIdleProxyConns: %d, MaxIdleProxyConnsPerHost: %d", env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost)
transport := pkgnet.NewProxyAutoTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost)

// Fetch networking configuration to determine whether EnableMeshPodAddressability
// is enabled or not.
Expand All @@ -158,23 +157,24 @@ func main() {
logger.Fatalw("Failed to construct network config", zap.Error(err))
}

// Enable TLS against queue-proxy when internal-encryption is enabled.
tlsEnabled := networkConfig.InternalEncryption

// Enable TLS both as client and as server if DataplaneTrust != TrustDisabled
tlsEnabled := networkConfig.DataplaneTrust != netcfg.TrustDisabled
var roundtrip http.RoundTripper
var certCache *certificate.CertCache

// 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 tlsEnabled {
logger.Info("Internal Encryption is enabled")
certCache = certificate.NewCertCache(ctx)
transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, &certCache.TLSConf)
logger.Info("Dataplane trust %q is used", networkConfig.DataplaneTrust)
certCache = certificate.NewCertCache(ctx, networkConfig.DataplaneTrust)
Comment thread
davidhadas marked this conversation as resolved.
roundtrip = activatorhandler.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, &certCache.ClientTLSConf)
} else {
roundtrip = activatorhandler.NewProxyAutoTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost)
Comment thread
dprotaso marked this conversation as resolved.
Outdated
}

// Start throttler.
throttler := activatornet.NewThrottler(ctx, env.PodIP)
go throttler.Run(ctx, transport, networkConfig.EnableMeshPodAddressability, networkConfig.MeshCompatibilityMode)
go throttler.Run(ctx, roundtrip, networkConfig.EnableMeshPodAddressability, networkConfig.MeshCompatibilityMode)

oct := tracing.NewOpenCensusTracer(tracing.WithExporterFull(networking.ActivatorServiceName, env.PodIP, logger))
defer oct.Shutdown(context.Background())
Expand All @@ -189,7 +189,7 @@ func main() {

// Set up our config store
configMapWatcher := configmapinformer.NewInformedWatcher(kubeClient, system.Namespace())
configStore := activatorconfig.NewStore(logger, tracerUpdater)
configStore := activatorconfig.NewStore(logger, networkConfig.DataplaneTrust, tracerUpdater)
configStore.WatchConfigs(configMapWatcher)

statCh := make(chan []asmetrics.StatMessage)
Expand All @@ -208,7 +208,7 @@ 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)
ah := activatorhandler.New(ctx, throttler, roundtrip, networkConfig.EnableMeshPodAddressability, logger, tlsEnabled)
ah = handler.NewTimeoutHandler(ah, "activator request timeout", func(r *http.Request) (time.Duration, time.Duration, time.Duration) {
if rev := activatorhandler.RevisionFrom(r.Context()); rev != nil {
var responseStartTimeout = 0 * time.Second
Expand Down Expand Up @@ -278,16 +278,13 @@ func main() {
}(name, server)
}

// Enable TLS server when internal-encryption is specified.
// Enable TLS server when DataPlaneTrust is not netcfg.TrustDisabled.
// At this moment activator with TLS does not disable HTTP.
// See also https://github.com/knative/serving/issues/12808.
if tlsEnabled {
name, server := "https", pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah)
go func(name string, s *http.Server) {
s.TLSConfig = &tls.Config{
MinVersion: tls.VersionTLS12,
GetCertificate: certCache.GetCertificate,
}
s.TLSConfig = &certCache.ServerTLSConf
Comment thread
davidhadas marked this conversation as resolved.
// Don't forward ErrServerClosed as that indicates we're already shutting down.
if err := s.ListenAndServeTLS("", ""); err != nil && !errors.Is(err, http.ErrServerClosed) {
errCh <- fmt.Errorf("%s server failed: %w", name, err)
Expand Down
43 changes: 35 additions & 8 deletions pkg/activator/certificate/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/tls"
"crypto/x509"
"encoding/pem"
"fmt"
"sync"

"go.uber.org/zap"
Expand All @@ -40,23 +41,26 @@ import (
type CertCache struct {
secretInformer v1.SecretInformer
logger *zap.SugaredLogger
trust netcfg.Trust
Comment thread
davidhadas marked this conversation as resolved.
Outdated

certificate *tls.Certificate
TLSConf tls.Config
certificate *tls.Certificate
ClientTLSConf tls.Config
ServerTLSConf tls.Config
Comment thread
davidhadas marked this conversation as resolved.

certificatesMux sync.RWMutex
}

// NewCertCache starts secretInformer.
func NewCertCache(ctx context.Context) *CertCache {
func NewCertCache(ctx context.Context, trust netcfg.Trust) *CertCache {
secretInformer := secretinformer.Get(ctx)

cr := &CertCache{
secretInformer: secretInformer,
logger: logging.FromContext(ctx),
trust: trust,
}

secret, err := cr.secretInformer.Lister().Secrets(system.Namespace()).Get(netcfg.ServingInternalCertName)
secret, err := cr.secretInformer.Lister().Secrets(system.Namespace()).Get(netcfg.ServingRoutingCertName)
if err != nil {
cr.logger.Warnw("failed to get secret", zap.Error(err))
return nil
Expand All @@ -65,7 +69,7 @@ func NewCertCache(ctx context.Context) *CertCache {
cr.updateCache(secret)

secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingInternalCertName),
FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingRoutingCertName),
Handler: cache.ResourceEventHandlerFuncs{
UpdateFunc: cr.handleCertificateUpdate,
AddFunc: cr.handleCertificateAdd,
Expand Down Expand Up @@ -101,9 +105,32 @@ func (cr *CertCache) updateCache(secret *corev1.Secret) {
}
pool.AddCert(ca)

cr.TLSConf.RootCAs = pool
cr.TLSConf.ServerName = certificates.LegacyFakeDnsName
cr.TLSConf.MinVersion = tls.VersionTLS12
cr.ClientTLSConf.RootCAs = pool
cr.ClientTLSConf.ServerName = certificates.LegacyFakeDnsName
cr.ClientTLSConf.MinVersion = tls.VersionTLS12
cr.ClientTLSConf.Certificates = []tls.Certificate{cert}

cr.ServerTLSConf.MinVersion = tls.VersionTLS12
cr.ServerTLSConf.GetCertificate = cr.GetCertificate

switch cr.trust {
case netcfg.TrustIdentity, netcfg.TrustMutual:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between identity and mutual?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is only at the ingress.
With Identity, an ingress is supposed to also verify the identity of the original sender and propagate the verified identity (claims etc.) to the service.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Identity, an ingress is supposed to also verify the identity of the original sender and propagate the verified identity (claims etc.) to the service.

Using what mechanic/format?

@davidhadas davidhadas Jun 2, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have not yet designed this.
We identified that envoy has the necessary support to verify and forward the claims and all that we did so far is decide to add such an option in the future.
Our focus now is on TLS and mTLS support, identity will come after we have TLS and mTLS ingress to queue (identity depends anyhow on mTLS to ensure all requests go via the ingress).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We identified that envoy has the necessary support to verify and forward the claims and all that we did so far is decide to add such an option in the future.

FYI we can't assume envoy is the underlying proxy here. Whatever use case you have you'll want to ensure it's possible to do this with the GatewayAPI - https://gateway-api.sigs.k8s.io/

In the future we'll be dropping our net-* repos in favour of programming against this API.

cr.ServerTLSConf.ClientAuth = tls.RequireAndVerifyClientCert
cr.ServerTLSConf.ClientCAs = pool
cr.ServerTLSConf.VerifyConnection = func(cs tls.ConnectionState) error {
for _, match := range cs.PeerCertificates[0].DNSNames {
Comment thread
davidhadas marked this conversation as resolved.
Outdated
if match == "kn-routing-0" { // routingId not yet supported
Comment thread
davidhadas marked this conversation as resolved.
Outdated
return nil
}
//Until all ingresses work with updated dataplane certificates - allow also any legacy certificate
if match == certificates.LegacyFakeDnsName {
return nil
}
}
cr.logger.Info("mTLS: Failed Client with DNSNames: %v\n", cs.PeerCertificates[0].DNSNames)
Comment thread
davidhadas marked this conversation as resolved.
Outdated
return fmt.Errorf("mTLS Failed to approve %v", cs.PeerCertificates[0].DNSNames)
Comment thread
davidhadas marked this conversation as resolved.
Outdated
}
}
}

func (cr *CertCache) handleCertificateUpdate(_, new interface{}) {
Expand Down
130 changes: 125 additions & 5 deletions pkg/activator/certificate/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ func fakeCertCache(ctx context.Context) *CertCache {
cr := &CertCache{
secretInformer: secretInformer,
certificate: nil,
TLSConf: tls.Config{},
ClientTLSConf: tls.Config{},
logger: logging.FromContext(ctx),
trust: netcfg.TrustMutual,
}

secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingInternalCertName),
FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingRoutingCertName),
Handler: cache.ResourceEventHandlerFuncs{
UpdateFunc: cr.handleCertificateUpdate,
AddFunc: cr.handleCertificateAdd,
Expand All @@ -62,7 +63,28 @@ func fakeCertCache(ctx context.Context) *CertCache {
return cr
}

func TestReconcile(t *testing.T) {
func routingCertCache(ctx context.Context) *CertCache {
secretInformer := fakesecretinformer.Get(ctx)

cr := &CertCache{
secretInformer: secretInformer,
certificate: nil,
ClientTLSConf: tls.Config{},
logger: logging.FromContext(ctx),
}

secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingRoutingCertName),
Handler: cache.ResourceEventHandlerFuncs{
UpdateFunc: cr.handleCertificateUpdate,
AddFunc: cr.handleCertificateAdd,
},
})

return cr
}

func TestFakeReconcile(t *testing.T) {
ctx, cancel, informers := rtesting.SetupFakeContextWithCancel(t)
cr := fakeCertCache(ctx)

Expand All @@ -78,7 +100,105 @@ func TestReconcile(t *testing.T) {

secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: netcfg.ServingInternalCertName,
Name: netcfg.ServingRoutingCertName,
Namespace: system.Namespace(),
},
Data: map[string][]byte{
certificates.CaCertName: ca,
certificates.PrivateKeyName: tlsKey,
certificates.CertName: tlsCrt,
},
}

fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Create(ctx, secret, metav1.CreateOptions{})
fakesecretinformer.Get(ctx).Informer().GetIndexer().Add(secret)

// Wait for the resources to be created and the handler is called.
if err := wait.PollImmediate(10*time.Millisecond, 2*time.Second, func() (bool, error) {
// To access cert.Certificate, take a lock.
cr.certificatesMux.RLock()
defer cr.certificatesMux.RUnlock()
cert, _ := cr.GetCertificate(nil)
return cert != nil, nil
}); err != nil {
t.Fatal("timeout to get the secret:", err)
}

cs := tls.ConnectionState{PeerCertificates: []*x509.Certificate{{DNSNames: []string{"ddd", "xxx", "kn-routing-0", "ddd", "xxx"}}}}
if verifyErr := cr.ServerTLSConf.VerifyConnection(cs); verifyErr != nil {
t.Error("cr.ServerTLSConf.VerifyConnection: expected to find kn-routing-0")
}

cs = tls.ConnectionState{PeerCertificates: []*x509.Certificate{{DNSNames: []string{"ddd", "xxx", certificates.LegacyFakeDnsName, "ddd", "xxx"}}}}
if verifyErr := cr.ServerTLSConf.VerifyConnection(cs); verifyErr != nil {
t.Error("cr.ServerTLSConf.VerifyConnection: expected to find kn-routing-0")
}
cs = tls.ConnectionState{PeerCertificates: []*x509.Certificate{{DNSNames: []string{"ddd", "xxx"}}}}
if verifyErr := cr.ServerTLSConf.VerifyConnection(cs); verifyErr == nil {
t.Error("cr.ServerTLSConf.VerifyConnection: expected error")
}
cs = tls.ConnectionState{PeerCertificates: []*x509.Certificate{{DNSNames: []string{}}}}
if verifyErr := cr.ServerTLSConf.VerifyConnection(cs); verifyErr == nil {
t.Error("cr.ServerTLSConf.VerifyConnection: expected error")
}

// Update cert and key but keep using old CA, then the error is expected.
secret.Data[certificates.CertName] = newTLSCrt
secret.Data[certificates.PrivateKeyName] = newTLSKey
newCert, _ := tls.X509KeyPair(newTLSCrt, newTLSKey)

fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Update(ctx, secret, metav1.UpdateOptions{})
if err := wait.PollImmediate(10*time.Millisecond, 5*time.Second, func() (bool, error) {
// To access cert.Certificate, take a lock.
cr.certificatesMux.RLock()
defer cr.certificatesMux.RUnlock()
cert, err := cr.GetCertificate(nil)
return err == nil && reflect.DeepEqual(newCert.Certificate, cert.Certificate), nil
}); err != nil {
t.Fatalf("timeout to update the cert: %v", err)
}

// Update CA, now the error is gone.
secret.Data[certificates.CaCertName] = newCA

pool := x509.NewCertPool()
block, _ := pem.Decode(secret.Data[certificates.CaCertName])
ca, err := x509.ParseCertificate(block.Bytes)
if err != nil {
cr.logger.Warnw("Failed to parse CA: %v", zap.Error(err))
return
}

pool.AddCert(ca)

fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Update(ctx, secret, metav1.UpdateOptions{})
if err := wait.PollImmediate(10*time.Millisecond, 10*time.Second, func() (bool, error) {
// To access cr.TLSConf.RootCAs, take a lock.
cr.certificatesMux.RLock()
defer cr.certificatesMux.RUnlock()
return err == nil && pool.Equal(cr.ClientTLSConf.RootCAs), nil
}); err != nil {
t.Fatalf("timeout to update the cert: %v", err)
}
}

func TestRoutingReconcile(t *testing.T) {
ctx, cancel, informers := rtesting.SetupFakeContextWithCancel(t)
cr := routingCertCache(ctx)

waitInformers, err := rtesting.RunAndSyncInformers(ctx, informers...)
if err != nil {
cancel()
t.Fatal("failed to start informers:", err)
}
t.Cleanup(func() {
cancel()
waitInformers()
})

secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: netcfg.ServingRoutingCertName,
Namespace: system.Namespace(),
},
Data: map[string][]byte{
Expand Down Expand Up @@ -136,7 +256,7 @@ func TestReconcile(t *testing.T) {
// To access cr.TLSConf.RootCAs, take a lock.
cr.certificatesMux.RLock()
defer cr.certificatesMux.RUnlock()
return err == nil && pool.Equal(cr.TLSConf.RootCAs), nil
return err == nil && pool.Equal(cr.ClientTLSConf.RootCAs), nil
}); err != nil {
t.Fatalf("timeout to update the cert: %v", err)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/activator/config/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"

"go.uber.org/atomic"
netcfg "knative.dev/networking/pkg/config"
"knative.dev/pkg/configmap"
tracingconfig "knative.dev/pkg/tracing/config"
)
Expand All @@ -29,6 +30,7 @@ type cfgKey struct{}
// Config is the configuration for the activator.
type Config struct {
Tracing *tracingconfig.Config
Trust netcfg.Trust
}

// FromContext obtains a Config injected into the passed context.
Expand All @@ -45,14 +47,15 @@ type Store struct {
}

// NewStore creates a new configuration Store.
func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store {
func NewStore(logger configmap.Logger, trust netcfg.Trust, onAfterStore ...func(name string, value interface{})) *Store {
Comment thread
dprotaso marked this conversation as resolved.
s := &Store{}

// Append an update function to run after a ConfigMap has updated to update the
// current state of the Config.
onAfterStore = append(onAfterStore, func(_ string, _ interface{}) {
s.current.Store(&Config{
Tracing: s.UntypedLoad(tracingconfig.ConfigName).(*tracingconfig.Config).DeepCopy(),
Trust: trust,
})
})
s.UntypedStore = configmap.NewUntypedStore(
Expand Down
4 changes: 2 additions & 2 deletions pkg/activator/config/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var tracingConfig = &corev1.ConfigMap{

func TestStore(t *testing.T) {
logger := ltesting.TestLogger(t)
store := NewStore(logger)
store := NewStore(logger, "Disabled")
store.OnConfigChanged(tracingConfig)

ctx := store.ToContext(context.Background())
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestStore(t *testing.T) {

func BenchmarkStoreToContext(b *testing.B) {
logger := ltesting.TestLogger(b)
store := NewStore(logger)
store := NewStore(logger, "Disabled")
store.OnConfigChanged(tracingConfig)

b.Run("sequential", func(b *testing.B) {
Expand Down
Loading