diff --git a/README.md b/README.md index 6788a4ef..a8d62d2e 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,17 @@ The key for OCSP response in TLS Secret is `tls.ocsp-resp` by default. It can be changed by `--ocsp-resp-key` flag. The value of OCSP response in TLS Secret must be DER encoded. +## Sharing TLS ticket keys + +By default, each nghttpx encrypts TLS ticket by its own key. This +means that if there are several nghttpx ingress controller instances, +TLS session resumption might not work if the new connection goes to +the different instance. With `--share-tls-ticket-key` flag, the +controller generates TLS ticket key in a Secret specified by +`--nghttpx-secret`, which is shared by all controllers. This ensures +that all nghttpx instances use the same encryption key, which enables +stable TLS session resumption. + ## HTTP/3 (Experimental) In order to enable the experimental HTTP/3 feature, run the controller diff --git a/cmd/nghttpx-ingress-controller/main.go b/cmd/nghttpx-ingress-controller/main.go index 8a6e034a..db967070 100644 --- a/cmd/nghttpx-ingress-controller/main.go +++ b/cmd/nghttpx-ingress-controller/main.go @@ -91,6 +91,7 @@ var ( internalDefaultBackend = false http3 = false nghttpxSecret = "nghttpx-km" + shareTLSTicketKey = false reconcileTimeout = 10 * time.Minute leaderElectionConfig = componentbaseconfig.LeaderElectionConfiguration{ LeaseDuration: metav1.Duration{Duration: 15 * time.Second}, @@ -191,6 +192,8 @@ func main() { rootCmd.Flags().StringVar(&nghttpxSecret, "nghttpx-secret", nghttpxSecret, `The name of Secret resource which contains the keying materials for nghttpx. The resource must belong to the same namespace as the controller Pod. If it is not found, the controller will create new one.`) + rootCmd.Flags().BoolVar(&shareTLSTicketKey, "share-tls-ticket-key", shareTLSTicketKey, `Share TLS ticket key among all nghttpx-ingress-controllers. TLS ticket keys are stored to the Secret specified by nghttpx-secret flag. If this flag is set to true, TLS ticket keys are generated and rotated by the controller every 1 hour. At most 12 latest keys are retained. TLS tickets are encrypted with AES-128-CBC.`) + rootCmd.Flags().DurationVar(&reconcileTimeout, "reconcile-timeout", reconcileTimeout, `A timeout for a single reconciliation. It is a safe guard to prevent a reconciliation from getting stuck indefinitely.`) @@ -385,6 +388,7 @@ func run(ctx context.Context, _ *cobra.Command, _ []string) { HealthzPort: healthzPort, InternalDefaultBackend: internalDefaultBackend, HTTP3: http3, + ShareTLSTicketKey: shareTLSTicketKey, ReconcileTimeout: reconcileTimeout, LeaderElectionConfig: leaderElectionConfig, RequireIngressClass: requireIngressClass, diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index ea45b2c3..ba8f259f 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -72,6 +72,8 @@ const ( syncKey = "ingress" // quicSecretTimeout is the timeout for the last QUIC keying material. quicSecretTimeout = time.Hour + // tlsTicketKeyTimeout is the timeout, when it is fired, TLS ticket keys are rotated. New key is also generated. + tlsTicketKeyTimeout = time.Hour noResyncPeriod = 0 @@ -81,6 +83,12 @@ const ( // secret is last updated. quicKeyingMaterialsUpdateTimestampKey = "ingress.zlab.co.jp/quic-keying-materials-update-timestamp" + // nghttpxTLSTicketKeySecretKey is a field name of TLS ticket keys in Secret. + nghttpxTLSTicketKeySecretKey = "nghttpx-tls-ticket-key" + // tlsTicketKeyUpdateTimestampKey is an annotation key which is associated to the value that contains the timestamp when TLS ticket + // keys are last updated. + tlsTicketKeyUpdateTimestampKey = "ingress.zlab.co.jp/tls-ticket-key-update-timestamp" + // certificateGarbageCollectionPeriod is the period between garbage collection against certificate cache is performed. certificateGarbageCollectionPeriod = time.Hour ) @@ -137,6 +145,7 @@ type LoadBalancerController struct { healthzPort int32 internalDefaultBackend bool http3 bool + shareTLSTicketKey bool nghttpxSecret types.NamespacedName reconcileTimeout time.Duration leaderElectionConfig componentbaseconfig.LeaderElectionConfiguration @@ -209,6 +218,8 @@ type Config struct { InternalDefaultBackend bool // HTTP3, if true, enables HTTP/3. HTTP3 bool + // ShareTLSTicketKey, if true, shares TLS ticket key among ingress controllers via Secret. + ShareTLSTicketKey bool // ReconcileTimeout is a timeout for a single reconciliation. It is a safe guard to prevent a reconciliation from getting stuck // indefinitely. ReconcileTimeout time.Duration @@ -259,6 +270,7 @@ func NewLoadBalancerController(ctx context.Context, clientset clientset.Interfac healthzPort: config.HealthzPort, internalDefaultBackend: config.InternalDefaultBackend, http3: config.HTTP3, + shareTLSTicketKey: config.ShareTLSTicketKey, reconcileTimeout: config.ReconcileTimeout, leaderElectionConfig: config.LeaderElectionConfig, requireIngressClass: config.RequireIngressClass, @@ -997,14 +1009,25 @@ func (lbc *LoadBalancerController) sync(ctx context.Context, key string) error { log.Error(err, "nghttpx secret not found") // Continue to processing so that missing Secret does not prevent the controller from reconciling new configuration. - } + } else { + if lbc.shareTLSTicketKey { + ticketKey, ok := secret.Data[nghttpxTLSTicketKeySecretKey] + if !ok { + log.Error(nil, "Secret does not contain TLS ticket key") + } else if err := nghttpx.VerifyTLSTicketKey(ticketKey); err != nil { + log.Error(err, "Secret contains malformed TLS ticket key") + } else { + ingConfig.TLSTicketKeyFiles = nghttpx.CreateTLSTicketKeyFiles(ingConfig.ConfDir, ticketKey) + } + } - if secret != nil && lbc.http3 { - quicKM, ok := secret.Data[nghttpxQUICKeyingMaterialsSecretKey] - if !ok { - log.Error(nil, "Secret does not contain QUIC keying materials") - } else { - ingConfig.QUICSecretFile = nghttpx.CreateQUICSecretFile(ingConfig.ConfDir, quicKM) + if lbc.http3 { + quicKM, ok := secret.Data[nghttpxQUICKeyingMaterialsSecretKey] + if !ok { + log.Error(nil, "Secret does not contain QUIC keying materials") + } else { + ingConfig.QUICSecretFile = nghttpx.CreateQUICSecretFile(ingConfig.ConfDir, quicKM) + } } } @@ -1107,6 +1130,7 @@ func (lbc *LoadBalancerController) createIngressConfig(ctx context.Context, ings FetchOCSPRespFromSecret: lbc.fetchOCSPRespFromSecret, ProxyProto: lbc.proxyProto, HTTP3: lbc.http3, + ShareTLSTicketKey: lbc.shareTLSTicketKey, } var ( @@ -2796,6 +2820,10 @@ func (lc *LeaderController) syncSecret(ctx context.Context, key string, now time return nil } + tstamp := now.Format(time.RFC3339) + + requeueAfter := 12 * time.Hour + secret, err := lc.secretLister.Secrets(ns).Get(name) if err != nil { if !apierrors.IsNotFound(err) { @@ -2807,76 +2835,218 @@ func (lc *LeaderController) syncSecret(ctx context.Context, key string, now time ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: ns, - Annotations: map[string]string{ - quicKeyingMaterialsUpdateTimestampKey: now.Format(time.RFC3339), - }, - }, - Data: map[string][]byte{ - nghttpxQUICKeyingMaterialsSecretKey: []byte(hex.EncodeToString(nghttpx.NewQUICKeyingMaterial())), }, } + if lc.lbc.shareTLSTicketKey || lc.lbc.http3 { + secret.Annotations = make(map[string]string) + secret.Data = make(map[string][]byte) + } + + if lc.lbc.shareTLSTicketKey { + key, err := nghttpx.NewInitialTLSTicketKey() + if err != nil { + return err + } + + secret.Annotations[tlsTicketKeyUpdateTimestampKey] = tstamp + secret.Data[nghttpxTLSTicketKeySecretKey] = key + + if requeueAfter > tlsTicketKeyTimeout { + requeueAfter = tlsTicketKeyTimeout + } + } + + if lc.lbc.http3 { + secret.Annotations[quicKeyingMaterialsUpdateTimestampKey] = tstamp + secret.Data[nghttpxQUICKeyingMaterialsSecretKey] = []byte(hex.EncodeToString(nghttpx.NewQUICKeyingMaterial())) + + if requeueAfter > quicSecretTimeout { + requeueAfter = quicSecretTimeout + } + } + if _, err := lc.lbc.clientset.CoreV1().Secrets(secret.Namespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { log.Error(err, "Unable to create Secret") return err } - log.Info("Secret was created") + log.Info("Secret was created", "requeueAfter", requeueAfter) // If Secret has been added to the queue and is waiting, this effectively overrides it. - lc.secretQueue.AddAfter(key, quicSecretTimeout) + lc.secretQueue.AddAfter(key, requeueAfter) return nil } - var km []byte + var ( + ticketKey []byte + ticketKeyUpdate bool + ) - if ts, ok := secret.Annotations[quicKeyingMaterialsUpdateTimestampKey]; ok { - if t, err := time.Parse(time.RFC3339, ts); err == nil { - km = secret.Data[nghttpxQUICKeyingMaterialsSecretKey] + if lc.lbc.shareTLSTicketKey { + var ticketKeyAddAfter time.Duration - if err := nghttpx.VerifyQUICKeyingMaterials(km); err != nil { - log.Error(err, "QUIC keying materials are malformed") - km = nil - } else { - d := t.Add(quicSecretTimeout).Sub(now) - if d > 0 { - log.Info("QUIC keying materials are not expired and in a good shape", "retryAfter", d) - // If Secret has been added to the queue and is waiting, this effectively overrides it. - lc.secretQueue.AddAfter(key, d) + ticketKey, ticketKeyUpdate, ticketKeyAddAfter = lc.getTLSTicketKeyFromSecret(ctx, secret, now) - return nil - } - } + if ticketKeyAddAfter != 0 && requeueAfter > ticketKeyAddAfter { + requeueAfter = ticketKeyAddAfter + } + } + + var ( + quicKM []byte + quicKMUpdate bool + ) + + if lc.lbc.http3 { + var quicKMAddAfter time.Duration + + quicKM, quicKMUpdate, quicKMAddAfter = lc.getQUICKeyingMaterialsFromSecret(ctx, secret, now) + + if quicKMAddAfter != 0 && requeueAfter > quicKMAddAfter { + requeueAfter = quicKMAddAfter } } + if !ticketKeyUpdate && !quicKMUpdate { + log.Info("No update is required", "requeueAfter", requeueAfter) + + lc.secretQueue.AddAfter(key, requeueAfter) + + return nil + } + updatedSecret := secret.DeepCopy() if updatedSecret.Annotations == nil { updatedSecret.Annotations = make(map[string]string) } - updatedSecret.Annotations[quicKeyingMaterialsUpdateTimestampKey] = now.Format(time.RFC3339) - if updatedSecret.Data == nil { updatedSecret.Data = make(map[string][]byte) } - updatedSecret.Data[nghttpxQUICKeyingMaterialsSecretKey] = nghttpx.UpdateQUICKeyingMaterials(km) + if ticketKeyUpdate { + updatedSecret.Annotations[tlsTicketKeyUpdateTimestampKey] = tstamp + + var ( + key []byte + err error + ) + + if len(ticketKey) == 0 { + key, err = nghttpx.NewInitialTLSTicketKey() + } else { + key, err = nghttpx.UpdateTLSTicketKey(ticketKey) + } + if err != nil { + return err + } + + updatedSecret.Data[nghttpxTLSTicketKeySecretKey] = key + + log.Info("TLS ticket keys were updated") + + if requeueAfter > tlsTicketKeyTimeout { + requeueAfter = tlsTicketKeyTimeout + } + } + + if quicKMUpdate { + updatedSecret.Annotations[quicKeyingMaterialsUpdateTimestampKey] = tstamp + + updatedSecret.Data[nghttpxQUICKeyingMaterialsSecretKey] = nghttpx.UpdateQUICKeyingMaterials(quicKM) + + log.Info("QUIC keying materials were updated") + + if requeueAfter > quicSecretTimeout { + requeueAfter = quicSecretTimeout + } + } if _, err := lc.lbc.clientset.CoreV1().Secrets(updatedSecret.Namespace).Update(ctx, updatedSecret, metav1.UpdateOptions{}); err != nil { log.Error(err, "Unable to update Secret") return err } - log.Info("Secret was updated") + log.Info("Secret was updated", "requeueAfter", requeueAfter) // If Secret has been added to the queue and is waiting, this effectively overrides it. - lc.secretQueue.AddAfter(key, quicSecretTimeout) + lc.secretQueue.AddAfter(key, requeueAfter) return nil } +func (lc *LeaderController) getTLSTicketKeyFromSecret(ctx context.Context, s *corev1.Secret, t time.Time) (ticketKey []byte, needsUpdate bool, requeueAfter time.Duration) { + log := klog.FromContext(ctx) + + ts, ok := s.Annotations[tlsTicketKeyUpdateTimestampKey] + if !ok { + log.Error(nil, "Secret does not contain the annotation", "annotation", tlsTicketKeyUpdateTimestampKey) + + return nil, true, 0 + } + + lastUpdate, err := time.Parse(time.RFC3339, ts) + if err != nil { + log.Error(err, "Unable to parse timestamp", "annotation", tlsTicketKeyUpdateTimestampKey) + + return nil, true, 0 + } + + ticketKey = s.Data[nghttpxTLSTicketKeySecretKey] + + if err := nghttpx.VerifyTLSTicketKey(ticketKey); err != nil { + log.Error(err, "TLS ticket keys are malformed") + + return nil, true, 0 + } + + requeueAfter = lastUpdate.Add(tlsTicketKeyTimeout).Sub(t) + if requeueAfter > 0 { + log.Info("TLS ticket keys are not expired and in a good shape", "requeueAfter", requeueAfter) + + return ticketKey, false, requeueAfter + } + + return ticketKey, true, 0 +} + +func (lc *LeaderController) getQUICKeyingMaterialsFromSecret(ctx context.Context, s *corev1.Secret, t time.Time) (quicKM []byte, needsUpdate bool, requeueAfter time.Duration) { + log := klog.FromContext(ctx) + + ts, ok := s.Annotations[quicKeyingMaterialsUpdateTimestampKey] + if !ok { + log.Error(nil, "Secret does not contain the annotation", "annotation", quicKeyingMaterialsUpdateTimestampKey) + + return nil, true, 0 + } + + lastUpdate, err := time.Parse(time.RFC3339, ts) + if err != nil { + log.Error(err, "Unable to parse timestamp", "annotation", quicKeyingMaterialsUpdateTimestampKey) + + return nil, true, 0 + } + + quicKM = s.Data[nghttpxQUICKeyingMaterialsSecretKey] + + if err := nghttpx.VerifyQUICKeyingMaterials(quicKM); err != nil { + log.Error(err, "QUIC keying materials are malformed") + + return nil, true, 0 + } + + requeueAfter = lastUpdate.Add(quicSecretTimeout).Sub(t) + if requeueAfter > 0 { + log.Info("QUIC keying materials are not expired and in a good shape", "requeueAfter", requeueAfter) + + return quicKM, false, requeueAfter + } + + return quicKM, true, 0 +} + func (lc *LeaderController) ingressWorker(ctx context.Context) { log := klog.FromContext(ctx) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index abf6acf0..b635a896 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -74,6 +74,7 @@ type fixture struct { enableEndpointSlice bool http3 bool + shareTLSTicketKey bool publishService *types.NamespacedName requireIngressClass bool } @@ -155,6 +156,7 @@ func (f *fixture) preparePod(pod *corev1.Pod) { ReloadRate: 1.0, ReloadBurst: 1, HTTP3: f.http3, + ShareTLSTicketKey: f.shareTLSTicketKey, PublishService: f.publishService, RequireIngressClass: f.requireIngressClass, Pod: pod, @@ -1002,6 +1004,10 @@ func TestSyncDefaultBackend(t *testing.T) { }); !reflect.DeepEqual(got, want) { t.Errorf("flb.ingConfig.MrubyFile = %q, want %q", got, want) } + + if got, want := len(flb.ingConfig.TLSTicketKeyFiles), 0; got != want { + t.Errorf("len(flb.ingConfig.TLSTicketKeyFiles) = %v, want %v", got, want) + } }) } } @@ -2037,8 +2043,8 @@ func TestSyncNormalizePath(t *testing.T) { } } -// TestSyncSecret verifies syncSecret. -func TestSyncSecret(t *testing.T) { +// TestSyncSecretQUIC verifies syncSecret for QUIC keying materials. +func TestSyncSecretQUIC(t *testing.T) { now := time.Now().Round(time.Second) expiredTimestamp := now.Add(-quicSecretTimeout) notExpiredTimestamp := now.Add(-quicSecretTimeout + time.Second) @@ -2153,6 +2159,128 @@ func TestSyncSecret(t *testing.T) { } } +// TestSyncSecretTLSTicketKey verifies syncSecret for TLS ticket key. +func TestSyncSecretTLSTicketKey(t *testing.T) { + now := time.Now().Round(time.Second) + expiredTimestamp := now.Add(-tlsTicketKeyTimeout) + notExpiredTimestamp := now.Add(-tlsTicketKeyTimeout + time.Second) + + tests := []struct { + desc string + secret *corev1.Secret + wantKeepTimestamp bool + }{ + { + desc: "No existing TLS ticket key secret", + }, + { + desc: "TLS ticket key secret is up to date", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultNghttpxSecret.Name, + Namespace: defaultNghttpxSecret.Namespace, + Annotations: map[string]string{ + tlsTicketKeyUpdateTimestampKey: notExpiredTimestamp.Format(time.RFC3339), + }, + }, + Data: map[string][]byte{ + nghttpxTLSTicketKeySecretKey: []byte("" + + "................................................" + + "................................................"), + }, + }, + wantKeepTimestamp: true, + }, + { + desc: "TLS ticket key secret has been expired", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultNghttpxSecret.Name, + Namespace: defaultNghttpxSecret.Namespace, + Annotations: map[string]string{ + tlsTicketKeyUpdateTimestampKey: expiredTimestamp.Format(time.RFC3339), + }, + }, + Data: map[string][]byte{ + nghttpxTLSTicketKeySecretKey: []byte("" + + "................................................" + + "................................................", + ), + }, + }, + }, + { + desc: "TLS ticket key secret timestamp is not expired, but data is malformed", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultNghttpxSecret.Name, + Namespace: defaultNghttpxSecret.Namespace, + Annotations: map[string]string{ + tlsTicketKeyUpdateTimestampKey: notExpiredTimestamp.Format(time.RFC3339), + }, + }, + Data: map[string][]byte{ + nghttpxTLSTicketKeySecretKey: []byte("foo"), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + f := newFixture(t) + f.shareTLSTicketKey = true + + if tt.secret != nil { + f.secretStore = append(f.secretStore, tt.secret) + + f.objects = append(f.objects, tt.secret) + } + + f.prepare() + f.setupStore() + + f.lbc.nghttpxSecret = defaultNghttpxSecret + + err := f.lc.syncSecret(context.Background(), defaultNghttpxSecret.String(), now) + if err != nil { + t.Fatalf("f.lc.syncSecret(...): %v", err) + } + + updatedSecret, err := f.clientset.CoreV1().Secrets(defaultNghttpxSecret.Namespace).Get(context.Background(), + defaultNghttpxSecret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Unable to get Secret %v/%v: %v", defaultNghttpxSecret.Namespace, defaultNghttpxSecret.Name, err) + } + + if tt.wantKeepTimestamp { + if got, want := updatedSecret.Annotations[tlsTicketKeyUpdateTimestampKey], tt.secret.Annotations[tlsTicketKeyUpdateTimestampKey]; got != want { + t.Errorf("updatedSecret.Annotations[%q] = %v, want %v", tlsTicketKeyUpdateTimestampKey, got, want) + } + + if got, want := updatedSecret.Data[nghttpxTLSTicketKeySecretKey], tt.secret.Data[nghttpxTLSTicketKeySecretKey]; !bytes.Equal(got, want) { + t.Errorf("updatedSecret.Data[%q] = %q, want %q", nghttpxTLSTicketKeySecretKey, got, want) + } + } else { + if got, want := updatedSecret.Annotations[tlsTicketKeyUpdateTimestampKey], now.Format(time.RFC3339); got != want { + t.Errorf("updatedSecret.Annotations[%q] = %v, want %v", tlsTicketKeyUpdateTimestampKey, got, want) + } + + if tt.secret != nil { + if bytes.Equal(updatedSecret.Data[nghttpxTLSTicketKeySecretKey], tt.secret.Data[nghttpxTLSTicketKeySecretKey]) { + t.Fatalf("updatedSecret.Data[%q] must be updated", nghttpxTLSTicketKeySecretKey) + } + } + + key := updatedSecret.Data[nghttpxTLSTicketKeySecretKey] + if err := nghttpx.VerifyTLSTicketKey(key); err != nil { + t.Fatalf("VerifyTLSTicketKey(...): %v", err) + } + } + }) + } +} + // TestRemoveUpstreamsWithInconsistentBackendParams verifies removeUpstreamsWithInconsistentBackendParams. func TestRemoveUpstreamsWithInconsistentBackendParams(t *testing.T) { tests := []struct { @@ -2796,3 +2924,41 @@ func TestCreateTLSCredFromSecret(t *testing.T) { t.Errorf("f.lbc.certCache[%q] = %v, want %v", cacheKey, got, want) } } + +func TestSyncWithTLSTicketKey(t *testing.T) { + f := newFixture(t) + f.shareTLSTicketKey = true + + dCrt := []byte(tlsCrt) + dKey := []byte(tlsKey) + tlsSecret := newTLSSecret("kube-system", "default-tls", dCrt, dKey) + nghttpxSecret := newNghttpxSecret() + ticketKey, err := nghttpx.NewInitialTLSTicketKey() + if err != nil { + t.Fatalf("nghttpx.NewInitialTLSTicketKey: %v", err) + } + nghttpxSecret.Data = map[string][]byte{ + nghttpxTLSTicketKeySecretKey: ticketKey, + } + svc, eps, _ := newDefaultBackend() + + f.secretStore = append(f.secretStore, tlsSecret, nghttpxSecret) + f.svcStore = append(f.svcStore, svc) + f.epStore = append(f.epStore, eps) + + f.objects = append(f.objects, tlsSecret, nghttpxSecret, svc, eps) + + f.prepare() + f.lbc.defaultTLSSecret = &types.NamespacedName{ + Namespace: tlsSecret.Namespace, + Name: tlsSecret.Name, + } + f.run() + + flb := f.lbc.nghttpx.(*fakeLoadBalancer) + ingConfig := flb.ingConfig + + if got, want := len(ingConfig.TLSTicketKeyFiles), 2; got != want { + t.Errorf("len(ingConfig.TLSTicketKeyFiles) = %v, want %v", got, want) + } +} diff --git a/pkg/nghttpx/command.go b/pkg/nghttpx/command.go index 76d29125..12276218 100644 --- a/pkg/nghttpx/command.go +++ b/pkg/nghttpx/command.go @@ -120,6 +120,9 @@ func (lb *LoadBalancer) CheckAndReload(ctx context.Context, ingressCfg *IngressC if err := writePerPatternMrubyFile(ingressCfg); err != nil { return false, err } + if err := writeTLSTicketKeyFiles(ingressCfg); err != nil { + return false, err + } if err := writeQUICSecretFile(ingressCfg); err != nil { return false, err } @@ -172,7 +175,11 @@ func (lb *LoadBalancer) deleteStaleAssets(ctx context.Context, ingConfig *Ingres // deleteStaleTLSAssets deletes TLS asset files which are no longer used. func (lb *LoadBalancer) deleteStaleTLSAssets(ctx context.Context, ingConfig *IngressConfig, t time.Time) error { - return deleteAssetFiles(ctx, filepath.Join(ingConfig.ConfDir, tlsDir), t, lb.staleAssetsThreshold) + if err := deleteAssetFiles(ctx, filepath.Join(ingConfig.ConfDir, tlsDir), t, lb.staleAssetsThreshold); err != nil { + return err + } + + return deleteAssetFiles(ctx, filepath.Join(ingConfig.ConfDir, tlsTicketKeyDir), t, lb.staleAssetsThreshold) } // deleteStaleMrubyAssets deletes mruby asset files which are no longer used. diff --git a/pkg/nghttpx/nghttpx.tmpl b/pkg/nghttpx/nghttpx.tmpl index e3c47824..90500ae2 100644 --- a/pkg/nghttpx/nghttpx.tmpl +++ b/pkg/nghttpx/nghttpx.tmpl @@ -27,6 +27,9 @@ certificate-file={{ .DefaultTLSCred.Cert.Path }} {{ range $cred := .SubTLSCred -}} subcert={{ $cred.Key.Path }}:{{ $cred.Cert.Path }} {{ end -}} +{{ range $ticketKey := .TLSTicketKeyFiles -}} +tls-ticket-key-file={{ $ticketKey.Path }} +{{ end -}} {{ else if .HTTPSPort -}} # just listen {{ .HTTPSPort }} to gain port {{ .HTTPSPort }}, so that we can always bind that address. frontend=*,{{ .HTTPSPort }};no-tls{{ if .ProxyProto }};proxyproto{{ end }} @@ -52,3 +55,7 @@ fetch-ocsp-response-file=/cat-ocsp-resp {{ else -}} fetch-ocsp-response-file=/fetch-ocsp-response {{ end -}} +{{ if and .ShareTLSTicketKey .HTTPSPort .TLS -}} +# TLS ticket key +tls-ticket-key-cipher=aes-128-cbc +{{ end -}} diff --git a/pkg/nghttpx/template_test.go b/pkg/nghttpx/template_test.go index 847a2aa6..0b61d844 100644 --- a/pkg/nghttpx/template_test.go +++ b/pkg/nghttpx/template_test.go @@ -721,6 +721,90 @@ fetch-ocsp-response-file=/fetch-ocsp-response wantBackendConfig: `# foo backend=192.168.0.1,8080;example.com/;proto=h2;tls;sni=foo.example.com;dns;group=group1;group-weight=100;affinity=cookie;affinity-cookie-name=sticky;affinity-cookie-path=/;affinity-cookie-secure=auto;affinity-cookie-stickiness=strict;redirect-if-not-tls;mruby=/mruby.rb;read-timeout=180;write-timeout=300;dnf backend=192.168.0.2,80;example.com/;proto=h2;affinity=cookie;affinity-cookie-name=sticky;affinity-cookie-path=/;affinity-cookie-secure=auto;affinity-cookie-stickiness=strict;redirect-if-not-tls;mruby=/mruby.rb;read-timeout=180;write-timeout=300;dnf +`, + }, + { + desc: "Share TLS ticket key", + ingConfig: &IngressConfig{ + HTTPPort: 80, + HTTPSPort: 443, + TLS: true, + DefaultTLSCred: &TLSCred{ + Key: PrivateChecksumFile{ + Path: "/tls/server.key", + Content: []byte("key"), + Checksum: hexMustDecodeString("2c70e12b7a0646f92279f427c7b38e7334d8e5389cff167a1dc30e73f826b683"), + }, + Cert: ChecksumFile{ + Path: "/tls/server.crt", + Content: []byte("cert"), + Checksum: hexMustDecodeString("06298432e8066b29e2223bcc23aa9504b56ae508fabf3435508869b9c3190e22"), + }, + }, + Upstreams: []*Upstream{ + { + Name: "foo", + Host: "example.com", + Path: "/", + Affinity: AffinityNone, + Backends: []Backend{ + { + Address: "192.168.0.1", + Port: "8080", + Protocol: ProtocolH2, + }, + { + Address: "192.168.0.2", + Port: "80", + Protocol: ProtocolH2, + }, + }, + }, + }, + Workers: 8, + WorkerProcessGraceShutdownPeriod: 30 * time.Second, + MaxWorkerProcesses: 111, + ShareTLSTicketKey: true, + TLSTicketKeyFiles: []*PrivateChecksumFile{ + { + Path: "/tls-ticket-key/key-0", + Content: []byte("key-0"), + Checksum: hexMustDecodeString("d5ead6fdd3d16630aad4f07f5e49486337a42e58fb4eef0deaabb814c003b134"), + }, + { + Path: "/tls-ticket-key/key-1", + Content: []byte("key-1"), + Checksum: hexMustDecodeString("be2974546978e3739e6d6da85c4be9f334ce32df2b9fd4b6ff1b55c0d57e9d44"), + }, + }, + }, + wantMainConfig: `accesslog-file=/dev/stdout +include=/nghttpx-backend.conf +# HTTP port +frontend=*,80;no-tls +# API endpoint +frontend=127.0.0.1,0;api;no-tls +# HTTPS port +frontend=*,443 +# Default TLS credential +private-key-file=/tls/server.key +certificate-file=/tls/server.crt +tls-ticket-key-file=/tls-ticket-key/key-0 +tls-ticket-key-file=/tls-ticket-key/key-1 +# for health check +frontend=127.0.0.1,0;healthmon;no-tls +# default configuration by controller +workers=8 +worker-process-grace-shutdown-period=30 +max-worker-processes=111 +# OCSP +fetch-ocsp-response-file=/fetch-ocsp-response +# TLS ticket key +tls-ticket-key-cipher=aes-128-cbc +`, + wantBackendConfig: `# foo +backend=192.168.0.1,8080;example.com/;proto=h2;affinity=none +backend=192.168.0.2,80;example.com/;proto=h2;affinity=none `, }, } diff --git a/pkg/nghttpx/tls.go b/pkg/nghttpx/tls.go index b135d28c..663a4b60 100644 --- a/pkg/nghttpx/tls.go +++ b/pkg/nghttpx/tls.go @@ -27,21 +27,32 @@ package nghttpx import ( "bytes" "context" + "crypto/rand" + "crypto/sha256" "crypto/x509" "encoding/hex" "encoding/pem" "errors" "fmt" + "io" "path/filepath" "sort" "time" + "golang.org/x/crypto/hkdf" "k8s.io/klog/v2" ) const ( // tlsDir is the directory where TLS certificates and private keys are stored. tlsDir = "tls" + // tlsTicketKeyDir is the directory where TLS ticket key files are stored. + tlsTicketKeyDir = "tls-ticket-key" + + // TLSTicketKeySize is the length of TLS ticket key. The default value is for AES-128-CBC encryption. + TLSTicketKeySize = 48 + // MaxTLSTicketKeyNum is the maximum number of TLS ticket keys retained in a Secret. + MaxTLSTicketKeyNum = 12 ) // CreateTLSKeyPath returns TLS private key file path. @@ -235,3 +246,127 @@ func NormalizePEM(data []byte) ([]byte, error) { } return dst.Bytes(), nil } + +func NewTLSTicketKey() ([]byte, error) { + const ikmLen = 8 + + ikmSalt := make([]byte, ikmLen+sha256.Size) + if _, err := rand.Read(ikmSalt); err != nil { + return nil, err + } + + ikm := ikmSalt[:ikmLen] + salt := ikmSalt[ikmLen:] + + r := hkdf.New(sha256.New, ikm, salt, []byte("tls ticket key")) + + key := make([]byte, TLSTicketKeySize) + if _, err := io.ReadFull(r, key); err != nil { + return nil, err + } + + return key, nil +} + +func NewInitialTLSTicketKey() ([]byte, error) { + keys := make([][]byte, 2) + + for i := range keys { + var err error + + keys[i], err = NewTLSTicketKey() + if err != nil { + return nil, err + } + } + + return bytes.Join(keys, nil), nil +} + +func VerifyTLSTicketKey(ticketKey []byte) error { + // Requires at least 2 keys for stable key rotation. + if len(ticketKey) < TLSTicketKeySize*2 || len(ticketKey)%TLSTicketKeySize != 0 { + return errors.New("invalid TLS ticket key size") + } + + return nil +} + +func UpdateTLSTicketKey(ticketKey []byte) ([]byte, error) { + return UpdateTLSTicketKeyFunc(ticketKey, NewTLSTicketKey) +} + +// UpdateTLSTicketKeyFunc generates new key via newTLSTicketKeyFunc, and rotates keys, then returns new TLS ticket key. This function +// assumes that VerifyTLSTicketKey was called against ticketKey and succeeded. +// +// ticketKey must include at least 2 keys. New key is placed to the last. Because the first key is used for encryption, new key is not +// used for encryption immediately. It starts encrypting TLS ticket after the next rotation in order to ensure that all controllers see +// this key. At most MaxTLSTicketKeyNum keys, including new key, are retained. The oldest keys are removed if the number of keys exceeds +// MaxTLSTicketKeyNum. +// +// The rotation works as follows: +// +// 1. Move the last key (which is the new key generated in the previous update) to the first. +// 2. Remove oldest keys if the number of keys exceeds MaxTLSTicketKeyNum - 1. +// 3. Generate new key and place it to the last. +func UpdateTLSTicketKeyFunc(ticketKey []byte, newTLSTicketKeyFunc func() ([]byte, error)) ([]byte, error) { + newKey, err := newTLSTicketKeyFunc() + if err != nil { + return nil, err + } + + var newTicketKeyLen int + if len(ticketKey) < TLSTicketKeySize*MaxTLSTicketKeyNum { + newTicketKeyLen = len(ticketKey) + TLSTicketKeySize + } else { + newTicketKeyLen = TLSTicketKeySize * MaxTLSTicketKeyNum + } + + newTicketKey := make([]byte, newTicketKeyLen) + + // The last key is the next encryption key. + copy(newTicketKey, ticketKey[len(ticketKey)-TLSTicketKeySize:]) + copy(newTicketKey[TLSTicketKeySize:], ticketKey[:newTicketKeyLen-TLSTicketKeySize*2]) + copy(newTicketKey[newTicketKeyLen-TLSTicketKeySize:], newKey) + + return newTicketKey, nil +} + +// CreateTLSTicketKeyFiles creates TLS ticket key files. This function assume that VerifyTLSTicketKey was called against ticketKey and +// succeeded. +func CreateTLSTicketKeyFiles(dir string, ticketKey []byte) []*PrivateChecksumFile { + dir = filepath.Join(dir, tlsTicketKeyDir) + + files := make([]*PrivateChecksumFile, len(ticketKey)/TLSTicketKeySize) + + for i := range files { + offset := TLSTicketKeySize * i + key := ticketKey[offset : offset+TLSTicketKeySize] + + files[i] = &PrivateChecksumFile{ + Path: filepath.Join(dir, fmt.Sprintf("key-%d", i)), + Content: key, + Checksum: Checksum(key), + } + } + + return files +} + +func writeTLSTicketKeyFiles(ingConfig *IngressConfig) error { + if len(ingConfig.TLSTicketKeyFiles) == 0 { + return nil + } + + if err := MkdirAll(filepath.Join(ingConfig.ConfDir, tlsTicketKeyDir)); err != nil { + return fmt.Errorf("unable to create TLS ticket key directory: %w", err) + } + + for _, f := range ingConfig.TLSTicketKeyFiles { + if err := WriteFile(f.Path, f.Content); err != nil { + return fmt.Errorf("unable to write TLS ticket key file: %w", err) + } + } + + return nil +} diff --git a/pkg/nghttpx/tls_test.go b/pkg/nghttpx/tls_test.go index 0ec82a74..27f195b5 100644 --- a/pkg/nghttpx/tls_test.go +++ b/pkg/nghttpx/tls_test.go @@ -25,12 +25,15 @@ limitations under the License. package nghttpx import ( + "bytes" "context" "crypto/tls" "encoding/base64" "encoding/hex" "path/filepath" "reflect" + "strconv" + "strings" "testing" "time" ) @@ -387,3 +390,180 @@ r1K7N2unJBaH84CjJpejcuLfzCvLCthdsu3CqXbwMNesL82+niOAyJETd2m5IlgW }) } } + +func TestNewTLSTicketKey(t *testing.T) { + ticketKey, err := NewTLSTicketKey() + if err != nil { + t.Fatalf("NewTLSTicketKey: %v", err) + } + + if got, want := len(ticketKey), TLSTicketKeySize; got != want { + t.Errorf("len(ticketKey) = %v, want %v", got, want) + } +} + +func TestVerifyTLSTicketKey(t *testing.T) { + tests := []struct { + desc string + ticketKey []byte + wantErr bool + }{ + { + desc: "Empty key", + wantErr: true, + }, + { + desc: "Good key", + ticketKey: []byte("" + + "012345678901234567890123456789012345678901234567" + + "012345678901234567890123456789012345678901234567", + ), + }, + { + desc: "Malformed key", + ticketKey: []byte("" + + "012345678901234567890123456789012345678901234567" + + "0123456789012345678901234567890123456789012345678", + ), + wantErr: true, + }, + { + desc: "Single key", + ticketKey: []byte("012345678901234567890123456789012345678901234567"), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + err := VerifyTLSTicketKey(tt.ticketKey) + if err != nil { + if tt.wantErr { + return + } + + t.Fatalf("VerifyTLSTicketKey: %v", err) + } + + if tt.wantErr { + t.Fatal("VerifyTLSTicketKey should fail") + } + }) + } +} + +type tlsKeyGenerator struct { + seq int +} + +func (g *tlsKeyGenerator) generateKey() ([]byte, error) { + prefix := "new" + strconv.Itoa(g.seq) + g.seq++ + + return []byte(prefix + strings.Repeat(".", TLSTicketKeySize-len(prefix))), nil +} + +func TestUpdateTLSTicketKeyFunc(t *testing.T) { + tests := []struct { + desc string + ticketKey []byte + want []byte + }{ + { + desc: "Key is rotated and new key is appended", + ticketKey: []byte("" + + "old0............................................" + + "old1............................................", + ), + want: []byte("" + + "old1............................................" + + "old0............................................" + + "new0............................................", + ), + }, + { + desc: "oldest key is discarded", + ticketKey: []byte("" + + "old10..........................................." + + "old9............................................" + + "old8............................................" + + "old7............................................" + + "old6............................................" + + "old5............................................" + + "old4............................................" + + "old3............................................" + + "old2............................................" + + "old1............................................" + + "old0............................................" + + "old11...........................................", + ), + want: []byte("" + + "old11..........................................." + + "old10..........................................." + + "old9............................................" + + "old8............................................" + + "old7............................................" + + "old6............................................" + + "old5............................................" + + "old4............................................" + + "old3............................................" + + "old2............................................" + + "old1............................................" + + "new0............................................", + ), + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + var g tlsKeyGenerator + + newTicketKey, err := UpdateTLSTicketKeyFunc(tt.ticketKey, g.generateKey) + if err != nil { + t.Fatalf("UpdateTLSTicketKeyFunc: %v", err) + } + + if got, want := newTicketKey, tt.want; !bytes.Equal(got, want) { + t.Errorf("newTicketKey = %s, want %s", got, want) + } + }) + } +} + +func TestCreateTLSTicketKeyFiles(t *testing.T) { + ticketKey := []byte("" + + "old1............................................" + + "old0............................................" + + "new0............................................", + ) + + files := CreateTLSTicketKeyFiles("/foo/bar", ticketKey) + + if got, want := len(files), len(ticketKey)/TLSTicketKeySize; got != want { + t.Fatalf("len(files) = %v, want %v", got, want) + } + + wantFiles := []*PrivateChecksumFile{ + { + Path: "/foo/bar/tls-ticket-key/key-0", + Content: []byte("old1............................................"), + Checksum: hexMustDecodeString("caba3c7923d7e867d0fd00665fbcfbe9b1d73925097aa0bc0f220590df5a7fc4"), + }, + { + Path: "/foo/bar/tls-ticket-key/key-1", + Content: []byte("old0............................................"), + Checksum: hexMustDecodeString("a86b67f250b9a6bba17908f0c448d7dc63f0c66964229a2341b73d7008d35cf3"), + }, + { + Path: "/foo/bar/tls-ticket-key/key-2", + Content: []byte("new0............................................"), + Checksum: hexMustDecodeString("c2816f6b0278c715029a1de1487c14fb6daad4655e8149becda7259b42cc1fc5"), + }, + } + + for i, f := range files { + if got, want := f, wantFiles[i]; !reflect.DeepEqual(got, want) { + t.Errorf("files[%v] = %q, want %q", i, got, want) + } + } +} diff --git a/pkg/nghttpx/types.go b/pkg/nghttpx/types.go index e3fc85ef..062011e6 100644 --- a/pkg/nghttpx/types.go +++ b/pkg/nghttpx/types.go @@ -85,6 +85,10 @@ type IngressConfig struct { HTTP3 bool // QUICSecretFile is the file which contains QUIC keying materials. QUICSecretFile *PrivateChecksumFile + // ShareTLSTicketKey, if true, shares TLS ticket key among ingress controllers via Secret. + ShareTLSTicketKey bool + // TLSTicketKeyFiles is the list of files that contain TLS ticket key. + TLSTicketKeyFiles []*PrivateChecksumFile } // Upstream describes an nghttpx upstream