Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify handling of ssl certificates #956

Merged
merged 1 commit into from
Jul 13, 2017
Merged

Simplify handling of ssl certificates #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/pkg/ingress/controller/backend_ssl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ func (ic *GenericController) syncSecret() {
}
glog.Infof("updating secret %v in the local store", key)
ic.sslCertTracker.Update(key, cert)
ic.reloadRequired = true
continue
}

glog.Infof("adding secret %v to the local store", key)
ic.sslCertTracker.Add(key, cert)
ic.reloadRequired = true
}
}

Expand All @@ -86,8 +88,8 @@ func (ic *GenericController) getPemCertificate(secretName string) (*ingress.SSLC

var s *ingress.SSLCert
if okcert && okkey {
glog.V(3).Infof("found certificate and private key, configuring %v as a TLS Secret", secretName)
s, err = ssl.AddOrUpdateCertAndKey(nsSecName, cert, key, ca)
glog.V(3).Infof("found certificate and private key, configuring %v as a TLS Secret (CN: %v)", secretName, s.CN)
} else if ca != nil {
glog.V(3).Infof("found only ca.crt, configuring %v as an Certificate Authentication secret", secretName)
s, err = ssl.AddCertAuth(nsSecName, ca)
Expand Down
94 changes: 46 additions & 48 deletions core/pkg/ingress/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/flowcontrol"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress/core/pkg/ingress"
"k8s.io/ingress/core/pkg/ingress/annotations/class"
"k8s.io/ingress/core/pkg/ingress/annotations/healthcheck"
Expand Down Expand Up @@ -113,8 +114,8 @@ type GenericController struct {
// runningConfig contains the running configuration in the Backend
runningConfig *ingress.Configuration

// configmapChanged indicates the configmap
configmapChanged bool
// reloadRequired indicates the configmap
reloadRequired bool
}

// Configuration contains all the settings required by an Ingress controller
Expand Down Expand Up @@ -262,7 +263,7 @@ func newIngressController(config *Configuration) *GenericController {
if mapKey == ic.cfg.ConfigMapName {
glog.V(2).Infof("adding configmap %v to backend", mapKey)
ic.cfg.Backend.SetConfig(upCmap)
ic.configmapChanged = true
ic.reloadRequired = true
}
},
UpdateFunc: func(old, cur interface{}) {
Expand All @@ -272,7 +273,7 @@ func newIngressController(config *Configuration) *GenericController {
if mapKey == ic.cfg.ConfigMapName {
glog.V(2).Infof("updating configmap backend (%v)", mapKey)
ic.cfg.Backend.SetConfig(upCmap)
ic.configmapChanged = true
ic.reloadRequired = true
}
// updates to configuration configmaps can trigger an update
if mapKey == ic.cfg.ConfigMapName || mapKey == ic.cfg.TCPConfigMapName || mapKey == ic.cfg.UDPConfigMapName {
Expand Down Expand Up @@ -419,7 +420,7 @@ func (ic *GenericController) syncIngress(key interface{}) error {
PassthroughBackends: passUpstreams,
}

if !ic.configmapChanged && (ic.runningConfig != nil && ic.runningConfig.Equal(&pcfg)) {
if !ic.reloadRequired && (ic.runningConfig != nil && ic.runningConfig.Equal(&pcfg)) {
glog.V(3).Infof("skipping backend reload (no changes detected)")
return nil
}
Expand All @@ -433,7 +434,7 @@ func (ic *GenericController) syncIngress(key interface{}) error {
return err
}

ic.configmapChanged = false
ic.reloadRequired = false
glog.Infof("ingress backend successfully reloaded...")
incReloadCount()
setSSLExpireTime(servers)
Expand Down Expand Up @@ -1018,55 +1019,52 @@ func (ic *GenericController) createServers(data []interface{},
}

// only add a certificate if the server does not have one previously configured
// TODO: TLS without secret?
if len(ing.Spec.TLS) > 0 && servers[host].SSLCertificate == "" {
tlsSecretName := ""
found := false
for _, tls := range ing.Spec.TLS {
for _, tlsHost := range tls.Hosts {
if tlsHost == host {
tlsSecretName = tls.SecretName
found = true
break
}
}
}
if len(ing.Spec.TLS) == 0 || servers[host].SSLCertificate != "" {
continue
}

// the current ing.Spec.Rules[].Host doesn't have an entry at
// ing.Spec.TLS[].Hosts[], skipping to the next Rule
if !found {
continue
tlsSecretName := ""
found := false
for _, tls := range ing.Spec.TLS {
if sets.NewString(tls.Hosts...).Has(host) {
tlsSecretName = tls.SecretName
found = true
break
}
}

// Current Host listed on ing.Spec.TLS[].Hosts[]
// but TLS[].SecretName is empty; using default cert
if tlsSecretName == "" {
servers[host].SSLCertificate = defaultPemFileName
servers[host].SSLPemChecksum = defaultPemSHA
continue
}
// the current ing.Spec.Rules[].Host doesn't have an entry at
// ing.Spec.TLS[].Hosts[] skipping to the next Rule
if !found {
continue
}

key := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName)
bc, exists := ic.sslCertTracker.Get(key)
if exists {
cert := bc.(*ingress.SSLCert)
if isHostValid(host, cert) {
servers[host].SSLCertificate = cert.PemFileName
servers[host].SSLPemChecksum = cert.PemSHA
servers[host].SSLExpireTime = cert.ExpireTime

if cert.ExpireTime.Before(time.Now().Add(240 * time.Hour)) {
glog.Warningf("ssl certificate for host %v is about to expire in 10 days", host)
}
if tlsSecretName == "" {
glog.Warningf("host %v is listed on tls section but secretName is empty. Using default cert", host)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aledbf I do use wildcard domains and a wildcard certificate as the default cert. Using host without a secret is a feature on my deployment. Could this log be changed to V(3).Info?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, there's already an open issue to increase the log leve

servers[host].SSLCertificate = defaultPemFileName
servers[host].SSLPemChecksum = defaultPemSHA
continue
}

} else {
glog.Warningf("ssl certificate %v does not contain a common name for host %v", key, host)
}
key := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName)
bc, exists := ic.sslCertTracker.Get(key)
if !exists {
glog.Infof("ssl certificate \"%v\" does not exist in local store", key)
continue
}

continue
}
cert := bc.(*ingress.SSLCert)
if !isHostValid(host, cert) {
glog.Warningf("ssl certificate %v does not contain a common name for host %v", key, host)
continue
}

glog.Infof("ssl certificate \"%v\" does not exist in local store", key)
servers[host].SSLCertificate = cert.PemFileName
servers[host].SSLPemChecksum = cert.PemSHA
servers[host].SSLExpireTime = cert.ExpireTime

if cert.ExpireTime.Before(time.Now().Add(240 * time.Hour)) {
glog.Warningf("ssl certificate for host %v is about to expire in 10 days", host)
}
}
}
Expand Down
21 changes: 15 additions & 6 deletions core/pkg/net/ssl/ssl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (

"github.com/golang/glog"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress/core/pkg/ingress"
)

Expand Down Expand Up @@ -104,19 +105,27 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte) (*ingress.SSLCert,
return nil, err
}

cn := []string{pemCert.Subject.CommonName}
if len(pemCert.DNSNames) > 0 {
cn = append(cn, pemCert.DNSNames...)
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
}
cn = append(cn, dns...)

for _, dns := range dns {
if !cn.Has(dns) {
cn.Insert(dns)
}
}
}
}

Expand Down Expand Up @@ -155,15 +164,15 @@ func AddOrUpdateCertAndKey(name string, cert, key, ca []byte) (*ingress.SSLCert,
CAFileName: pemFileName,
PemFileName: pemFileName,
PemSHA: PemSHA1(pemFileName),
CN: cn,
CN: cn.List(),
ExpireTime: pemCert.NotAfter,
}, nil
}

return &ingress.SSLCert{
PemFileName: pemFileName,
PemSHA: PemSHA1(pemFileName),
CN: cn,
CN: cn.List(),
ExpireTime: pemCert.NotAfter,
}, nil
}
Expand Down