diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index 07c2e19c4..7ea420011 100644 --- a/cmd/nginx/flags.go +++ b/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, diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 910a8c049..5cd700f35 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -685,6 +685,7 @@ type TemplateConfig struct { ListenPorts *ListenPorts PublishService *apiv1.Service DynamicConfigurationEnabled bool + DynamicCertificatesEnabled bool DisableLua bool } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 55bbfc6bf..943459d82 100644 --- a/internal/ingress/controller/controller.go +++ b/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)) { diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 492f7cad5..18c40eccd 100644 --- a/internal/ingress/controller/nginx.go +++ b/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, ©OfServer) + } + 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(©OfRunningConfig) + clearCertificates(©OfPcfg) + } + return copyOfRunningConfig.Equal(©OfPcfg) } // 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 diff --git a/internal/ingress/controller/nginx_test.go b/internal/ingress/controller/nginx_test.go index 5b174f1c0..969ef2a57 100644 --- a/internal/ingress/controller/nginx_test.go +++ b/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 diff --git a/internal/ingress/controller/store/backend_ssl.go b/internal/ingress/controller/store/backend_ssl.go index b6635d51e..b85abe136 100644 --- a/internal/ingress/controller/store/backend_ssl.go +++ b/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 '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 + 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, fmt.Errorf("unexpected error creating pem file: %v", err) + } } msg := fmt.Sprintf("Configuring Secret %q for TLS encryption (CN: %v)", secretName, sslCert.CN) diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index e92fc3536..781ee6501 100644 --- a/internal/ingress/controller/store/store.go +++ b/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,19 +222,21 @@ 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, - informers: &Informer{}, - listers: &Lister{}, - sslStore: NewSSLCertTracker(), - filesystem: fs, - updateCh: updateCh, - backendConfig: ngx_config.NewDefault(), - mu: &sync.Mutex{}, - secretIngressMap: NewObjectRefMap(), - defaultSSLCertificate: defaultSSLCertificate, + isOCSPCheckEnabled: checkOCSP, + informers: &Informer{}, + listers: &Lister{}, + sslStore: NewSSLCertTracker(), + filesystem: fs, + updateCh: updateCh, + backendConfig: ngx_config.NewDefault(), + mu: &sync.Mutex{}, + secretIngressMap: NewObjectRefMap(), + defaultSSLCertificate: defaultSSLCertificate, + isDynamicCertificatesEnabled: isDynamicCertificatesEnabled, } eventBroadcaster := record.NewBroadcaster() diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 04eeefea1..8bb6f8f66 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/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) diff --git a/internal/ingress/sslcert.go b/internal/ingress/sslcert.go index f54b86d25..4b585a583 100644 --- a/internal/ingress/sslcert.go +++ b/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 diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index 337d32ce9..0c1b64beb 100644 --- a/internal/ingress/types_equals.go +++ b/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 diff --git a/internal/net/ssl/ssl.go b/internal/net/ssl/ssl.go index 6e7753b2c..ca708b79f 100644 --- a/internal/net/ssl/ssl.go +++ b/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 { diff --git a/internal/net/ssl/ssl_test.go b/internal/net/ssl/ssl_test.go index d6456050b..f3ec381d5 100644 --- a/internal/net/ssl/ssl_test.go +++ b/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]) + } +}