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.

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

Dcow/challenge retry #241

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ coverage.txt
vendor
output
.idea

# vscode
*.code-workspace
22 changes: 14 additions & 8 deletions acme/api/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ package api

import (
"fmt"
"net/http"

"github.com/go-chi/chi"
"github.com/pkg/errors"
"github.com/smallstep/certificates/acme"
"github.com/smallstep/certificates/api"
"github.com/smallstep/certificates/authority/provisioner"
"github.com/smallstep/cli/jose"
"net/http"
)

func link(url, typ string) string {
Expand Down Expand Up @@ -181,13 +180,20 @@ func (h *Handler) GetChallenge(w http.ResponseWriter, r *http.Request) {
ch, err = h.Auth.ValidateChallenge(prov, acc.GetID(), chID, acc.GetKey())
if err != nil {
api.WriteError(w, err)
return
} else if ch.Retry.Active {
retryAfter, err := h.Auth.BackoffChallenge(prov, acc.GetID(), chID, acc.GetKey())
if err != nil {
api.WriteError(w, err)
} else {
w.Header().Add("Retry-After", retryAfter.String())
api.WriteProcessing(w, ch)
}
} else {
getLink := h.Auth.GetLink
w.Header().Add("Link", link(getLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, ch.GetAuthzID()), "up"))
w.Header().Set("Location", getLink(acme.ChallengeLink, acme.URLSafeProvisionerName(prov), true, ch.GetID()))
api.JSON(w, ch)
}

getLink := h.Auth.GetLink
w.Header().Add("Link", link(getLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, ch.GetAuthzID()), "up"))
w.Header().Set("Location", getLink(acme.ChallengeLink, acme.URLSafeProvisionerName(prov), true, ch.GetID()))
api.JSON(w, ch)
}

// GetCertificate ACME api for retrieving a Certificate.
Expand Down
52 changes: 51 additions & 1 deletion acme/api/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type mockAcmeAuthority struct {
updateAccount func(provisioner.Interface, string, []string) (*acme.Account, error)
useNonce func(string) error
validateChallenge func(p provisioner.Interface, accID string, id string, jwk *jose.JSONWebKey) (*acme.Challenge, error)
backoffChallenge func(p provisioner.Interface, accID, chID string, jwk *jose.JSONWebKey) (time.Duration, error)
ret1 interface{}
err error
}
Expand Down Expand Up @@ -203,6 +204,17 @@ func (m *mockAcmeAuthority) ValidateChallenge(p provisioner.Interface, accID str
}
}

func (m *mockAcmeAuthority) BackoffChallenge(p provisioner.Interface, accID, chID string, jwk *jose.JSONWebKey) (time.Duration, error) {
switch {
case m.backoffChallenge != nil:
return m.backoffChallenge(p, accID, chID, jwk)
case m.err != nil:
return -1, m.err
default:
return m.ret1.(time.Duration), m.err
}
}

func TestHandlerGetNonce(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -589,6 +601,7 @@ func ch() acme.Challenge {
URL: "https://ca.smallstep.com/acme/challenge/chID",
ID: "chID",
AuthzID: "authzID",
Retry: &acme.Retry{Called: 0, Active: false},
}
}

Expand Down Expand Up @@ -733,6 +746,37 @@ func TestHandlerGetChallenge(t *testing.T) {
ch: ch,
}
},
"ok/retry-after": func(t *testing.T) test {
key, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
acc := &acme.Account{ID: "accID", Key: key}
ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
ctx = context.WithValue(ctx, accContextKey, acc)
// TODO: Add correct key such that challenge object is already "active"
chiCtxInactive := chi.NewRouteContext()
chiCtxInactive.URLParams.Add("chID", "chID")
//chiCtxInactive.URLParams.Add("Active", "true")
ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtxInactive)
ch := ch()
ch.Retry.Active = true
chJSON, err := json.Marshal(ch)
assert.FatalError(t, err)
ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: chJSON})
return test{
auth: &mockAcmeAuthority{
validateChallenge: func(p provisioner.Interface, accID, id string, jwk *jose.JSONWebKey) (*acme.Challenge, error) {
assert.Equals(t, p, prov)
assert.Equals(t, accID, acc.ID)
assert.Equals(t, id, ch.ID)
assert.Equals(t, jwk.KeyID, key.KeyID)
return &ch, nil
},
},
ctx: ctx,
statusCode: 200,
ch: ch,
}
},
}
for name, run := range tests {
tc := run(t)
Expand Down Expand Up @@ -760,13 +804,19 @@ func TestHandlerGetChallenge(t *testing.T) {
assert.Equals(t, ae.Identifier, prob.Identifier)
assert.Equals(t, ae.Subproblems, prob.Subproblems)
assert.Equals(t, res.Header["Content-Type"], []string{"application/problem+json"})
} else {
} else if res.StatusCode >= 200 {
expB, err := json.Marshal(tc.ch)
assert.FatalError(t, err)
assert.Equals(t, bytes.TrimSpace(body), expB)
assert.Equals(t, res.Header["Link"], []string{fmt.Sprintf("<https://ca.smallstep.com/acme/authz/%s>;rel=\"up\"", tc.ch.AuthzID)})
assert.Equals(t, res.Header["Location"], []string{url})
assert.Equals(t, res.Header["Content-Type"], []string{"application/json"})
} else if res.StatusCode >= 100 {
expB, err := json.Marshal(tc.ch)
assert.FatalError(t, err)
assert.Equals(t, bytes.TrimSpace(body), expB)
assert.True(t, res.Header["Retry-After"] != nil)
assert.Equals(t, res.Header["Content-Type"], []string{"application/json"})
}
})
}
Expand Down
57 changes: 48 additions & 9 deletions acme/authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/tls"
"crypto/x509"
"encoding/base64"
"math"
"net"
"net/http"
"net/url"
Expand All @@ -19,6 +20,7 @@ import (

// Interface is the acme authority interface.
type Interface interface {
BackoffChallenge(provisioner.Interface, string, string, *jose.JSONWebKey) (time.Duration, error)
DeactivateAccount(provisioner.Interface, string) (*Account, error)
FinalizeOrder(provisioner.Interface, string, string, *x509.CertificateRequest) (*Order, error)
GetAccount(provisioner.Interface, string) (*Account, error)
Expand Down Expand Up @@ -263,21 +265,37 @@ func (a *Authority) ValidateChallenge(p provisioner.Interface, accID, chID strin
if accID != ch.getAccountID() {
return nil, UnauthorizedErr(errors.New("account does not own challenge"))
}
retry := ch.getRetry()
if retry.Active {
return ch.toACME(a.db, a.dir, p)
}
retry.Mux.Lock()
defer retry.Mux.Unlock()

client := http.Client{
Timeout: time.Duration(30 * time.Second),
}
dialer := &net.Dialer{
Timeout: 30 * time.Second,
}
ch, err = ch.validate(a.db, jwk, validateOptions{
httpGet: client.Get,
lookupTxt: net.LookupTXT,
tlsDial: func(network, addr string, config *tls.Config) (*tls.Conn, error) {
return tls.DialWithDialer(dialer, network, addr, config)
},
})
if err != nil {
return nil, Wrap(err, "error attempting challenge validation")
for ch.getRetry().Active {
ch, err = ch.validate(a.db, jwk, validateOptions{
httpGet: client.Get,
lookupTxt: net.LookupTXT,
tlsDial: func(network, addr string, config *tls.Config) (*tls.Conn, error) {
return tls.DialWithDialer(dialer, network, addr, config)
},
})
if err != nil {
return nil, Wrap(err, "error attempting challenge validation")
}
if ch.getStatus() == StatusValid {
break
}
if ch.getStatus() == StatusInvalid {
return ch.toACME(a.db, a.dir, p)
}
time.Sleep(ch.getBackoff())
}
return ch.toACME(a.db, a.dir, p)
}
Expand All @@ -293,3 +311,24 @@ func (a *Authority) GetCertificate(accID, certID string) ([]byte, error) {
}
return cert.toACME(a.db, a.dir)
}

// BackoffChallenge returns the total time to wait until the next sequence of validation attempts completes
func (a *Authority) BackoffChallenge(p provisioner.Interface, accID, chID string, jwk *jose.JSONWebKey) (time.Duration, error) {
ch, err := getChallenge(a.db, chID)
if err != nil {
return -1, err
}
if accID != ch.getAccountID() {
return -1, UnauthorizedErr(errors.New("account does not own challenge"))
}

remCalls := ch.getRetry().MaxAttempts - math.Mod(ch.getRetry().Called, ch.getRetry().MaxAttempts)
totBackoff := 0 * time.Second
for i := 0; i < int(remCalls); i++ {
clone := ch.clone()
clone.Retry.Called += float64(i)
totBackoff += clone.getBackoff()
}

return totBackoff, nil
}
3 changes: 1 addition & 2 deletions acme/authority_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,7 @@ func TestAuthorityValidateChallenge(t *testing.T) {
assert.Fatal(t, ok)
_ch.baseChallenge.Status = StatusValid
_ch.baseChallenge.Validated = clock.Now()
_ch.baseChallenge.Retry.Called = 0
b, err := json.Marshal(ch)
assert.FatalError(t, err)
auth, err := NewAuthority(&db.MockNoSQLDB{
Expand Down Expand Up @@ -1309,12 +1310,10 @@ func TestAuthorityValidateChallenge(t *testing.T) {
if assert.Nil(t, tc.err) {
gotb, err := json.Marshal(acmeCh)
assert.FatalError(t, err)

acmeExp, err := tc.ch.toACME(nil, tc.auth.dir, prov)
assert.FatalError(t, err)
expb, err := json.Marshal(acmeExp)
assert.FatalError(t, err)

assert.Equals(t, expb, gotb)
}
}
Expand Down
Loading