acme: stop polling authz on 4xx client errors

"At Let's Encrypt, we are seeing clients in the wild that continue
polling their challenges long after those challenges have expired and
started serving 404."

The 4xx response code errors are client errors and should not be
retried.

Fixes golang/go#24145

Change-Id: I012c584fc4defd3a0d64a653860c35705c5c6653
Reviewed-on: https://go-review.googlesource.com/97695
Run-TryBot: Alex Vaghin <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>

acme/acme.go

@@ -400,7 +400,7 @@ func (c *Client) RevokeAuthorization(ctx context.Context, url string) error {
 
 // WaitAuthorization polls an authorization at the given URL
 // until it is in one of the final states, StatusValid or StatusInvalid,
-// or the context is done.
+// the ACME CA responded with a 4xx error code, or the context is done.
 //
 // It returns a non-nil Authorization only if its Status is StatusValid.
 // In all other cases WaitAuthorization returns an error.
@@ -412,6 +412,13 @@ func (c *Client) WaitAuthorization(ctx context.Context, url string) (*Authorizat
 		if err != nil {
 			return nil, err
 		}
+		if res.StatusCode >= 400 && res.StatusCode <= 499 {
+			// Non-retriable error. For instance, Let's Encrypt may return 404 Not Found
+			// when requesting an expired authorization.
+			defer res.Body.Close()
+			return nil, responseError(res)
+		}
+
 		retry := res.Header.Get("Retry-After")
 		if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusAccepted {
 			res.Body.Close()

acme/acme_test.go

@@ -549,6 +549,34 @@ func TestWaitAuthorizationInvalid(t *testing.T) {
 	}
 }
 
+func TestWaitAuthorizationClientError(t *testing.T) {
+	const code = http.StatusBadRequest
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(code)
+	}))
+	defer ts.Close()
+
+	ch := make(chan error, 1)
+	go func() {
+		var client Client
+		_, err := client.WaitAuthorization(context.Background(), ts.URL)
+		ch <- err
+	}()
+
+	select {
+	case <-time.After(3 * time.Second):
+		t.Fatal("WaitAuthz took too long to return")
+	case err := <-ch:
+		res, ok := err.(*Error)
+		if !ok {
+			t.Fatalf("err is %v (%T); want a non-nil *Error", err, err)
+		}
+		if res.StatusCode != code {
+			t.Errorf("res.StatusCode = %d; want %d", res.StatusCode, code)
+		}
+	}
+}
+
 func TestWaitAuthorizationCancel(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Retry-After", "60")