From e881c8b4a18882c5fa8bb0b6d8b6f5c91938e4c1 Mon Sep 17 00:00:00 2001 From: rekby Date: Wed, 1 May 2019 03:11:01 +0300 Subject: [PATCH 1/3] Can lock certificates Close #48 --- internal/cert_manager/cert-state.go | 18 +++++++++++- internal/cert_manager/helpers.go | 10 +++++-- internal/cert_manager/manager.go | 42 ++++++++++++++++++++++----- internal/cert_manager/manager_test.go | 27 +++++++++++++++++ 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/internal/cert_manager/cert-state.go b/internal/cert_manager/cert-state.go index 5058d8ff..50ff5be6 100644 --- a/internal/cert_manager/cert-state.go +++ b/internal/cert_manager/cert-state.go @@ -19,6 +19,7 @@ type certState struct { issueContext context.Context // nil if no issue process now issueContextCancel func() cert *tls.Certificate + locked bool lastError error } @@ -102,10 +103,25 @@ func (s *certState) Cert() (cert *tls.Certificate, lastError error) { return cert, lastError } -func (s *certState) CertSet(ctx context.Context, cert *tls.Certificate) { +func (s *certState) CertSet(ctx context.Context, locked bool, cert *tls.Certificate) { zc.L(ctx).Debug("Store certificate in local state", log.Cert(cert)) s.mu.Lock() s.cert = cert + s.locked = locked + s.lastError = nil s.mu.Unlock() } + +func (s *certState) SetLocked() { + s.mu.Lock() + s.locked = true + s.mu.Unlock() +} + +func (s *certState) GetLocked() bool { + s.mu.RLock() + defer s.mu.RUnlock() + + return s.locked +} diff --git a/internal/cert_manager/helpers.go b/internal/cert_manager/helpers.go index 897e6ce7..83fa7ccd 100644 --- a/internal/cert_manager/helpers.go +++ b/internal/cert_manager/helpers.go @@ -53,7 +53,7 @@ func flatByteSlices(in [][]byte) []byte { } // Return valid parced certificate or error -func validCertDer(domains []DomainName, der [][]byte, key crypto.PrivateKey, now time.Time) (cert *tls.Certificate, err error) { +func validCertDer(domains []DomainName, der [][]byte, key crypto.PrivateKey, locked bool, now time.Time) (cert *tls.Certificate, err error) { // parse public part(s) x509Cert, err := x509.ParseCertificates(flatByteSlices(der)) if err != nil || len(x509Cert) == 0 { @@ -68,10 +68,10 @@ func validCertDer(domains []DomainName, der [][]byte, key crypto.PrivateKey, now Leaf: leaf, } - return validCertTLS(cert, domains, now) + return validCertTLS(cert, domains, locked, now) } -func validCertTLS(cert *tls.Certificate, domains []DomainName, now time.Time) (validCert *tls.Certificate, err error) { +func validCertTLS(cert *tls.Certificate, domains []DomainName, locked bool, now time.Time) (validCert *tls.Certificate, err error) { if cert == nil { return nil, errors.New("certificate is nil") } @@ -88,6 +88,10 @@ func validCertTLS(cert *tls.Certificate, domains []DomainName, now time.Time) (v return nil, errors.New("certificate has no public key") } + if locked { + return cert, nil + } + // ensure the leaf corresponds to the private key and matches the certKey type switch pub := cert.Leaf.PublicKey.(type) { case *rsa.PublicKey: diff --git a/internal/cert_manager/manager.go b/internal/cert_manager/manager.go index 041b97b1..1b30992c 100644 --- a/internal/cert_manager/manager.go +++ b/internal/cert_manager/manager.go @@ -133,8 +133,7 @@ func (m *Manager) GetCertificate(hello *tls.ClientHelloInfo) (resultCert *tls.Ce if cert != nil { logger.Debug("Got certificate from local state", log.Cert(cert)) - // TODO: Disable check for locked certificates https://github.com/rekby/lets-proxy2/issues/48 - cert, err = validCertDer([]DomainName{needDomain}, cert.Certificate, cert.PrivateKey, now) + cert, err = validCertDer([]DomainName{needDomain}, cert.Certificate, cert.PrivateKey, certState.GetLocked(), now) logger.Debug("Validate certificate from local state", zap.Error(err)) if err == nil { return cert, nil @@ -144,24 +143,29 @@ func (m *Manager) GetCertificate(hello *tls.ClientHelloInfo) (resultCert *tls.Ce logger.Debug("Can't get certificate from local state", zap.Error(err)) } + locked, err := isCertLocked(ctx, m.Cache, certName) + log.DebugDPanic(logger, err, "Check if certificate locked") + cert, err = getCertificate(ctx, m.Cache, certName, keyRSA) if err == nil { logger.Debug("Certificate loaded from cache") - // TODO: Disable check for locked certificates https://github.com/rekby/lets-proxy2/issues/48 - cert, err = validCertDer([]DomainName{needDomain}, cert.Certificate, cert.PrivateKey, now) + cert, err = validCertDer([]DomainName{needDomain}, cert.Certificate, cert.PrivateKey, locked, now) logger.Debug("Check if certificate ok", zap.Error(err)) if err == nil { - certState.CertSet(ctx, cert) + certState.CertSet(ctx, locked, cert) return cert, nil } } + if locked { + return nil, errHaveNoCert + } + // TODO: check domain certIssueContext, cancelFunc := context.WithTimeout(ctx, m.CertificateIssueTimeout) defer cancelFunc() - // TODO: receive cert for domain and subdomains same time res, err := m.createCertificateForDomains(certIssueContext, certName, domainNamesFromCertificateName(certName), needDomain) if err == nil { @@ -376,7 +380,7 @@ func (m *Manager) issueCertificate(ctx context.Context, certName certNameType, d return nil, err } - cert, err := validCertDer(domains, der, key, time.Now()) + cert, err := validCertDer(domains, der, key, false, time.Now()) log.DebugDPanic(logger, err, "Check certificate is valid") if err == nil { storeCertificate(ctx, m.Cache, certName, cert) @@ -524,6 +528,11 @@ func storeCertificate(ctx context.Context, cache cache.Cache, certName certNameT return } + locked, _ := isCertLocked(ctx, cache, certName) + if locked { + logger.Panic("Logical error - try to save to locked certificate") + } + var keyType keyType var certBuf bytes.Buffer @@ -601,7 +610,11 @@ func getCertificate(ctx context.Context, cache cache.Cache, certName certNameTyp return nil, err } } - return validCertTLS(&cert2, nil, time.Now()) + locked, err := isCertLocked(ctx, cache, certName) + if err != nil { + return nil, err + } + return validCertTLS(&cert2, nil, locked, time.Now()) } func getCertificateKeyBytes(ctx context.Context, cache cache.Cache, certName certNameType, keyType keyType) ([]byte, error) { @@ -674,3 +687,16 @@ func isNeedRenew(cert *tls.Certificate, now time.Time) bool { } return cert.Leaf.NotAfter.Add(-time.Hour * 24 * 30).Before(now) } + +func isCertLocked(ctx context.Context, storage cache.Cache, certName certNameType) (bool, error) { + lockName := certName.String() + ".lock" + _, err := storage.Get(ctx, lockName) + switch err { + case cache.ErrCacheMiss: + return false, nil + case nil: + return true, nil + default: + return false, err + } +} diff --git a/internal/cert_manager/manager_test.go b/internal/cert_manager/manager_test.go index a74383f0..5f36c749 100644 --- a/internal/cert_manager/manager_test.go +++ b/internal/cert_manager/manager_test.go @@ -65,6 +65,11 @@ func TestManager_GetCertificateTls(t *testing.T) { cacheMock := NewCacheMock(mc) cacheMock.GetMock.Set(func(ctx context.Context, key string) (ba1 []byte, err error) { zc.L(ctx).Debug("Cache mock get", zap.String("key", key)) + + if key == "locked.ru.lock" { + return []byte{}, nil + } + return nil, cache.ErrCacheMiss }) cacheMock.PutMock.Set(func(ctx context.Context, key string, data []byte) (err error) { @@ -137,6 +142,14 @@ func TestManager_GetCertificateTls(t *testing.T) { } }) + t.Run("Locked", func(t *testing.T) { + domain := "locked.ru" + + cert, err := manager.GetCertificate(&tls.ClientHelloInfo{ServerName: domain, Conn: contextConnection{Context: ctx}}) + td.CmpError(t, err) + td.CmpNil(t, cert) + }) + t.Run("punycode-domain", func(t *testing.T) { domain := "xn--80adjurfhd.xn--p1ai" // проверка.рф @@ -230,6 +243,11 @@ func TestManager_GetCertificateHttp01(t *testing.T) { cacheMock := NewCacheMock(mc) cacheMock.GetMock.Set(func(ctx context.Context, key string) (ba1 []byte, err error) { zc.L(ctx).Debug("Cache mock get", zap.String("key", key)) + + if key == "locked.ru.lock" { + return []byte{}, nil + } + return nil, cache.ErrCacheMiss }) cacheMock.PutMock.Set(func(ctx context.Context, key string, data []byte) (err error) { @@ -293,6 +311,14 @@ func TestManager_GetCertificateHttp01(t *testing.T) { } }) + t.Run("Locked", func(t *testing.T) { + domain := "locked.ru" + + cert, err := manager.GetCertificate(&tls.ClientHelloInfo{ServerName: domain, Conn: contextConnection{Context: ctx}}) + td.CmpError(t, err) + td.CmpNil(t, cert) + }) + t.Run("punycode-domain", func(t *testing.T) { domain := "xn--80adjurfhd.xn--p1ai" // проверка.рф @@ -419,6 +445,7 @@ func TestStoreCertificate(t *testing.T) { fmt.Println(string(data)) return nil }) + cacheMock.GetMock.Return(nil, cache.ErrCacheMiss) storeCertificate(ctx, cacheMock, "asd", cert) } From 6d584fd31751f1361bb842d10166e726ed325eae Mon Sep 17 00:00:00 2001 From: rekby Date: Wed, 1 May 2019 03:56:42 +0300 Subject: [PATCH 2/3] Manual versioning --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4b93d638..dd65a862 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,21 +30,21 @@ before_deploy: - git config --local user.name "$GIT_NAME" - git config --local user.email "$GIT_EMAIL" - export TRAVIS_TAG="$TAG_PREFIX.$TRAVIS_BUILD_NUMBER" - - BUILD_TIME=$(TZ=UTC date --rfc-3339=seconds) + - BUILD_TIME=$(TZ=UTC date --rfc-3339=seconds | cut -d '+' -f 1 | tr -d - | tr ' ' '-' | tr -d :) - git tag $TRAVIS_TAG - go get -mod= github.com/mitchellh/gox - mkdir -p output - OS_ARCH_BUILDS="darwin/amd64 linux/386 linux/amd64 linux/arm freebsd/386 freebsd/amd64 freebsd/arm windows/386 windows/amd64" - - gox --mod=vendor -osarch "$OS_ARCH_BUILDS" --ldflags "-X \"main.VERSION=$TRAVIS_TAG commit $TRAVIS_COMMIT builded '$BUILD_TIME' buildNumber $TRAVIS_BUILD_NUMBER\"" --output="output/lets-proxy_{{.OS}}_{{.Arch}}" -verbose --rebuild ./cmd/ + - gox --mod=vendor -osarch "$OS_ARCH_BUILDS" --ldflags "-X \"main.VERSION=$TRAVIS_TAG+build-$TRAVIS_BUILD_NUMBER, Build time $BUILD_TIME, commit $TRAVIS_COMMIT\"" --output="output/lets-proxy_{{.OS}}_{{.Arch}}" -verbose --rebuild ./cmd/ - bash tests/make_archives.sh deploy: skip_cleanup: true provider: releases - draft: true + tags: true on: repo: rekby/lets-proxy2 - branch: master + tags: true api_key: $GITHUB_TOKEN file_glob: true file: output/* From 4970c9a1f8e7cbef0f55c96beb38582ce206c8b3 Mon Sep 17 00:00:00 2001 From: rekby Date: Wed, 1 May 2019 04:03:47 +0300 Subject: [PATCH 3/3] Nop logger for test --- internal/cert_manager/manager_test.go | 20 +++++--------------- internal/th/helper.go | 12 +----------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/internal/cert_manager/manager_test.go b/internal/cert_manager/manager_test.go index 5f36c749..d760337a 100644 --- a/internal/cert_manager/manager_test.go +++ b/internal/cert_manager/manager_test.go @@ -43,22 +43,15 @@ func (c contextConnection) GetContext() context.Context { //nolint:gochecknoinits func init() { - logger, err := zap.NewDevelopment() - if err != nil { - panic(err) - } - zc.SetDefaultLogger(logger) + zc.SetDefaultLogger(zap.NewNop()) } func TestManager_GetCertificateTls(t *testing.T) { - logger, err := zap.NewDevelopment() - if err != nil { - t.Fatal(err) - } - ctx, flush := th.TestContext() defer flush() + logger := zc.L(ctx) + mc := minimock.NewController(t) defer mc.Finish() @@ -229,14 +222,11 @@ func TestManager_GetCertificateTls(t *testing.T) { } func TestManager_GetCertificateHttp01(t *testing.T) { - logger, err := zap.NewDevelopment() - if err != nil { - t.Fatal(err) - } - ctx, flush := th.TestContext() defer flush() + logger := zc.L(ctx) + mc := minimock.NewController(t) defer mc.Finish() diff --git a/internal/th/helper.go b/internal/th/helper.go index ddc5b49a..646ea7ef 100644 --- a/internal/th/helper.go +++ b/internal/th/helper.go @@ -2,8 +2,6 @@ package th import ( "context" - "fmt" - "time" zc "github.com/rekby/zapcontext" @@ -11,17 +9,9 @@ import ( ) func TestContext() (ctx context.Context, flush func()) { - logger, err := zap.NewDevelopment() - if err != nil { - fmt.Print() - panic(err) - } - - ctx, cancel := context.WithCancel(zc.WithLogger(context.Background(), logger)) + ctx, cancel := context.WithCancel(zc.WithLogger(context.Background(), zap.NewNop())) flush = func() { cancel() - time.Sleep(time.Millisecond) - _ = logger.Sync() } return ctx, flush }