Skip to content

Commit

Permalink
Change naming of SSL certs
Browse files Browse the repository at this point in the history
  • Loading branch information
Nick Sardo committed Apr 11, 2018
1 parent 8b08182 commit 9a0fc14
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 78 deletions.
23 changes: 12 additions & 11 deletions pkg/loadbalancers/l7.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"

"crypto/sha256"

"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/backends"
"k8s.io/ingress-gce/pkg/utils"
Expand Down Expand Up @@ -109,8 +110,6 @@ type L7 struct {
fws *compute.ForwardingRule
// ip is the static-ip associated with both GlobalForwardingRules.
ip *compute.Address
// prefix to use in ssl cert names
sslCertPrefix string
// sslCerts is the list of ssl certs associated with the targetHTTPSProxy.
sslCerts []*compute.SslCertificate
// oldSSLCerts is the list of certs that used to be hooked up to the
Expand Down Expand Up @@ -197,7 +196,7 @@ func (l *L7) deleteOldSSLCerts() (err error) {
}
certsMap := getMapfromCertList(l.sslCerts)
for _, cert := range l.oldSSLCerts {
if !l.isSSLCert(cert.Name) && !l.namer.IsLegacySSLCert(l.Name, cert.Name) {
if !l.namer.IsCertUsedForLB(l.Name, cert.Name) && !l.namer.IsLegacySSLCert(l.Name, cert.Name) {
// retain cert if it is managed by GCE(non-ingress)
continue
}
Expand Down Expand Up @@ -261,11 +260,6 @@ func (l *L7) usePreSharedCert() (bool, error) {
return true, nil
}

// isSSLCert returns true if name is ingress managed, specifically by this loadbalancer instance
func (l *L7) isSSLCert(name string) bool {
return strings.HasPrefix(name, l.sslCertPrefix)
}

func getMapfromCertList(certs []*compute.SslCertificate) map[string]*compute.SslCertificate {
if len(certs) == 0 {
return nil
Expand All @@ -287,7 +281,7 @@ func (l *L7) populateSSLCert() error {
return utils.IgnoreHTTPNotFound(err)
}
for _, c := range certs {
if l.isSSLCert(c.Name) {
if l.namer.IsCertUsedForLB(l.Name, c.Name) {
glog.Infof("Populating ssl cert %s for l7 %s", c.Name, l.Name)
l.sslCerts = append(l.sslCerts, c)
}
Expand Down Expand Up @@ -331,7 +325,7 @@ func (l *L7) checkSSLCert() error {
for _, tlsCert := range l.runtimeInfo.TLS {
ingCert := tlsCert.Cert
ingKey := tlsCert.Key
newCertName := l.namer.SSLCertName(l.sslCertPrefix, tlsCert.CertHash)
newCertName := l.namer.SSLCertName(l.Name, tlsCert.CertHash)

// PrivateKey is write only, so compare certs alone. We're assuming that
// no one will change just the key. We can remember the key and compare,
Expand Down Expand Up @@ -423,7 +417,7 @@ func (l *L7) checkHttpsProxy() (err error) {

if !l.compareCerts(proxy.SslCertificates) {
glog.Infof("Https proxy %v has the wrong ssl certs, setting %v overwriting %v",
proxy.Name, l.sslCerts, proxy.SslCertificates)
proxy.Name, toCertNames(l.sslCerts), proxy.SslCertificates)
if err := l.cloud.SetSslCertificateForTargetHttpsProxy(proxy, l.sslCerts); err != nil {
return err
}
Expand Down Expand Up @@ -989,3 +983,10 @@ func GCEResourceName(ingAnnotations map[string]string, resourceName string) stri
func GetCertHash(contents string) string {
return fmt.Sprintf("%x", sha256.Sum256([]byte(contents)))[:16]
}

func toCertNames(certs []*compute.SslCertificate) (names []string) {
for _, v := range certs {
names = append(names, v.Name)
}
return names
}
1 change: 0 additions & 1 deletion pkg/loadbalancers/l7s.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ func (l *L7s) create(ri *L7RuntimeInfo) (*L7, error) {
cloud: l.cloud,
glbcDefaultBackend: l.glbcDefaultBackend,
namer: l.namer,
sslCertPrefix: l.namer.SSLCertPrefix(ri.Name),
}, nil
}

Expand Down
68 changes: 37 additions & 31 deletions pkg/loadbalancers/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ import (
compute "google.golang.org/api/compute/v1"
"k8s.io/apimachinery/pkg/util/sets"

"strconv"
"strings"

"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/backends"
"k8s.io/ingress-gce/pkg/healthchecks"
"k8s.io/ingress-gce/pkg/instances"
"k8s.io/ingress-gce/pkg/neg"
"k8s.io/ingress-gce/pkg/utils"
"strconv"
"strings"
)

const (
Expand Down Expand Up @@ -65,11 +66,13 @@ func TestCreateHTTPLoadBalancer(t *testing.T) {
}
f := NewFakeLoadBalancers(lbInfo.Name, namer)
pool := newFakeLoadBalancerPool(f, t, namer)

// Run Sync
pool.Sync([]*L7RuntimeInfo{lbInfo})
l7, err := pool.Get(lbInfo.Name)

l7, err := pool.Get(lbInfo.Name)
if err != nil || l7 == nil {
t.Fatalf("Expected l7 not created")
t.Fatalf("Expected l7 not created, err: %v", err)
}
um, err := f.GetUrlMap(f.UMName())
if err != nil {
Expand Down Expand Up @@ -129,12 +132,12 @@ func TestCreateHTTPSLoadBalancer(t *testing.T) {
// and the proxy is updated to another cert when the provided cert changes
func TestCertUpdate(t *testing.T) {
namer := utils.NewNamer("uid1", "fw1")
sslCertPrefix := namer.SSLCertPrefix("test")
certName1 := namer.SSLCertName(sslCertPrefix, GetCertHash("cert"))
certName2 := namer.SSLCertName(sslCertPrefix, GetCertHash("cert2"))
lbName := namer.LoadBalancer("test")
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
TLS: []*TLSCerts{createCert("key", "cert", "name")},
}
Expand All @@ -144,7 +147,9 @@ func TestCertUpdate(t *testing.T) {

// Sync first cert
pool.Sync([]*L7RuntimeInfo{lbInfo})
t.Logf("name=%q", certName1)

// Verify certs
t.Logf("lbName=%q, name=%q", lbName, certName1)
expectCerts := map[string]string{certName1: lbInfo.TLS[0].Cert}
verifyCertAndProxyLink(expectCerts, expectCerts, f, t)

Expand All @@ -158,12 +163,12 @@ func TestCertUpdate(t *testing.T) {
// Tests that controller can overwrite existing, unused certificates
func TestCertCreationWithCollision(t *testing.T) {
namer := utils.NewNamer("uid1", "fw1")
sslCertPrefix := namer.SSLCertPrefix("test")
certName1 := namer.SSLCertName(sslCertPrefix, GetCertHash("cert"))
certName2 := namer.SSLCertName(sslCertPrefix, GetCertHash("cert2"))
lbName := namer.LoadBalancer("test")
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
TLS: []*TLSCerts{createCert("key", "cert", "name")},
}
Expand Down Expand Up @@ -202,18 +207,18 @@ func TestCertCreationWithCollision(t *testing.T) {

func TestMultipleCertRetentionAfterRestart(t *testing.T) {
namer := utils.NewNamer("uid1", "fw1")
sslCertPrefix := namer.SSLCertPrefix("test")
cert1 := createCert("key", "cert", "name")
cert2 := createCert("key2", "cert2", "name2")
cert3 := createCert("key3", "cert3", "name3")
certName1 := namer.SSLCertName(sslCertPrefix, cert1.CertHash)
certName2 := namer.SSLCertName(sslCertPrefix, cert2.CertHash)
certName3 := namer.SSLCertName(sslCertPrefix, cert3.CertHash)
lbName := namer.LoadBalancer("test")
certName1 := namer.SSLCertName(lbName, cert1.CertHash)
certName2 := namer.SSLCertName(lbName, cert2.CertHash)
certName3 := namer.SSLCertName(lbName, cert3.CertHash)

expectCerts := map[string]string{}

lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
TLS: []*TLSCerts{cert1},
}
Expand Down Expand Up @@ -249,14 +254,15 @@ func TestMultipleCertRetentionAfterRestart(t *testing.T) {
// are picked up and deleted when upgrading to the new scheme.
func TestUpgradeToNewCertNames(t *testing.T) {
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
}
oldCertName := "k8s-ssl-" + lbInfo.Name
tlsCert := createCert("key", "cert", "name")
lbInfo.TLS = []*TLSCerts{tlsCert}
newCertName := namer.SSLCertName(namer.SSLCertPrefix("test"), tlsCert.CertHash)
newCertName := namer.SSLCertName(lbName, tlsCert.CertHash)
f := NewFakeLoadBalancers(lbInfo.Name, namer)
pool := newFakeLoadBalancerPool(f, t, namer)

Expand Down Expand Up @@ -292,16 +298,16 @@ func TestMaxCertsUpload(t *testing.T) {
var tlsCerts []*TLSCerts
expectCerts := make(map[string]string)
namer := utils.NewNamer("uid1", "fw1")
certPrefix := namer.SSLCertPrefix("test")
lbName := namer.LoadBalancer("test")

for ix := 0; ix < TargetProxyCertLimit; ix++ {
str := strconv.Itoa(ix)
tlsCerts = append(tlsCerts, createCert("key-"+str, "cert-"+str, "name-"+str))
certName := namer.SSLCertName(certPrefix, GetCertHash("cert-"+str))
certName := namer.SSLCertName(lbName, GetCertHash("cert-"+str))
expectCerts[certName] = "cert-" + str
}
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
TLS: tlsCerts,
}
Expand All @@ -322,18 +328,18 @@ func TestIdenticalHostnameCerts(t *testing.T) {
var tlsCerts []*TLSCerts
expectCerts := make(map[string]string)
namer := utils.NewNamer("uid1", "fw1")
certPrefix := namer.SSLCertPrefix("test")
lbName := namer.LoadBalancer("test")
contents := ""

for ix := 0; ix < 3; ix++ {
str := strconv.Itoa(ix)
contents = "cert-" + str + " foo.com"
tlsCerts = append(tlsCerts, createCert("key-"+str, contents, "name-"+str))
certName := namer.SSLCertName(certPrefix, GetCertHash(contents))
certName := namer.SSLCertName(lbName, GetCertHash(contents))
expectCerts[certName] = contents
}
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
TLS: tlsCerts,
}
Expand Down Expand Up @@ -391,12 +397,12 @@ func TestIdenticalHostnameCertsPreShared(t *testing.T) {
// to secret based cert and verifies the pre-shared cert is retained.
func TestPreSharedToSecretBasedCertUpdate(t *testing.T) {
namer := utils.NewNamer("uid1", "fw1")
sslCertPrefix := namer.SSLCertPrefix("test")
certName1 := namer.SSLCertName(sslCertPrefix, GetCertHash("cert"))
certName2 := namer.SSLCertName(sslCertPrefix, GetCertHash("cert2"))
lbName := namer.LoadBalancer("test")
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: lbName,
AllowHTTP: false,
}

Expand Down Expand Up @@ -500,7 +506,7 @@ func verifyCertAndProxyLink(expectCerts map[string]string, expectCertsProxy map[
for certName, certValue := range expectCerts {
cert, err := f.GetSslCertificate(certName)
if err != nil {
t.Fatalf("expected ssl certificate to exist: %v, err: %v", certName, err)
t.Fatalf("expected ssl certificate to exist: %v, err: %v, all certs: %v", certName, err, toCertNames(allCerts))
}

if cert.Certificate != certValue {
Expand Down
45 changes: 22 additions & 23 deletions pkg/utils/namer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"sync"

"crypto/sha256"

"github.com/golang/glog"
)

Expand Down Expand Up @@ -302,35 +303,33 @@ func (n *Namer) TargetProxy(lbName string, protocol NamerProtocol) string {
return "invalid"
}

// IsLegacySSLCert returns true if certName is an Ingress managed name following the older naming convention. The check
// also ensures that the cert is managed by the specific ingress instance - lbName
func (n *Namer) IsLegacySSLCert(lbName string, name string) bool {
// old style name is of the form k8s-ssl-<lbname> or k8s-ssl-1-<lbName>.
legacyPrefixPrimary := truncate(strings.Join([]string{n.prefix, sslCertPrefix, lbName}, "-"))
legacyPrefixSec := truncate(strings.Join([]string{n.prefix, sslCertPrefix, "1", lbName}, "-"))
return strings.HasPrefix(name, legacyPrefixPrimary) || strings.HasPrefix(name, legacyPrefixSec)
// IsCertUsedForLB returns true if the resourceName belongs to this cluster's ingress.
// It checks that the hashed lbName exists and
func (n *Namer) IsCertUsedForLB(lbName, resourceName string) bool {
lbNameHash := n.lbNameToHash(lbName)
prefix := fmt.Sprintf("%s-%s-%s", n.prefix, sslCertPrefix, lbNameHash)
return strings.HasPrefix(resourceName, prefix) && strings.HasSuffix(resourceName, n.UID())
}

func (n *Namer) SSLCertPrefix(lbKey string) string {
// lbKey is of the form namespace/ingressname. Cert prefix will use the 8 byte(16 chars) hash of namespace-ingressname
// followed by the cluster id(also 16 char). We use hash instead of the actual name so that the cert prefix is of a
// fixed length and at the same time unique for a given loadbalancer instance.
func (n *Namer) lbNameToHash(lbName string) string {
ingHash := fmt.Sprintf("%x", sha256.Sum256([]byte(lbName)))
return ingHash[:16]
}

parts := strings.Split(lbKey, clusterNameDelimiter)
scrubbedName := strings.Replace(lbKey, "/", "-", -1)
clusterName := n.UID()
if parts[len(parts)-1] == clusterName {
scrubbedName = strings.TrimSuffix(scrubbedName, clusterNameDelimiter+clusterName)
}
// sha256 is 32 bytes(64 chars) long, truncating to first 8 bytes still results in low probability of collision
namespaceHash := fmt.Sprintf("%x", sha256.Sum256([]byte(scrubbedName)))
clusterPrefix := namespaceHash[:16] + clusterNameDelimiter + clusterName
return fmt.Sprintf("%s-%s-%s", n.prefix, sslCertPrefix, clusterPrefix)
// IsLegacySSLCert returns true if certName is an Ingress managed name following the older naming convention. The check
// also ensures that the cert is managed by the specific ingress instance - lbName
func (n *Namer) IsLegacySSLCert(lbName string, resourceName string) bool {
// old style name is of the form k8s-ssl-<lbname> or k8s-ssl-1-<lbName>.
legacyPrefixPrimary := truncate(fmt.Sprintf("%s-%s-%s", n.prefix, sslCertPrefix, lbName))
legacyPrefixSec := truncate(fmt.Sprintf("%s-%s-1-%s", n.prefix, sslCertPrefix, lbName))
return strings.HasPrefix(resourceName, legacyPrefixPrimary) || strings.HasPrefix(resourceName, legacyPrefixSec)
}

// SSLCertName returns the name of the certificate.
func (n *Namer) SSLCertName(prefix string, secretHash string) string {
return truncate(prefix + "-" + secretHash)
func (n *Namer) SSLCertName(lbName string, secretHash string) string {
lbNameHash := n.lbNameToHash(lbName)
// k8s-ssl-[lbNameHash]-[certhash]--[clusterUID]
return truncate(fmt.Sprintf("%s-%s-%s-%s%s%s", n.prefix, sslCertPrefix, lbNameHash, secretHash, clusterNameDelimiter, n.UID()))
}

// ForwardingRule returns the name of the forwarding rule prefix.
Expand Down
Loading

0 comments on commit 9a0fc14

Please sign in to comment.