Merge pull request #1221 from aledbf/fix-ca-ssl

Move certificate authentication from location to server

controllers/nginx/pkg/cmd/controller/nginx.go

@@ -488,7 +488,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 			} else {
 				n = fmt.Sprintf("www.%v", srv.Hostname)
 			}
-			glog.V(3).Infof("creating redirect from %v to", srv.Hostname, n)
+			glog.V(3).Infof("creating redirect from %v to %v", srv.Hostname, n)
 			if _, ok := redirectServers[n]; !ok {
 				found := false
 				for _, esrv := range ingressCfg.Servers {

controllers/nginx/pkg/template/template.go

@@ -339,7 +339,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
 
 func filterRateLimits(input interface{}) []ratelimit.RateLimit {
 	ratelimits := []ratelimit.RateLimit{}
-	found := map[string]bool{}
+	found := sets.String{}
 
 	servers, ok := input.([]*ingress.Server)
 	if !ok {
@@ -347,8 +347,8 @@ func filterRateLimits(input interface{}) []ratelimit.RateLimit {
 	}
 	for _, server := range servers {
 		for _, loc := range server.Locations {
-			if loc.RateLimit.ID != "" && !found[loc.RateLimit.ID] {
-				found[loc.RateLimit.ID] = true
+			if loc.RateLimit.ID != "" && !found.Has(loc.RateLimit.ID) {
+				found.Insert(loc.RateLimit.ID)
 				ratelimits = append(ratelimits, loc.RateLimit)
 			}
 		}

controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl

@@ -536,17 +536,17 @@ stream {
 
         {{ if $all.Cfg.EnableVtsStatus }}vhost_traffic_status_filter_by_set_key $geoip_country_code country::$server_name;{{ end }}
 
+        {{ if not (empty $server.CertificateAuth.CAFileName) }}
+        # PEM sha: {{ $server.CertificateAuth.PemSHA }}
+        ssl_client_certificate                  {{ $server.CertificateAuth.CAFileName }};
+        ssl_verify_client                       on;
+        ssl_verify_depth                        {{ $server.CertificateAuth.ValidationDepth }};
+        {{ end }}
+
         {{ range $location := $server.Locations }}
         {{ $path := buildLocation $location }}
         {{ $authPath := buildAuthLocation $location }}
 
-        {{ if not (empty $location.CertificateAuth.AuthSSLCert.CAFileName) }}
-        # PEM sha: {{ $location.CertificateAuth.AuthSSLCert.PemSHA }}
-        ssl_client_certificate              {{ $location.CertificateAuth.AuthSSLCert.CAFileName }};
-        ssl_verify_client on;
-        ssl_verify_depth {{ $location.CertificateAuth.ValidationDepth }};
-        {{ end }}
-
         {{ if not (empty $location.Rewrite.AppRoot)}}
         if ($uri = /) {
         return 302 {{ $location.Rewrite.AppRoot }};
@@ -648,7 +648,7 @@ stream {
         proxy_set_header Host                   $best_http_host;
 
         # Pass the extracted client certificate to the backend
-        {{ if not (empty $location.CertificateAuth.AuthSSLCert.CAFileName) }}
+        {{ if not (empty $server.CertificateAuth.CAFileName) }}
         proxy_set_header ssl-client-cert        $ssl_client_cert;
         {{ end }}
 

core/pkg/ingress/annotations/authtls/main.go

@@ -36,8 +36,8 @@ const (
 // AuthSSLConfig contains the AuthSSLCert used for muthual autentication
 // and the configured ValidationDepth
 type AuthSSLConfig struct {
-	AuthSSLCert     resolver.AuthSSLCert `json:"authSSLCert"`
-	ValidationDepth int                  `json:"validationDepth"`
+	resolver.AuthSSLCert
+	ValidationDepth int `json:"validationDepth"`
 }
 
 // Equal tests for equality between two AuthSSLConfig types

core/pkg/ingress/annotations/ratelimit/main.go

@@ -91,9 +91,15 @@ func (rt1 *RateLimit) Equal(rt2 *RateLimit) bool {
 	if rt1.LimitRateAfter != rt2.LimitRateAfter {
 		return false
 	}
+	if rt1.ID != rt2.ID {
+		return false
+	}
 	if rt1.Name != rt2.Name {
 		return false
 	}
+	if len(rt1.Whitelist) != len(rt2.Whitelist) {
+		return false
+	}
 
 	for _, r1l := range rt1.Whitelist {
 		found := false

core/pkg/ingress/controller/annotations.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/ingress/core/pkg/ingress/annotations/auth"
 	"k8s.io/ingress/core/pkg/ingress/annotations/authreq"
 	"k8s.io/ingress/core/pkg/ingress/annotations/authtls"
+	"k8s.io/ingress/core/pkg/ingress/annotations/clientbodybuffersize"
 	"k8s.io/ingress/core/pkg/ingress/annotations/cors"
 	"k8s.io/ingress/core/pkg/ingress/annotations/healthcheck"
 	"k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist"
@@ -39,7 +40,6 @@ import (
 	"k8s.io/ingress/core/pkg/ingress/annotations/sslpassthrough"
 	"k8s.io/ingress/core/pkg/ingress/errors"
 	"k8s.io/ingress/core/pkg/ingress/resolver"
-	"k8s.io/ingress/core/pkg/ingress/annotations/clientbodybuffersize"
 )
 
 type extractorConfig interface {
@@ -115,6 +115,7 @@ const (
 	serviceUpstream      = "ServiceUpstream"
 	serverAlias          = "Alias"
 	clientBodyBufferSize = "ClientBodyBufferSize"
+	certificateAuth      = "CertificateAuth"
 )
 
 func (e *annotationExtractor) ServiceUpstream(ing *extensions.Ingress) bool {
@@ -155,3 +156,16 @@ func (e *annotationExtractor) SessionAffinity(ing *extensions.Ingress) *sessiona
 	val, _ := e.annotations[sessionAffinity].Parse(ing)
 	return val.(*sessionaffinity.AffinityConfig)
 }
+
+func (e *annotationExtractor) CertificateAuth(ing *extensions.Ingress) *authtls.AuthSSLConfig {
+	val, err := e.annotations[certificateAuth].Parse(ing)
+	if errors.IsMissingAnnotations(err) {
+		return nil
+	}
+
+	if err != nil {
+		glog.Errorf("error parsing certificate auth: %v", err)
+	}
+	secure := val.(*authtls.AuthSSLConfig)
+	return secure
+}

core/pkg/ingress/controller/controller.go

@@ -652,6 +652,15 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress
 				continue
 			}
 
+			if server.CertificateAuth.CAFileName == "" {
+				ca := ic.annotations.CertificateAuth(ing)
+				if ca != nil {
+					server.CertificateAuth = *ca
+				}
+			} else {
+				glog.V(3).Infof("server %v already contains a muthual autentication configuration - ingress rule %v/%v", server.Hostname, ing.Namespace, ing.Name)
+			}
+
 			for _, path := range rule.HTTP.Paths {
 				upsName := fmt.Sprintf("%v-%v-%v",
 					ing.GetNamespace(),
@@ -880,7 +889,7 @@ func (ic *GenericController) createUpstreams(data []interface{}) map[string]*ing
 				if serviceUpstream {
 					endpoint, err := ic.getServiceClusterEndpoint(svcKey, &path.Backend)
 					if err != nil {
-						glog.Errorf("Failed to get service cluster endpoint for service %s: %v", svcKey, err)
+						glog.Errorf("failed to get service cluster endpoint for service %s: %v", svcKey, err)
 					} else {
 						upstreams[name].Endpoints = []ingress.Endpoint{endpoint}
 					}

core/pkg/ingress/controller/util.go

@@ -20,6 +20,8 @@ import (
 	"github.com/golang/glog"
 	"github.com/imdario/mergo"
 
+	api "k8s.io/api/core/v1"
+
 	"k8s.io/ingress/core/pkg/ingress"
 )
 
@@ -36,6 +38,7 @@ func newUpstream(name string) *ingress.Backend {
 	return &ingress.Backend{
 		Name:      name,
 		Endpoints: []ingress.Endpoint{},
+		Service:   &api.Service{},
 		SessionAffinity: ingress.SessionAffinityConfig{
 			CookieSessionAffinity: ingress.CookieSessionAffinity{
 				Locations: make(map[string][]string),

core/pkg/ingress/controller/util_test.go

@@ -23,7 +23,6 @@ import (
 	"k8s.io/ingress/core/pkg/ingress"
 	"k8s.io/ingress/core/pkg/ingress/annotations/auth"
 	"k8s.io/ingress/core/pkg/ingress/annotations/authreq"
-	"k8s.io/ingress/core/pkg/ingress/annotations/authtls"
 	"k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist"
 	"k8s.io/ingress/core/pkg/ingress/annotations/proxy"
 	"k8s.io/ingress/core/pkg/ingress/annotations/ratelimit"
@@ -53,7 +52,6 @@ func TestMergeLocationAnnotations(t *testing.T) {
 		"Rewrite":            rewrite.Redirect{},
 		"Whitelist":          ipwhitelist.SourceRange{},
 		"Proxy":              proxy.Configuration{},
-		"CertificateAuth":    authtls.AuthSSLConfig{},
 		"UsePortInRedirects": true,
 	}
 

core/pkg/ingress/types.go

@@ -158,7 +158,7 @@ type Backend struct {
 	Secure bool `json:"secure"`
 	// SecureCACert has the filename and SHA1 of the certificate authorities used to validate
 	// a secured connection to the backend
-	SecureCACert resolver.AuthSSLCert `json:"secureCert"`
+	SecureCACert resolver.AuthSSLCert `json:"secureCACert"`
 	// SSLPassthrough indicates that Ingress controller will delegate TLS termination to the endpoints.
 	SSLPassthrough bool `json:"sslPassthrough"`
 	// Endpoints contains the list of endpoints currently running
@@ -225,6 +225,9 @@ type Server struct {
 	Alias string `json:"alias,omitempty"`
 	// RedirectFromToWWW returns if a redirect to/from prefix www is required
 	RedirectFromToWWW bool `json:"redirectFromToWWW,omitempty"`
+	// CertificateAuth indicates the this server requires mutual authentication
+	// +optional
+	CertificateAuth authtls.AuthSSLConfig `json:"certificateAuth"`
 }
 
 // Location describes an URI inside a server.
@@ -236,7 +239,6 @@ type Server struct {
 // In some cases when more than one annotations is defined a particular order in the execution
 // is required.
 // The chain in the execution order of annotations should be:
-// - CertificateAuth
 // - Whitelist
 // - RateLimit
 // - BasicDigestAuth
@@ -293,10 +295,6 @@ type Location struct {
 	// to be used in connections against endpoints
 	// +optional
 	Proxy proxy.Configuration `json:"proxy,omitempty"`
-	// CertificateAuth indicates the access to this location requires
-	// external authentication
-	// +optional
-	CertificateAuth authtls.AuthSSLConfig `json:"certificateAuth,omitempty"`
 	// UsePortInRedirects indicates if redirects must specify the port
 	// +optional
 	UsePortInRedirects bool `json:"use-port-in-redirects"`

core/pkg/ingress/types_equals.go

@@ -279,6 +279,9 @@ func (s1 *Server) Equal(s2 *Server) bool {
 	if s1.Hostname != s2.Hostname {
 		return false
 	}
+	if s1.Alias != s2.Alias {
+		return false
+	}
 	if s1.SSLPassthrough != s2.SSLPassthrough {
 		return false
 	}
@@ -288,6 +291,12 @@ func (s1 *Server) Equal(s2 *Server) bool {
 	if s1.SSLPemChecksum != s2.SSLPemChecksum {
 		return false
 	}
+	if !(&s1.CertificateAuth).Equal(&s2.CertificateAuth) {
+		return false
+	}
+	if s1.RedirectFromToWWW != s2.RedirectFromToWWW {
+		return false
+	}
 
 	if len(s1.Locations) != len(s2.Locations) {
 		return false
@@ -370,9 +379,6 @@ func (l1 *Location) Equal(l2 *Location) bool {
 	if !(&l1.Proxy).Equal(&l2.Proxy) {
 		return false
 	}
-	if !(&l1.CertificateAuth).Equal(&l2.CertificateAuth) {
-		return false
-	}
 	if l1.UsePortInRedirects != l2.UsePortInRedirects {
 		return false
 	}