Merge pull request #2923 from Shopify/dynamic-certs-controller

Add dynamic certificate serving feature to controller

cmd/nginx/flags.go

@@ -147,6 +147,10 @@ Requires the update-status parameter.`)
 			`Dynamically refresh backends on topology changes instead of reloading NGINX.
 Feature backed by OpenResty Lua libraries.`)
 
+		dynamicCertificatesEnabled = flags.Bool("enable-dynamic-certificates", false,
+			`Dynamically update SSL certificates instead of reloading NGINX.
+Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not enabled`)
+
 		httpPort      = flags.Int("http-port", 80, `Port to use for servicing HTTP traffic.`)
 		httpsPort     = flags.Int("https-port", 443, `Port to use for servicing HTTPS traffic.`)
 		statusPort    = flags.Int("status-port", 18080, `Port to use for exposing NGINX status pages.`)
@@ -213,6 +217,11 @@ Feature backed by OpenResty Lua libraries.`)
 		glog.Warningf("SSL certificate chain completion is disabled (--enable-ssl-chain-completion=false)")
 	}
 
+	if (*enableSSLChainCompletion || !*dynamicConfigurationEnabled) && *dynamicCertificatesEnabled {
+		return false, nil, fmt.Errorf(`SSL certificate chain completion cannot be enabled and dynamic configration cannot be disabled when 
+dynamic certificates functionality is enabled. Please check the flags --enable-ssl-chain-completion and --enable-dynamic-configuration`)
+	}
+
 	// LuaJIT is not available on arch s390x and ppc64le
 	disableLua := false
 	if runtime.GOARCH == "s390x" || runtime.GOARCH == "ppc64le" {
@@ -248,6 +257,7 @@ Feature backed by OpenResty Lua libraries.`)
 		SyncRateLimit:               *syncRateLimit,
 		DynamicConfigurationEnabled: *dynamicConfigurationEnabled,
 		DisableLua:                  disableLua,
+		DynamicCertificatesEnabled:  *dynamicCertificatesEnabled,
 		ListenPorts: &ngx_config.ListenPorts{
 			Default:  *defServerPort,
 			Health:   *healthzPort,

internal/ingress/controller/config/config.go

@@ -685,6 +685,7 @@ type TemplateConfig struct {
 	ListenPorts                 *ListenPorts
 	PublishService              *apiv1.Service
 	DynamicConfigurationEnabled bool
+	DynamicCertificatesEnabled  bool
 	DisableLua                  bool
 }
 

internal/ingress/controller/controller.go

@@ -96,6 +96,8 @@ type Configuration struct {
 	DynamicConfigurationEnabled bool
 
 	DisableLua bool
+
+	DynamicCertificatesEnabled bool
 }
 
 // GetPublishService returns the Service used to set the load-balancer status of Ingresses.
@@ -197,7 +199,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
 				// it takes time for NGINX to start listening on the configured ports
 				time.Sleep(1 * time.Second)
 			}
-			err := configureDynamically(pcfg, n.cfg.ListenPorts.Status)
+			err := configureDynamically(pcfg, n.cfg.ListenPorts.Status, n.cfg.DynamicCertificatesEnabled)
 			if err == nil {
 				glog.Infof("Dynamic reconfiguration succeeded.")
 			} else {
@@ -1071,6 +1073,12 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
 				}
 			}
 
+			if n.cfg.DynamicCertificatesEnabled {
+				// useless placeholders: just to shut up NGINX configuration loader errors:
+				cert.PemFileName = defaultPemFileName
+				cert.PemSHA = defaultPemSHA
+			}
+
 			servers[host].SSLCert = *cert
 
 			if cert.ExpireTime.Before(time.Now().Add(240 * time.Hour)) {

internal/ingress/controller/nginx.go

@@ -118,7 +118,8 @@ func NewNGINXController(config *Configuration, mc metric.Collector, fs file.File
 		config.ResyncPeriod,
 		config.Client,
 		fs,
-		n.updateCh)
+		n.updateCh,
+		config.DynamicCertificatesEnabled)
 
 	n.syncQueue = task.NewTaskQueue(n.syncIngress)
 
@@ -592,6 +593,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 		ListenPorts:                 n.cfg.ListenPorts,
 		PublishService:              n.GetPublishService(),
 		DynamicConfigurationEnabled: n.cfg.DynamicConfigurationEnabled,
+		DynamicCertificatesEnabled:  n.cfg.DynamicCertificatesEnabled,
 		DisableLua:                  n.cfg.DisableLua,
 	}
 
@@ -723,6 +725,18 @@ func (n *NGINXController) setupSSLProxy() {
 	}()
 }
 
+// Helper function to clear Certificates from the ingress configuration since they should be ignored when
+// checking if the new configuration changes can be applied dynamically if dynamic certificates is on
+func clearCertificates(config *ingress.Configuration) {
+	var clearedServers []*ingress.Server
+	for _, server := range config.Servers {
+		copyOfServer := *server
+		copyOfServer.SSLCert = ingress.SSLCert{}
+		clearedServers = append(clearedServers, &copyOfServer)
+	}
+	config.Servers = clearedServers
+}
+
 // IsDynamicConfigurationEnough returns whether a Configuration can be
 // dynamically applied, without reloading the backend.
 func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configuration) bool {
@@ -732,12 +746,17 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati
 	copyOfRunningConfig.Backends = []*ingress.Backend{}
 	copyOfPcfg.Backends = []*ingress.Backend{}
 
+	if n.cfg.DynamicCertificatesEnabled {
+		clearCertificates(&copyOfRunningConfig)
+		clearCertificates(&copyOfPcfg)
+	}
+
 	return copyOfRunningConfig.Equal(&copyOfPcfg)
 }
 
 // configureDynamically encodes new Backends in JSON format and POSTs the
 // payload to an internal HTTP endpoint handled by Lua.
-func configureDynamically(pcfg *ingress.Configuration, port int) error {
+func configureDynamically(pcfg *ingress.Configuration, port int, isDynamicCertificatesEnabled bool) error {
 	backends := make([]*ingress.Backend, len(pcfg.Backends))
 
 	for i, backend := range pcfg.Backends {
@@ -770,14 +789,53 @@ func configureDynamically(pcfg *ingress.Configuration, port int) error {
 		backends[i] = luaBackend
 	}
 
-	buf, err := json.Marshal(backends)
+	url := fmt.Sprintf("http://localhost:%d/configuration/backends", port)
+	err := post(url, backends)
 	if err != nil {
 		return err
 	}
 
-	glog.V(2).Infof("Posting backends configuration: %s", buf)
+	if isDynamicCertificatesEnabled {
+		err = configureCertificates(pcfg, port)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// configureCertificates JSON encodes certificates and POSTs it to an internal HTTP endpoint
+// that is handled by Lua
+func configureCertificates(pcfg *ingress.Configuration, port int) error {
+	var servers []*ingress.Server
+
+	for _, server := range pcfg.Servers {
+		servers = append(servers, &ingress.Server{
+			Hostname: server.Hostname,
+			SSLCert: ingress.SSLCert{
+				PemCertKey: server.SSLCert.PemCertKey,
+			},
+		})
+	}
+
+	url := fmt.Sprintf("http://localhost:%d/configuration/servers", port)
+	err := post(url, servers)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func post(url string, data interface{}) error {
+	buf, err := json.Marshal(data)
+	if err != nil {
+		return err
+	}
+
+	glog.V(2).Infof("Posting to %s: %s", url, buf)
 
-	url := fmt.Sprintf("http://localhost:%d/configuration/backends", port)
 	resp, err := http.Post(url, "application/json", bytes.NewReader(buf))
 	if err != nil {
 		return err

internal/ingress/controller/nginx_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controller
 
 import (
+	"encoding/json"
 	"io"
 	"io/ioutil"
 	"net"
@@ -52,6 +53,9 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
 				Backend: "fakenamespace-myapp-80",
 			},
 		},
+		SSLCert: ingress.SSLCert{
+			PemCertKey: "fake-certificate",
+		},
 	}}
 
 	commonConfig := &ingress.Configuration{
@@ -64,6 +68,9 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
 			Backends: backends,
 			Servers:  servers,
 		},
+		cfg: &Configuration{
+			DynamicCertificatesEnabled: false,
+		},
 	}
 
 	newConfig := commonConfig
@@ -87,11 +94,53 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
 		t.Errorf("Expected to be dynamically configurable when only backends change")
 	}
 
+	n.cfg.DynamicCertificatesEnabled = true
+
+	newServers := []*ingress.Server{{
+		Hostname: "myapp1.fake",
+		Locations: []*ingress.Location{
+			{
+				Path:    "/",
+				Backend: "fakenamespace-myapp-80",
+			},
+		},
+		SSLCert: ingress.SSLCert{
+			PemCertKey: "fake-certificate",
+		},
+	}}
+
+	newConfig = &ingress.Configuration{
+		Backends: backends,
+		Servers:  newServers,
+	}
+	if n.IsDynamicConfigurationEnough(newConfig) {
+		t.Errorf("Expected to not be dynamically configurable when dynamic certificates is enabled and a non-certificate field in servers is updated")
+	}
+
+	newServers[0].Hostname = "myapp.fake"
+	newServers[0].SSLCert.PemCertKey = "new-fake-certificate"
+
+	newConfig = &ingress.Configuration{
+		Backends: backends,
+		Servers:  newServers,
+	}
+	if !n.IsDynamicConfigurationEnough(newConfig) {
+		t.Errorf("Expected to be dynamically configurable when only SSLCert changes")
+	}
+
+	newConfig = &ingress.Configuration{
+		Backends: []*ingress.Backend{{Name: "a-backend-8080"}},
+		Servers:  newServers,
+	}
+	if !n.IsDynamicConfigurationEnough(newConfig) {
+		t.Errorf("Expected to be dynamically configurable when backend and SSLCert changes")
+	}
+
 	if !n.runningConfig.Equal(commonConfig) {
 		t.Errorf("Expected running config to not change")
 	}
 
-	if !newConfig.Equal(&ingress.Configuration{Backends: []*ingress.Backend{{Name: "a-backend-8080"}}, Servers: servers}) {
+	if !newConfig.Equal(&ingress.Configuration{Backends: []*ingress.Backend{{Name: "a-backend-8080"}}, Servers: newServers}) {
 		t.Errorf("Expected new config to not change")
 	}
 }
@@ -157,7 +206,7 @@ func TestConfigureDynamically(t *testing.T) {
 	port := ts.Listener.Addr().(*net.TCPAddr).Port
 	defer ts.Close()
 
-	err := configureDynamically(commonConfig, port)
+	err := configureDynamically(commonConfig, port, false)
 	if err != nil {
 		t.Errorf("unexpected error posting dynamic configuration: %v", err)
 	}
@@ -167,6 +216,56 @@ func TestConfigureDynamically(t *testing.T) {
 	}
 }
 
+func TestConfigureCertificates(t *testing.T) {
+
+	servers := []*ingress.Server{{
+		Hostname: "myapp.fake",
+		SSLCert: ingress.SSLCert{
+			PemCertKey: "fake-cert",
+		},
+	}}
+
+	commonConfig := &ingress.Configuration{
+		Servers: servers,
+	}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusCreated)
+
+		if r.Method != "POST" {
+			t.Errorf("expected a 'POST' request, got '%s'", r.Method)
+		}
+
+		b, err := ioutil.ReadAll(r.Body)
+		if err != nil && err != io.EOF {
+			t.Fatal(err)
+		}
+		var postedServers []ingress.Server
+		err = json.Unmarshal(b, &postedServers)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if len(servers) != len(postedServers) {
+			t.Errorf("Expected servers to be the same length as the posted servers")
+		}
+
+		for i, server := range servers {
+			if !server.Equal(&postedServers[i]) {
+				t.Errorf("Expected servers and posted servers to be equal")
+			}
+		}
+	}))
+
+	port := ts.Listener.Addr().(*net.TCPAddr).Port
+	defer ts.Close()
+
+	err := configureCertificates(commonConfig, port)
+	if err != nil {
+		t.Errorf("unexpected error posting dynamic certificate configuration: %v", err)
+	}
+}
+
 func TestNginxHashBucketSize(t *testing.T) {
 	tests := []struct {
 		n        int

internal/ingress/controller/store/backend_ssl.go

@@ -98,11 +98,18 @@ func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error)
 			return nil, fmt.Errorf("key 'tls.key' missing from Secret %q", secretName)
 		}
 
+		if s.isDynamicCertificatesEnabled {
+			sslCert, err = ssl.CreateSSLCert(nsSecName, cert, key, ca)
+			if err != nil {
+				return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err)
+			}
+		} else {
 			// If 'ca.crt' is also present, it will allow this secret to be used in the
 			// 'nginx.ingress.kubernetes.io/auth-tls-secret' annotation
 			sslCert, err = ssl.AddOrUpdateCertAndKey(nsSecName, cert, key, ca, s.filesystem)
 			if err != nil {
-			return nil, err
+				return nil, fmt.Errorf("unexpected error creating pem file: %v", err)
+			}
 		}
 
 		msg := fmt.Sprintf("Configuring Secret %q for TLS encryption (CN: %v)", secretName, sslCert.CN)

internal/ingress/controller/store/store.go

@@ -212,6 +212,8 @@ type k8sStore struct {
 	mu *sync.Mutex
 
 	defaultSSLCertificate string
+
+	isDynamicCertificatesEnabled bool
 }
 
 // New creates a new object store to be used in the ingress controller
@@ -220,7 +222,8 @@ func New(checkOCSP bool,
 	resyncPeriod time.Duration,
 	client clientset.Interface,
 	fs file.Filesystem,
-	updateCh *channels.RingChannel) Storer {
+	updateCh *channels.RingChannel,
+	isDynamicCertificatesEnabled bool) Storer {
 
 	store := &k8sStore{
 		isOCSPCheckEnabled:           checkOCSP,
@@ -233,6 +236,7 @@ func New(checkOCSP bool,
 		mu:                           &sync.Mutex{},
 		secretIngressMap:             NewObjectRefMap(),
 		defaultSSLCertificate:        defaultSSLCertificate,
+		isDynamicCertificatesEnabled: isDynamicCertificatesEnabled,
 	}
 
 	eventBroadcaster := record.NewBroadcaster()

internal/ingress/controller/store/store_test.go

@@ -68,7 +68,8 @@ func TestStore(t *testing.T) {
 			10*time.Minute,
 			clientSet,
 			fs,
-			updateCh)
+			updateCh,
+			false)
 
 		storer.Run(stopCh)
 
@@ -155,7 +156,8 @@ func TestStore(t *testing.T) {
 			10*time.Minute,
 			clientSet,
 			fs,
-			updateCh)
+			updateCh,
+			false)
 
 		storer.Run(stopCh)
 
@@ -302,7 +304,8 @@ func TestStore(t *testing.T) {
 			10*time.Minute,
 			clientSet,
 			fs,
-			updateCh)
+			updateCh,
+			false)
 
 		storer.Run(stopCh)
 
@@ -390,7 +393,8 @@ func TestStore(t *testing.T) {
 			10*time.Minute,
 			clientSet,
 			fs,
-			updateCh)
+			updateCh,
+			false)
 
 		storer.Run(stopCh)
 
@@ -501,7 +505,8 @@ func TestStore(t *testing.T) {
 			10*time.Minute,
 			clientSet,
 			fs,
-			updateCh)
+			updateCh,
+			false)
 
 		storer.Run(stopCh)
 

internal/ingress/sslcert.go

@@ -42,6 +42,8 @@ type SSLCert struct {
 	CN []string `json:"cn"`
 	// ExpiresTime contains the expiration of this SSL certificate in timestamp format
 	ExpireTime time.Time `json:"expires"`
+	// Pem encoded certificate and key concatenated
+	PemCertKey string `json:"pemCertKey"`
 }
 
 // GetObjectKind implements the ObjectKind interface as a noop

internal/ingress/types_equals.go

@@ -505,6 +505,9 @@ func (s1 *SSLCert) Equal(s2 *SSLCert) bool {
 	if s1.FullChainPemFileName != s2.FullChainPemFileName {
 		return false
 	}
+	if s1.PemCertKey != s2.PemCertKey {
+		return false
+	}
 
 	for _, cn1 := range s1.CN {
 		found := false

internal/net/ssl/ssl.go

@@ -17,6 +17,7 @@ limitations under the License.
 package ssl
 
 import (
+	"bytes"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/tls"
@@ -187,6 +188,86 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte,
 	return s, nil
 }
 
+// CreateSSLCert creates an SSLCert and avoids writing on disk
+func CreateSSLCert(name string, cert, key, ca []byte) (*ingress.SSLCert, error) {
+	var pemCertBuffer bytes.Buffer
+
+	pemCertBuffer.Write(cert)
+	pemCertBuffer.Write([]byte("\n"))
+	pemCertBuffer.Write(key)
+
+	pemBlock, _ := pem.Decode(pemCertBuffer.Bytes())
+	if pemBlock == nil {
+		return nil, fmt.Errorf("no valid PEM formatted block found")
+	}
+
+	// If the file does not start with 'BEGIN CERTIFICATE' it's invalid and must not be used.
+	if pemBlock.Type != "CERTIFICATE" {
+		return nil, fmt.Errorf("certificate %v contains invalid data, and must be created with 'kubectl create secret tls'", name)
+	}
+
+	pemCert, err := x509.ParseCertificate(pemBlock.Bytes)
+	if err != nil {
+		return nil, err
+	}
+
+	//Ensure that certificate and private key have a matching public key
+	if _, err := tls.X509KeyPair(cert, key); err != nil {
+		return nil, err
+	}
+
+	cn := sets.NewString(pemCert.Subject.CommonName)
+	for _, dns := range pemCert.DNSNames {
+		if !cn.Has(dns) {
+			cn.Insert(dns)
+		}
+	}
+
+	if len(pemCert.Extensions) > 0 {
+		glog.V(3).Info("parsing ssl certificate extensions")
+		for _, ext := range getExtension(pemCert, oidExtensionSubjectAltName) {
+			dns, _, _, err := parseSANExtension(ext.Value)
+			if err != nil {
+				glog.Warningf("unexpected error parsing certificate extensions: %v", err)
+				continue
+			}
+
+			for _, dns := range dns {
+				if !cn.Has(dns) {
+					cn.Insert(dns)
+				}
+			}
+		}
+	}
+
+	if len(ca) > 0 {
+		bundle := x509.NewCertPool()
+		bundle.AppendCertsFromPEM(ca)
+		opts := x509.VerifyOptions{
+			Roots: bundle,
+		}
+
+		_, err := pemCert.Verify(opts)
+		if err != nil {
+			oe := fmt.Sprintf("failed to verify certificate chain: \n\t%s\n", err)
+			return nil, errors.New(oe)
+		}
+
+		pemCertBuffer.Write([]byte("\n"))
+		pemCertBuffer.Write(ca)
+		pemCertBuffer.Write([]byte("\n"))
+	}
+
+	s := &ingress.SSLCert{
+		Certificate: pemCert,
+		CN:          cn.List(),
+		ExpireTime:  pemCert.NotAfter,
+		PemCertKey:  pemCertBuffer.String(),
+	}
+
+	return s, nil
+}
+
 func getExtension(c *x509.Certificate, id asn1.ObjectIdentifier) []pkix.Extension {
 	var exts []pkix.Extension
 	for _, ext := range c.Extensions {

internal/net/ssl/ssl_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package ssl
 
 import (
+	"bytes"
 	"crypto/x509"
 	"fmt"
 	"testing"
@@ -147,3 +148,37 @@ func newFS(t *testing.T) file.Filesystem {
 	}
 	return fs
 }
+
+func TestCreateSSLCert(t *testing.T) {
+	cert, _, err := generateRSACerts("echoheaders")
+	if err != nil {
+		t.Fatalf("unexpected error creating SSL certificate: %v", err)
+	}
+
+	name := fmt.Sprintf("test-%v", time.Now().UnixNano())
+
+	c := certutil.EncodeCertPEM(cert.Cert)
+	k := certutil.EncodePrivateKeyPEM(cert.Key)
+
+	ngxCert, err := CreateSSLCert(name, c, k, []byte{})
+	if err != nil {
+		t.Fatalf("unexpected error checking SSL certificate: %v", err)
+	}
+
+	var certKeyBuf bytes.Buffer
+	certKeyBuf.Write(c)
+	certKeyBuf.Write([]byte("\n"))
+	certKeyBuf.Write(k)
+
+	if ngxCert.PemCertKey != certKeyBuf.String() {
+		t.Fatalf("expected concatenated PEM cert and key but returned %v", ngxCert.PemCertKey)
+	}
+
+	if len(ngxCert.CN) == 0 {
+		t.Fatalf("expected at least one cname but none returned")
+	}
+
+	if ngxCert.CN[0] != "echoheaders" {
+		t.Fatalf("expected cname echoheaders but %v returned", ngxCert.CN[0])
+	}
+}