diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go index 263b291..97281d8 100644 --- a/acme/autocert/autocert.go +++ b/acme/autocert/autocert.go @@ -542,6 +542,20 @@ func (m *Manager) authorizedCert(ctx context.Context, key crypto.Signer, domain return der, leaf, nil } +// revokePending revokes all provided authorizations (passed as a map of URIs) +func (m *Manager) revokePending(ctx context.Context, pendingAuthzURIs map[string]bool) { + f := func(ctx context.Context) { + for uri := range pendingAuthzURIs { + m.Client.RevokeAuthorization(ctx, uri) + } + } + if waitForRevocations { + f(ctx) + } else { + go f(context.Background()) + } +} + // verify runs the identifier (domain) authorization flow // using each applicable ACME challenge type. func (m *Manager) verify(ctx context.Context, client *acme.Client, domain string) error { @@ -554,6 +568,10 @@ func (m *Manager) verify(ctx context.Context, client *acme.Client, domain string } m.tokensMu.RUnlock() + // we keep track of pending authzs and revoke the ones that did not validate. + pendingAuthzs := make(map[string]bool) + defer m.revokePending(ctx, pendingAuthzs) + var nextTyp int // challengeType index of the next challenge type to try for { // Start domain authorization and get the challenge. @@ -570,6 +588,8 @@ func (m *Manager) verify(ctx context.Context, client *acme.Client, domain string return fmt.Errorf("acme/autocert: invalid authorization %q", authz.URI) } + pendingAuthzs[authz.URI] = true + // Pick the next preferred challenge. var chal *acme.Challenge for chal == nil && nextTyp < len(challengeTypes) { @@ -590,6 +610,7 @@ func (m *Manager) verify(ctx context.Context, client *acme.Client, domain string // A challenge is fulfilled and accepted: wait for the CA to validate. if _, err := client.WaitAuthorization(ctx, authz.URI); err == nil { + delete(pendingAuthzs, authz.URI) return nil } } @@ -959,4 +980,7 @@ var ( // Called when a state is removed. testDidRemoveState = func(domain string) {} + + // make testing of revokePending synchronous + waitForRevocations = false ) diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go index 2da1912..54d38e2 100644 --- a/acme/autocert/autocert_test.go +++ b/acme/autocert/autocert_test.go @@ -19,10 +19,12 @@ import ( "fmt" "html/template" "io" + "io/ioutil" "math/big" "net/http" "net/http/httptest" "reflect" + "strconv" "strings" "sync" "testing" @@ -529,6 +531,103 @@ func TestVerifyHTTP01(t *testing.T) { } } +func TestRevokeFailedAuth(t *testing.T) { + var ( + authzCount int // num. of created authorizations + revoked [3]bool // there will be three different authorization attempts; see below + ) + + // make cleanup revocations synchronous + waitForRevocations = true + + // ACME CA server stub, only the needed bits. + // TODO: Merge this with startACMEServerStub, making it a configurable CA for testing. + var ca *httptest.Server + ca = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Replay-Nonce", "nonce") + if r.Method == "HEAD" { + // a nonce request + return + } + + switch r.URL.Path { + // Discovery. + case "/": + if err := discoTmpl.Execute(w, ca.URL); err != nil { + t.Errorf("discoTmpl: %v", err) + } + // Client key registration. + case "/new-reg": + w.Write([]byte("{}")) + // New domain authorization. + case "/new-authz": + w.Header().Set("Location", fmt.Sprintf("%s/authz/%d", ca.URL, authzCount)) + w.WriteHeader(http.StatusCreated) + if err := authzTmpl.Execute(w, ca.URL); err != nil { + t.Errorf("authzTmpl: %v", err) + } + authzCount++ + // force error on Accept() + case "/challenge/2": + http.Error(w, "won't accept tls-sni-02 challenge", http.StatusBadRequest) + // accept; authorization will be expired (404; see /authz/1 below) + case "/challenge/1": + w.Write([]byte("{}")) + // Authorization statuses. + case "/authz/0", "/authz/1", "/authz/2": + if r.Method == "POST" { + body, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Errorf("could not read request body") + } + if reflect.DeepEqual(body, []byte(`{"status": "deactivated"}`)) { + t.Errorf("unexpected POST to authz: '%s'", body) + } + i, err := strconv.ParseInt(r.URL.Path[len(r.URL.Path)-1:], 10, 64) + if err != nil { + t.Errorf("could not convert authz ID to int") + } + revoked[i] = true + w.Write([]byte(`{"status": "invalid"}`)) + } else { + switch r.URL.Path { + case "/authz/0": + // ensure we get a spurious validation if error forcing did not work (see above) + w.Write([]byte(`{"status": "valid"}`)) + case "/authz/1": + // force error during WaitAuthorization() + w.WriteHeader(http.StatusNotFound) + } + } + default: + http.NotFound(w, r) + t.Errorf("unrecognized r.URL.Path: %s", r.URL.Path) + } + })) + defer ca.Close() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + m := &Manager{ + Client: &acme.Client{ + Key: key, + DirectoryURL: ca.URL, + }, + } + + // should fail and revoke tsl-sni-02, tls-sni-01 and the third time after not finding any remaining challenges + if err := m.verify(context.Background(), m.Client, "example.org"); err == nil { + t.Errorf("m.verify should have failed!") + } + for i := range revoked { + if !revoked[i] { + t.Errorf("authorization attempt %d not revoked after error", i) + } + } +} + func TestHTTPHandlerDefaultFallback(t *testing.T) { tt := []struct { method, url string