Skip to content

ACME (RFC 8555) § 8.2 Challenge Retries #242

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

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
6fdbd85
git: Ignore *.code-workspace
dcow Apr 28, 2020
40d7c42
Implement acme RFC 8555, challenge retries
wesgraham Feb 7, 2020
66b2c4b
Add automated challenge retries, RFC 8555
wesgraham Feb 14, 2020
f9779d0
Polish retry conditions
wesgraham Feb 19, 2020
8d43567
Implement standard backoff strategy
wesgraham Feb 21, 2020
8fb558d
handler_test: Remove unused field "Backoffs"
dcow Apr 30, 2020
f56c449
handler_test: Add BackoffChallenge
dcow Apr 30, 2020
5e6a020
acme/authority: Add space around `*`
dcow Apr 30, 2020
9af4dd3
acme: Retry challenge validation attempts
dcow May 6, 2020
bdadea8
acme: go fmt
dcow May 7, 2020
9518ba4
provisioner/acme: Add TODO for retry restarts
dcow May 12, 2020
8326632
vscode: Ignore vscode binaries
dcow May 12, 2020
2d0a00c
acme/api: Add missing return
dcow May 12, 2020
a857c45
acme/authority: Polymorph the challenge type
dcow May 12, 2020
9f18882
acme/challenge: Copy retry information on clone
dcow May 12, 2020
089e3ae
acme/challenge: Fix error return type on KeyAuthorization
dcow May 12, 2020
2514b58
acme/api: Fixup handler_test
dcow May 12, 2020
84af2ad
acme: Fix test compile
dcow May 12, 2020
8556d45
acme/authority: Move comment onto correct block
dcow May 13, 2020
794725b
acme/api: Remove unused BackoffChallenge func
dcow May 13, 2020
8ae32f5
acme: Fix comment style to appease linter
dcow May 13, 2020
609e131
acme/api: Write headers for invalid challenges
dcow May 13, 2020
b061d0a
acme/authority: Fix error message in test
dcow May 13, 2020
976c8f8
acme/authority: Fix tests
dcow May 13, 2020
5354906
acme/api: Add func name to beginning of comment
dcow May 13, 2020
5e5a76c
acme/api: Set Link and Location headers for all 200
dcow May 13, 2020
b8b3ca2
acme/authority: Add descriptive intro to ValidateChallenge
dcow May 13, 2020
c378e00
acme: Move ordinal to application
dcow May 14, 2020
f022818
project: go mod tidy
dcow May 14, 2020
deacbdc
acme: Don't panic on logic errors
dcow May 14, 2020
d5f95de
Merge branch 'master' into dcow/challenge-retry
dcow May 18, 2020
9103880
Merge branch 'master' into dcow/challenge-retry
dcow May 19, 2020
0f63e43
acme: Update http-01 challenge tests
dcow May 19, 2020
d54f963
acme/retry: Update DNS challenge tests
dcow May 21, 2020
0578055
acme/retry: Cleanup tls-alpn-01 tests
dcow May 21, 2020
6c39439
acme: make fmt
dcow May 21, 2020
112fc59
make: Fix lint errors
dcow May 21, 2020
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