From 2d946d7ee73466fb06fd9e272b2dedfeda0bf6c9 Mon Sep 17 00:00:00 2001 From: NicoMen Date: Fri, 25 May 2018 19:36:04 +0200 Subject: [PATCH] Remove ACME empty certificates from KV store --- acme/account.go | 12 +++ acme/acme_test.go | 266 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 278 insertions(+) diff --git a/acme/account.go b/acme/account.go index e32e68948..16db20d0d 100644 --- a/acme/account.go +++ b/acme/account.go @@ -152,11 +152,23 @@ func (dc *DomainsCertificates) removeDuplicates() { } } +func (dc *DomainsCertificates) removeEmpty() { + certs := []*DomainsCertificate{} + for _, cert := range dc.Certs { + if cert.Certificate != nil && len(cert.Certificate.Certificate) > 0 && len(cert.Certificate.PrivateKey) > 0 { + certs = append(certs, cert) + } + } + dc.Certs = certs +} + // Init DomainsCertificates func (dc *DomainsCertificates) Init() error { dc.lock.Lock() defer dc.lock.Unlock() + dc.removeEmpty() + for _, domainsCertificate := range dc.Certs { tlsCert, err := tls.X509KeyPair(domainsCertificate.Certificate.Certificate, domainsCertificate.Certificate.PrivateKey) if err != nil { diff --git a/acme/acme_test.go b/acme/acme_test.go index 8c7c78280..72201dd34 100644 --- a/acme/acme_test.go +++ b/acme/acme_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "sort" "sync" "testing" "time" @@ -550,3 +551,268 @@ func TestAcme_getCertificateForDomain(t *testing.T) { }) } } + +func TestRemoveEmptyCertificates(t *testing.T) { + now := time.Now() + fooCert, fooKey, _ := generate.KeyPair("foo.com", now) + acmeCert, acmeKey, _ := generate.KeyPair("acme.wtf", now.Add(24*time.Hour)) + barCert, barKey, _ := generate.KeyPair("bar.com", now) + testCases := []struct { + desc string + dc *DomainsCertificates + expectedDc *DomainsCertificates + }{ + { + desc: "No empty certificate", + dc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Certificate: &Certificate{ + Certificate: fooCert, + PrivateKey: fooKey, + }, + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + { + Certificate: &Certificate{ + Certificate: barCert, + PrivateKey: barKey, + }, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + expectedDc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Certificate: &Certificate{ + Certificate: fooCert, + PrivateKey: fooKey, + }, + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + { + Certificate: &Certificate{ + Certificate: barCert, + PrivateKey: barKey, + }, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + }, + { + desc: "First certificate is nil", + dc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + { + Certificate: &Certificate{ + Certificate: barCert, + PrivateKey: barKey, + }, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + expectedDc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + { + Certificate: &Certificate{ + Certificate: nil, + PrivateKey: barKey, + }, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + }, + { + desc: "Last certificate is empty", + dc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Certificate: &Certificate{ + Certificate: fooCert, + PrivateKey: fooKey, + }, + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + { + Certificate: &Certificate{}, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + expectedDc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Certificate: &Certificate{ + Certificate: fooCert, + PrivateKey: fooKey, + }, + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + }, + }, + }, + { + desc: "First and last certificates are nil or empty", + dc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + { + Certificate: &Certificate{}, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + expectedDc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Certificate: &Certificate{ + Certificate: acmeCert, + PrivateKey: acmeKey, + }, + Domains: types.Domain{ + Main: "acme.wtf", + }, + }, + }, + }, + }, + { + desc: "All certificates are nil or empty", + dc: &DomainsCertificates{ + Certs: []*DomainsCertificate{ + { + Domains: types.Domain{ + Main: "foo.com", + }, + }, + { + Domains: types.Domain{ + Main: "foo24.com", + }, + }, + { + Certificate: &Certificate{}, + Domains: types.Domain{ + Main: "bar.com", + }, + }, + }, + }, + expectedDc: &DomainsCertificates{ + Certs: []*DomainsCertificate{}, + }, + }, + } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + a := &Account{DomainsCertificate: *test.dc} + a.Init() + + assert.Equal(t, len(test.expectedDc.Certs), len(a.DomainsCertificate.Certs)) + sort.Sort(&a.DomainsCertificate) + sort.Sort(test.expectedDc) + for key, value := range test.expectedDc.Certs { + assert.Equal(t, value.Domains.Main, a.DomainsCertificate.Certs[key].Domains.Main) + } + }) + } +}