From 91a49db82a88618983a78a06c1cbd4e00ab749ab Mon Sep 17 00:00:00 2001
From: Alex Vaghin <ddos@google.com>
Date: Wed, 28 Feb 2018 16:18:34 +0100
Subject: [PATCH] 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 <ddos@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
---
 acme/acme.go      |  9 ++++++++-
 acme/acme_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/acme/acme.go b/acme/acme.go
index fa9c4b39e2..1f4fb69edd 100644
--- a/acme/acme.go
+++ b/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()
diff --git a/acme/acme_test.go b/acme/acme_test.go
index 89f2efaa5e..63cb79b985 100644
--- a/acme/acme_test.go
+++ b/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")