From 8b5a6e7661819452d024b1643f6cd150fcde01c1 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Sun, 14 May 2017 19:14:27 -0300 Subject: [PATCH] Add secure-verify-ca-secret annotation --- .../annotations/secureupstream/main.go | 44 +++++++++++++++-- .../annotations/secureupstream/main_test.go | 48 +++++++++++++++++-- core/pkg/ingress/controller/annotations.go | 12 +++-- .../ingress/controller/annotations_test.go | 47 +++++++++++++++++- core/pkg/ingress/controller/controller.go | 5 +- core/pkg/ingress/types.go | 5 +- 6 files changed, 144 insertions(+), 17 deletions(-) diff --git a/core/pkg/ingress/annotations/secureupstream/main.go b/core/pkg/ingress/annotations/secureupstream/main.go index 331b44b96..d07f92994 100644 --- a/core/pkg/ingress/annotations/secureupstream/main.go +++ b/core/pkg/ingress/annotations/secureupstream/main.go @@ -17,25 +17,61 @@ limitations under the License. package secureupstream import ( + "fmt" + "github.com/pkg/errors" extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1" "k8s.io/ingress/core/pkg/ingress/annotations/parser" + "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( - secureUpstream = "ingress.kubernetes.io/secure-backends" + secureUpstream = "ingress.kubernetes.io/secure-backends" + secureVerifyCASecret = "ingress.kubernetes.io/secure-verify-ca-secret" ) +// Secure describes SSL backend configuration +type Secure struct { + Secure bool + CACert resolver.AuthSSLCert +} + type su struct { + certResolver resolver.AuthCertificate } // NewParser creates a new secure upstream annotation parser -func NewParser() parser.IngressAnnotation { - return su{} +func NewParser(resolver resolver.AuthCertificate) parser.IngressAnnotation { + return su{ + certResolver: resolver, + } } // Parse parses the annotations contained in the ingress // rule used to indicate if the upstream servers should use SSL func (a su) Parse(ing *extensions.Ingress) (interface{}, error) { - return parser.GetBoolAnnotation(secureUpstream, ing) + s, _ := parser.GetBoolAnnotation(secureUpstream, ing) + ca, _ := parser.GetStringAnnotation(secureVerifyCASecret, ing) + secure := &Secure{ + Secure: s, + CACert: resolver.AuthSSLCert{}, + } + if !s && ca != "" { + return secure, + errors.Errorf("trying to use CA from secret %v/%v on a non secure backend", ing.Namespace, ca) + } + if ca == "" { + return secure, nil + } + caCert, err := a.certResolver.GetAuthCertificate(fmt.Sprintf("%v/%v", ing.Namespace, ca)) + if err != nil { + return secure, errors.Wrap(err, "error obtaining certificate") + } + if caCert == nil { + return secure, nil + } + return &Secure{ + Secure: s, + CACert: *caCert, + }, nil } diff --git a/core/pkg/ingress/annotations/secureupstream/main_test.go b/core/pkg/ingress/annotations/secureupstream/main_test.go index 388cc8ddc..baa0af715 100644 --- a/core/pkg/ingress/annotations/secureupstream/main_test.go +++ b/core/pkg/ingress/annotations/secureupstream/main_test.go @@ -23,7 +23,9 @@ import ( api "k8s.io/client-go/pkg/api/v1" extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1" + "fmt" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress/core/pkg/ingress/resolver" ) func buildIngress() *extensions.Ingress { @@ -61,22 +63,58 @@ func buildIngress() *extensions.Ingress { } } +type mockCfg struct { + certs map[string]resolver.AuthSSLCert +} + +func (cfg mockCfg) GetAuthCertificate(secret string) (*resolver.AuthSSLCert, error) { + if cert, ok := cfg.certs[secret]; ok { + return &cert, nil + } + return nil, fmt.Errorf("secret not found: %v", secret) +} + func TestAnnotations(t *testing.T) { ing := buildIngress() data := map[string]string{} data[secureUpstream] = "true" + data[secureVerifyCASecret] = "secure-verify-ca" ing.SetAnnotations(data) - _, err := NewParser().Parse(ing) + _, err := NewParser(mockCfg{ + certs: map[string]resolver.AuthSSLCert{ + "default/secure-verify-ca": resolver.AuthSSLCert{}, + }, + }).Parse(ing) if err != nil { - t.Error("Expected error with ingress without annotations") + t.Errorf("Unexpected error on ingress: %v", err) } } -func TestWithoutAnnotations(t *testing.T) { +func TestSecretNotFound(t *testing.T) { ing := buildIngress() - _, err := NewParser().Parse(ing) + data := map[string]string{} + data[secureUpstream] = "true" + data[secureVerifyCASecret] = "secure-verify-ca" + ing.SetAnnotations(data) + _, err := NewParser(mockCfg{}).Parse(ing) if err == nil { - t.Error("Expected error with ingress without annotations") + t.Error("Expected secret not found error on ingress") + } +} + +func TestSecretOnNonSecure(t *testing.T) { + ing := buildIngress() + data := map[string]string{} + data[secureUpstream] = "false" + data[secureVerifyCASecret] = "secure-verify-ca" + ing.SetAnnotations(data) + _, err := NewParser(mockCfg{ + certs: map[string]resolver.AuthSSLCert{ + "default/secure-verify-ca": resolver.AuthSSLCert{}, + }, + }).Parse(ing) + if err == nil { + t.Error("Expected CA secret on non secure backend error on ingress") } } diff --git a/core/pkg/ingress/controller/annotations.go b/core/pkg/ingress/controller/annotations.go index fb41f7bfa..601269ee6 100644 --- a/core/pkg/ingress/controller/annotations.go +++ b/core/pkg/ingress/controller/annotations.go @@ -68,7 +68,7 @@ func newAnnotationExtractor(cfg extractorConfig) annotationExtractor { "Proxy": proxy.NewParser(cfg), "RateLimit": ratelimit.NewParser(), "Redirect": rewrite.NewParser(cfg), - "SecureUpstream": secureupstream.NewParser(), + "SecureUpstream": secureupstream.NewParser(cfg), "SessionAffinity": sessionaffinity.NewParser(), "SSLPassthrough": sslpassthrough.NewParser(), "ConfigurationSnippet": snippet.NewParser(), @@ -111,9 +111,13 @@ const ( sessionAffinity = "SessionAffinity" ) -func (e *annotationExtractor) SecureUpstream(ing *extensions.Ingress) bool { - val, _ := e.annotations[secureUpstream].Parse(ing) - return val.(bool) +func (e *annotationExtractor) SecureUpstream(ing *extensions.Ingress) *secureupstream.Secure { + val, err := e.annotations[secureUpstream].Parse(ing) + if err != nil { + glog.Errorf("error parsing secure upstream: %v", err) + } + secure := val.(*secureupstream.Secure) + return secure } func (e *annotationExtractor) HealthCheck(ing *extensions.Ingress) *healthcheck.Upstream { diff --git a/core/pkg/ingress/controller/annotations_test.go b/core/pkg/ingress/controller/annotations_test.go index 7b612a7c6..51b91831b 100644 --- a/core/pkg/ingress/controller/annotations_test.go +++ b/core/pkg/ingress/controller/annotations_test.go @@ -30,6 +30,7 @@ import ( const ( annotationSecureUpstream = "ingress.kubernetes.io/secure-backends" + annotationSecureVerifyCACert = "ingress.kubernetes.io/secure-verify-ca-secret" annotationUpsMaxFails = "ingress.kubernetes.io/upstream-max-fails" annotationUpsFailTimeout = "ingress.kubernetes.io/upstream-fail-timeout" annotationPassthrough = "ingress.kubernetes.io/ssl-passthrough" @@ -51,7 +52,14 @@ func (m mockCfg) GetSecret(name string) (*api.Secret, error) { return m.MockSecrets[name], nil } -func (m mockCfg) GetAuthCertificate(string) (*resolver.AuthSSLCert, error) { +func (m mockCfg) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) { + if secret, _ := m.GetSecret(name); secret != nil { + return &resolver.AuthSSLCert{ + Secret: name, + CAFileName: "/opt/ca.pem", + PemSHA: "123", + }, nil + } return nil, nil } @@ -122,12 +130,47 @@ func TestSecureUpstream(t *testing.T) { for _, foo := range fooAnns { ing.SetAnnotations(foo.annotations) r := ec.SecureUpstream(ing) - if r != foo.er { + if r.Secure != foo.er { t.Errorf("Returned %v but expected %v", r, foo.er) } } } +func TestSecureVerifyCACert(t *testing.T) { + ec := newAnnotationExtractor(mockCfg{ + MockSecrets: map[string]*api.Secret{ + "default/secure-verify-ca": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "secure-verify-ca", + }, + }, + }, + }) + + anns := []struct { + it int + annotations map[string]string + exists bool + }{ + {1, map[string]string{annotationSecureUpstream: "true", annotationSecureVerifyCACert: "not"}, false}, + {2, map[string]string{annotationSecureUpstream: "false", annotationSecureVerifyCACert: "secure-verify-ca"}, false}, + {3, map[string]string{annotationSecureUpstream: "true", annotationSecureVerifyCACert: "secure-verify-ca"}, true}, + {4, map[string]string{annotationSecureUpstream: "true", annotationSecureVerifyCACert + "_not": "secure-verify-ca"}, false}, + {5, map[string]string{annotationSecureUpstream: "true"}, false}, + {6, map[string]string{}, false}, + {7, nil, false}, + } + + for _, ann := range anns { + ing := buildIngress() + ing.SetAnnotations(ann.annotations) + res := ec.SecureUpstream(ing) + if (res.CACert.CAFileName != "") != ann.exists { + t.Errorf("Expected exists was %v on iteration %v", ann.exists, ann.it) + } + } +} + func TestHealthCheck(t *testing.T) { ec := newAnnotationExtractor(mockCfg{}) ing := buildIngress() diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 5f65ddc55..d1c6fb219 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -783,7 +783,10 @@ func (ic *GenericController) createUpstreams(data []interface{}) map[string]*ing glog.V(3).Infof("creating upstream %v", name) upstreams[name] = newUpstream(name) if !upstreams[name].Secure { - upstreams[name].Secure = secUpstream + upstreams[name].Secure = secUpstream.Secure + } + if upstreams[name].SecureCACert.Secret == "" { + upstreams[name].SecureCACert = secUpstream.CACert } if upstreams[name].SessionAffinity.AffinityType == "" { upstreams[name].SessionAffinity.AffinityType = affinity.AffinityType diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index ec56817d5..4d5a3869f 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -31,6 +31,7 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" "k8s.io/ingress/core/pkg/ingress/defaults" + "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/ingress/core/pkg/ingress/store" ) @@ -154,8 +155,10 @@ type Backend struct { // Allowing the use of HTTPS // The endpoint/s must provide a TLS connection. // The certificate used in the endpoint cannot be a self signed certificate - // TODO: add annotation to allow the load of ca certificate 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"` // SSLPassthrough indicates that Ingress controller will delegate TLS termination to the endpoints. SSLPassthrough bool `json:"sslPassthrough"` // Endpoints contains the list of endpoints currently running