Skip to content

Commit

Permalink
Fix infinite authorization if error autorize domain.
Browse files Browse the repository at this point in the history
  • Loading branch information
rekby authored Mar 7, 2020
2 parents ad5e57c + eb6a3d1 commit fec9ab0
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 8 deletions.
18 changes: 15 additions & 3 deletions internal/acme_client_manager/client_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"sync"
"time"

zc "github.com/rekby/zapcontext"

"golang.org/x/xerrors"

"go.uber.org/zap"
Expand Down Expand Up @@ -62,18 +64,24 @@ type acmeManagerState struct {
}

func (m *AcmeManager) Close() error {
logger := zc.L(m.ctx)
logger.Debug("Start close")
m.mu.Lock()
alreadyClosed := m.closed
ctxAutorenewCompleted := m.ctxAutorenewCompleted
m.closed = true
m.ctxCancel()
m.mu.Unlock()
logger.Debug("Set closed flag", zap.Any("autorenew_context", ctxAutorenewCompleted))

if alreadyClosed {
return xerrors.Errorf("close: %w", errClosed)
}

if ctxAutorenewCompleted != nil {
logger.Debug("Start waiting for complete autorenew")
<-ctxAutorenewCompleted.Done()
logger.Debug("Autorenew context closed")
}
return nil
}
Expand Down Expand Up @@ -129,16 +137,20 @@ func (m *AcmeManager) GetClient(ctx context.Context) (*acme.Client, error) {
}

func (m *AcmeManager) accountRenew() {
logger := zc.L(m.ctx)
ctx, ctxCancel := context.WithCancel(m.ctx)
defer ctxCancel()
if ctx.Err() != nil {
return
}

m.mu.Lock()
m.ctxAutorenewCompleted = ctx
m.mu.Unlock()

if m.ctx.Err() != nil {
return
}

logger.Debug("Start account autorenew")

ticker := time.NewTicker(m.RenewAccountInterval)
ctxDone := m.ctx.Done()

Expand Down
35 changes: 30 additions & 5 deletions internal/cert_manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"sync"
"time"

"github.com/rekby/lets-proxy2/internal/contexthelper"

"github.com/rekby/lets-proxy2/internal/metrics"

"github.com/prometheus/client_golang/prometheus"
Expand All @@ -46,6 +48,7 @@ const (
const domainKeyRSALength = 2048
const renewBeforeExpire = time.Hour * 24 * 30
const revokeAuthorizationTimeout = 5 * time.Minute
const cleanupTimeout = time.Minute

var errHaveNoCert = errors.New("have no certificate for domain") // may return for any internal error

Expand Down Expand Up @@ -338,7 +341,7 @@ func (m *Manager) certStateGet(ctx context.Context, cd CertDescription) *certSta
if err == cache.ErrCacheMiss {
err = nil
}
log.DebugFatalCtx(ctx, err, "Got cert state from cache", zap.Bool("is_emapty", resInterface == nil))
log.DebugFatalCtx(ctx, err, "Got cert state from cache", zap.Bool("is_empty", resInterface == nil))
if resInterface == nil {
resInterface = &certState{}
err = m.certState.Put(ctx, cd.String(), resInterface)
Expand Down Expand Up @@ -397,8 +400,18 @@ func (m *Manager) createOrderForDomains(ctx context.Context, domains ...DomainNa
logger.Debug("Start order authorization.")
var order *acme.Order

firstLoop := true
authorizeOrderLoop:
for {
if ctx.Err() != nil {
return nil, xerrors.Errorf("context canceled: %w", ctx.Err())
}

if firstLoop {
firstLoop = false
} else {
time.Sleep(time.Second)
}
authIDs := make([]acme.AuthzID, len(domains))
for i := range domains {
authIDs[i] = acme.AuthzID{Type: "dns", Value: domains[i].ASCII()}
Expand Down Expand Up @@ -467,16 +480,24 @@ authorizeOrderLoop:

// Respond to the challenge and wait for validation result.
cleanup, err := m.fulfill(ctx, chal, DomainName(z.Identifier.Value))
log.DebugError(logger, err, "Write respond to challenge")
if err != nil {
continue authorizeOrderLoop
}
cleanupContext, cleanupContextCancel := context.WithTimeout(contexthelper.DropCancelContext(ctx), cleanupTimeout)
//noinspection GoDeferInLoop
defer cleanupContextCancel()
//noinspection GoDeferInLoop
defer cleanup(ctx)
defer cleanup(cleanupContext)

if _, err := client.Accept(ctx, chal); err != nil {
authorizedChallenge, err := client.Accept(ctx, chal)
log.DebugError(logger, err, "accept authorization", zap.Reflect("authorized_challenge", authorizedChallenge))
if err != nil {
continue authorizeOrderLoop
}
if _, err := client.WaitAuthorization(ctx, z.URI); err != nil {
authorization, err := client.WaitAuthorization(ctx, z.URI)
log.DebugError(logger, err, "wait authorization", zap.Reflect("authorization", authorization))
if err != nil {
continue authorizeOrderLoop
}
}
Expand Down Expand Up @@ -614,7 +635,11 @@ func (m *Manager) fulfill(ctx context.Context, challenge *acme.Challenge, domain
key := domain.ASCII() + "/" + challenge.Token
err = m.httpTokens.Put(ctx, key, []byte(resp))
log.DebugError(logger, err, "Put token for http-01", zap.String("key", key))
return func(localContext context.Context) { _ = m.httpTokens.Delete(localContext, key) }, err
if err == nil {
return func(localContext context.Context) { _ = m.httpTokens.Delete(localContext, key) }, nil
} else {
return nil, err
}
default:
logger.Error("Unknow challenge type", zap.Reflect("challenge", challenge))
return nil, errors.New("unknown challenge type")
Expand Down
24 changes: 24 additions & 0 deletions internal/contexthelper/contexthelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ type CombinedContext struct {
err error
}

type droppedCancelContext struct {
ctx context.Context
}

// Deadline return minimum of contextx deadlines, if present
func (cc *CombinedContext) Deadline() (deadline time.Time, ok bool) {
return cc.deadline, cc.deadlineOk
Expand Down Expand Up @@ -93,3 +97,23 @@ func (cc *CombinedContext) waitCloseAny() {
close(cc.done)
cc.mu.Unlock()
}

func DropCancelContext(ctx context.Context) context.Context {
return droppedCancelContext{ctx}
}

func (d droppedCancelContext) Deadline() (deadline time.Time, ok bool) {
return time.Time{}, false
}

func (d droppedCancelContext) Done() <-chan struct{} {
return nil
}

func (d droppedCancelContext) Err() error {
return nil
}

func (d droppedCancelContext) Value(key interface{}) interface{} {
return d.ctx.Value(key)
}
19 changes: 19 additions & 0 deletions internal/contexthelper/contexthelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,22 @@ func TestCombinedContext_Done(t *testing.T) {
wait()
td.True(done)
}

func TestDropCancelContext(t *testing.T) {
td := testdeep.NewT(t)
type keyType string
const key keyType = "key"
const val = "val"

ctx, ctxCancel := context.WithCancel(context.WithValue(context.Background(), key, val))
dropCancel := DropCancelContext(ctx)
ctxCancel()

deadline, ok := dropCancel.Deadline()
td.Cmp(deadline, time.Time{})
td.False(ok)

td.CmpNoError(dropCancel.Err())
td.Nil(dropCancel.Done())
td.Cmp(dropCancel.Value(key), val)
}

0 comments on commit fec9ab0

Please sign in to comment.