Merge pull request #1104 from aledbf/ssl-check

Simplify verification of hostname in ssl certificates
kubernetes · Aug 10, 2017 · d28ea36 · d28ea36
2 parents c67dc0f + c3dd00c
commit d28ea36
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 130 deletions.
diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go
@@ -1109,8 +1109,9 @@ func (ic *GenericController) createServers(data []interface{},
 			}
 
 			cert := bc.(*ingress.SSLCert)
-			if !isHostValid(host, cert) {
-				glog.Warningf("ssl certificate %v does not contain a common name for host %v", key, host)
+			err = cert.Certificate.VerifyHostname(host)
+			if err != nil {
+				glog.Warningf("ssl certificate %v does not contain a Common Name or Subject Alternative Name for host %v", key, host)
 				continue
 			}
 

diff --git a/core/pkg/ingress/controller/util.go b/core/pkg/ingress/controller/util.go
@@ -17,9 +17,6 @@ limitations under the License.
 package controller
 
 import (
-	"strings"
-	"unicode/utf8"
-
 	"github.com/golang/glog"
 	"github.com/imdario/mergo"
 
@@ -47,80 +44,6 @@ func newUpstream(name string) *ingress.Backend {
 	}
 }
 
-func isHostValid(host string, cert *ingress.SSLCert) bool {
-	if cert == nil {
-		return false
-	}
-
-	lowered := toLowerCaseASCII(host)
-	for _, cn := range cert.CN {
-		if matchHostnames(toLowerCaseASCII(cn), lowered) {
-			return true
-		}
-	}
-
-	return false
-}
-
-func matchHostnames(pattern, host string) bool {
-	host = strings.TrimSuffix(host, ".")
-	pattern = strings.TrimSuffix(pattern, ".")
-
-	if len(pattern) == 0 || len(host) == 0 {
-		return false
-	}
-
-	patternParts := strings.Split(pattern, ".")
-	hostParts := strings.Split(host, ".")
-
-	if len(patternParts) != len(hostParts) {
-		return false
-	}
-
-	for i, patternPart := range patternParts {
-		if i == 0 && patternPart == "*" {
-			continue
-		}
-		if patternPart != hostParts[i] {
-			return false
-		}
-	}
-
-	return true
-}
-
-// toLowerCaseASCII returns a lower-case version of in. See RFC 6125 6.4.1. We use
-// an explicitly ASCII function to avoid any sharp corners resulting from
-// performing Unicode operations on DNS labels.
-func toLowerCaseASCII(in string) string {
-	// If the string is already lower-case then there's nothing to do.
-	isAlreadyLowerCase := true
-	for _, c := range in {
-		if c == utf8.RuneError {
-			// If we get a UTF-8 error then there might be
-			// upper-case ASCII bytes in the invalid sequence.
-			isAlreadyLowerCase = false
-			break
-		}
-		if 'A' <= c && c <= 'Z' {
-			isAlreadyLowerCase = false
-			break
-		}
-	}
-
-	if isAlreadyLowerCase {
-		return in
-	}
-
-	out := []byte(in)
-	for i, c := range out {
-		if 'A' <= c && c <= 'Z' {
-			out[i] += 'a' - 'A'
-		}
-	}
-	return string(out)
-}
-
 func mergeLocationAnnotations(loc *ingress.Location, anns map[string]interface{}) {
 	if _, ok := anns[DeniedKeyName]; ok {
 		loc.Denied = anns[DeniedKeyName].(error)

diff --git a/core/pkg/ingress/controller/util_test.go b/core/pkg/ingress/controller/util_test.go
@@ -36,57 +36,6 @@ func (fe *fakeError) Error() string {
 	return "fakeError"
 }
 
-func TestIsHostValid(t *testing.T) {
-	fkCert := &ingress.SSLCert{
-		CAFileName:  "foo",
-		PemFileName: "foo.cr",
-		PemSHA:      "perha",
-		CN: []string{
-			"*.cluster.local", "default.local",
-		},
-	}
-
-	fooTests := []struct {
-		cr   *ingress.SSLCert
-		host string
-		er   bool
-	}{
-		{nil, "foo1.cluster.local", false},
-		{fkCert, "foo1.cluster.local", true},
-		{fkCert, "default.local", true},
-		{fkCert, "foo2.cluster.local.t", false},
-		{fkCert, "", false},
-	}
-
-	for _, foo := range fooTests {
-		r := isHostValid(foo.host, foo.cr)
-		if r != foo.er {
-			t.Errorf("Returned %v but expected %v for foo=%v", r, foo.er, foo)
-		}
-	}
-}
-
-func TestMatchHostnames(t *testing.T) {
-	fooTests := []struct {
-		pattern string
-		host    string
-		er      bool
-	}{
-		{"*.cluster.local.", "foo1.cluster.local.", true},
-		{"foo1.cluster.local.", "foo2.cluster.local.", false},
-		{"cluster.local.", "foo1.cluster.local.", false},
-		{".", "foo1.cluster.local.", false},
-		{"cluster.local.", ".", false},
-	}
-
-	for _, foo := range fooTests {
-		r := matchHostnames(foo.pattern, foo.host)
-		if r != foo.er {
-			t.Errorf("Returned %v but expected %v for foo=%v", r, foo.er, foo)
-		}
-	}
-}
-
 func TestMergeLocationAnnotations(t *testing.T) {
 	// initial parameters
 	loc := ingress.Location{}

diff --git a/core/pkg/ingress/sort_ingress.go b/core/pkg/ingress/sort_ingress.go
@@ -17,6 +17,7 @@ limitations under the License.
 package ingress
 
 import (
+	"crypto/x509"
 	"time"
 
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -72,6 +73,7 @@ func (c LocationByPath) Less(i, j int) bool {
 // SSLCert describes a SSL certificate to be used in a server
 type SSLCert struct {
 	meta_v1.ObjectMeta `json:"metadata,omitempty"`
+	Certificate        *x509.Certificate `json:"certificate,omitempty"`
 	// CAFileName contains the path to the file with the root certificate
 	CAFileName string `json:"caFileName"`
 	// PemFileName contains the path to the file with the certificate and key concatenated

diff --git a/core/pkg/net/ssl/ssl.go b/core/pkg/net/ssl/ssl.go
@@ -160,6 +160,7 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte) (*ingress.SSLCert,
 		caFile.Write([]byte("\n"))
 
 		return &ingress.SSLCert{
+			Certificate: pemCert,
 			CAFileName:  pemFileName,
 			PemFileName: pemFileName,
 			PemSHA:      file.SHA1(pemFileName),
@@ -169,6 +170,7 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte) (*ingress.SSLCert,
 	}
 
 	return &ingress.SSLCert{
+		Certificate: pemCert,
 		PemFileName: pemFileName,
 		PemSHA:      file.SHA1(pemFileName),
 		CN:          cn.List(),