Merge pull request #2782 from diazjf/fix-2756

Add Better Error Handling for SSLSessionTicketKey

Author: k8s-ci-robot

docs/user-guide/nginx-configuration/configmap.md

@@ -434,8 +434,9 @@ Enables or disables session resumption through [TLS session tickets](http://ngin
 ## ssl-session-ticket-key
 
 Sets the secret key used to encrypt and decrypt TLS session tickets. The value must be a valid base64 string.
+To create a ticket: `openssl rand 80 | openssl enc -A -base64`
 
-[TLS session ticket-key](http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_session_tickets), by default, a randomly generated key is used. To create a ticket: `openssl rand 80 | base64 -w0`
+[TLS session ticket-key](http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_session_tickets), by default, a randomly generated key is used. 
 
 ## ssl-session-timeout
 

internal/ingress/controller/config/config.go

@@ -303,7 +303,7 @@ type Configuration struct {
 	// Sets the secret key used to encrypt and decrypt TLS session tickets.
 	// http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_session_tickets
 	// By default, a randomly generated key is used.
-	// Example: openssl rand 80 | base64 -w0
+	// Example: openssl rand 80 | openssl enc -A -base64
 	SSLSessionTicketKey string `json:"ssl-session-ticket-key,omitempty"`
 
 	// Time during which a client may reuse the session parameters stored in a cache.

internal/ingress/controller/store/store.go

@@ -521,8 +521,8 @@ func New(checkOCSP bool,
 	if err != nil {
 		glog.Warningf("Unexpected error reading configuration configmap: %v", err)
 	}
-	store.setConfig(cm)
 
+	store.setConfig(cm)
 	return store
 }
 
@@ -708,6 +708,34 @@ func (s k8sStore) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error)
 	}, nil
 }
 
+func (s k8sStore) writeSSLSessionTicketKey(cmap *corev1.ConfigMap, fileName string) {
+	ticketString := ngx_template.ReadConfig(cmap.Data).SSLSessionTicketKey
+	s.backendConfig.SSLSessionTicketKey = ""
+
+	if ticketString != "" {
+		ticketBytes := base64.StdEncoding.WithPadding(base64.StdPadding).DecodedLen(len(ticketString))
+
+		// 81 used instead of 80 because of padding
+		if !(ticketBytes == 48 || ticketBytes == 81) {
+			glog.Warningf("ssl-session-ticket-key must contain either 48 or 80 bytes")
+		}
+
+		decodedTicket, err := base64.StdEncoding.DecodeString(ticketString)
+		if err != nil {
+			glog.Errorf("unexpected error decoding ssl-session-ticket-key: %v", err)
+			return
+		}
+
+		err = ioutil.WriteFile(fileName, decodedTicket, file.ReadWriteByUser)
+		if err != nil {
+			glog.Errorf("unexpected error writing ssl-session-ticket-key to %s: %v", fileName, err)
+			return
+		}
+
+		s.backendConfig.SSLSessionTicketKey = ticketString
+	}
+}
+
 // GetDefaultBackend returns the default backend
 func (s k8sStore) GetDefaultBackend() defaults.Backend {
 	return s.backendConfig.Backend
@@ -719,16 +747,7 @@ func (s k8sStore) GetBackendConfiguration() ngx_config.Configuration {
 
 func (s *k8sStore) setConfig(cmap *corev1.ConfigMap) {
 	s.backendConfig = ngx_template.ReadConfig(cmap.Data)
-
-	// TODO: this should not be done here
-	if s.backendConfig.SSLSessionTicketKey != "" {
-		d, err := base64.StdEncoding.DecodeString(s.backendConfig.SSLSessionTicketKey)
-		if err != nil {
-			glog.Warningf("unexpected error decoding key ssl-session-ticket-key: %v", err)
-			s.backendConfig.SSLSessionTicketKey = ""
-		}
-		ioutil.WriteFile("/etc/nginx/tickets.key", d, file.ReadWriteByUser)
-	}
+	s.writeSSLSessionTicketKey(cmap, "/etc/nginx/tickets.key")
 }
 
 // Run initiates the synchronization of the informers and the initial

internal/ingress/controller/store/store_test.go

@@ -33,6 +33,9 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 
+	"encoding/base64"
+	"io/ioutil"
+	"k8s.io/api/core/v1"
 	"k8s.io/ingress-nginx/internal/file"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
 	"k8s.io/ingress-nginx/test/e2e/framework"
@@ -828,3 +831,39 @@ func TestListIngresses(t *testing.T) {
 		t.Errorf("Expected 3 Ingresses but got %v", s)
 	}
 }
+
+func TestWriteSSLSessionTicketKey(t *testing.T) {
+	tests := []string{
+		"9DyULjtYWz520d1rnTLbc4BOmN2nLAVfd3MES/P3IxWuwXkz9Fby0lnOZZUdNEMV",
+		"9SvN1C9AB5DvNde5fMKoJwAwICpqdjiMyxR+cv6NpAWv22rFd3gKt4wMyGxCm7l9Wh6BQPG0+csyBZSHHr2NOWj52Wx8xCegXf4NsSMBUqA=",
+	}
+
+	for _, test := range tests {
+		s := newStore(t)
+
+		cmap := &v1.ConfigMap{
+			Data: map[string]string{
+				"ssl-session-ticket-key": test,
+			},
+		}
+
+		f, err := ioutil.TempFile("", "ssl-session-ticket-test")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		s.writeSSLSessionTicketKey(cmap, f.Name())
+
+		content, err := ioutil.ReadFile(f.Name())
+		if err != nil {
+			t.Fatal(err)
+		}
+		encodedContent := base64.StdEncoding.EncodeToString(content)
+
+		f.Close()
+
+		if test != encodedContent {
+			t.Fatalf("expected %v but returned %s", test, encodedContent)
+		}
+	}
+}