acme: try to fetch nonce from directory first

The change should reduce resource quota consumed by the client overall.

Instead of sending HEAD to an ACME resource URL to get a new nonce,
the Client will now try to fetch it from the Directory URL first
and only then from the ACME resource URL if the former fails.

This builds up on an abandoned https://golang.org/cl/34623,
only this time with a fallback to the original behaviour.

Change-Id: I6e75c0e524c4bc751f3a651b290c0ac2493e0628
Reviewed-on: https://go-review.googlesource.com/c/162057
Run-TryBot: Alex Vaghin <ddos@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

acme/acme.go

@@ -128,11 +128,7 @@ func (c *Client) Discover(ctx context.Context) (Directory, error) {
 		return *c.dir, nil
 	}
 
-	dirURL := c.DirectoryURL
+	res, err := c.get(ctx, c.directoryURL(), wantStatus(http.StatusOK))
-	if dirURL == "" {
-		dirURL = LetsEncryptURL
-	}
-	res, err := c.get(ctx, dirURL, wantStatus(http.StatusOK))
 	if err != nil {
 		return Directory{}, err
 	}
@@ -165,6 +161,13 @@ func (c *Client) Discover(ctx context.Context) (Directory, error) {
 	return *c.dir, nil
 }
 
+func (c *Client) directoryURL() string {
+	if c.DirectoryURL != "" {
+		return c.DirectoryURL
+	}
+	return LetsEncryptURL
+}
+
 // CreateCert requests a new certificate using the Certificate Signing Request csr encoded in DER format.
 // The exp argument indicates the desired certificate validity duration. CA may issue a certificate
 // with a different duration.
@@ -711,12 +714,18 @@ func (c *Client) doReg(ctx context.Context, url string, typ string, acct *Accoun
 }
 
 // popNonce returns a nonce value previously stored with c.addNonce
-// or fetches a fresh one from the given URL.
+// or fetches a fresh one from a URL by issuing a HEAD request.
+// It first tries c.directoryURL() and then the provided url if the former fails.
 func (c *Client) popNonce(ctx context.Context, url string) (string, error) {
 	c.noncesMu.Lock()
 	defer c.noncesMu.Unlock()
 	if len(c.nonces) == 0 {
-		return c.fetchNonce(ctx, url)
+		dirURL := c.directoryURL()
+		v, err := c.fetchNonce(ctx, dirURL)
+		if err != nil && url != dirURL {
+			v, err = c.fetchNonce(ctx, url)
+		}
+		return v, err
 	}
 	var nonce string
 	for nonce = range c.nonces {

acme/acme_test.go

@@ -75,6 +75,7 @@ func TestDiscover(t *testing.T) {
 	)
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
+		w.Header().Set("Replay-Nonce", "testnonce")
 		fmt.Fprintf(w, `{
 			"new-reg": %q,
 			"new-authz": %q,
@@ -100,6 +101,9 @@ func TestDiscover(t *testing.T) {
 	if dir.RevokeURL != revoke {
 		t.Errorf("dir.RevokeURL = %q; want %q", dir.RevokeURL, revoke)
 	}
+	if _, exist := c.nonces["testnonce"]; !exist {
+		t.Errorf("c.nonces = %q; want 'testnonce' in the map", c.nonces)
+	}
 }
 
 func TestRegister(t *testing.T) {
@@ -147,7 +151,11 @@ func TestRegister(t *testing.T) {
 		return false
 	}
 
-	c := Client{Key: testKeyEC, dir: &Directory{RegURL: ts.URL}}
+	c := Client{
+		Key:          testKeyEC,
+		DirectoryURL: ts.URL,
+		dir:          &Directory{RegURL: ts.URL},
+	}
 	a := &Account{Contact: contacts}
 	var err error
 	if a, err = c.Register(context.Background(), a, prompt); err != nil {
@@ -351,7 +359,11 @@ func TestAuthorize(t *testing.T) {
 				auth *Authorization
 				err  error
 			)
-			cl := Client{Key: testKeyEC, dir: &Directory{AuthzURL: ts.URL}}
+			cl := Client{
+				Key:          testKeyEC,
+				DirectoryURL: ts.URL,
+				dir:          &Directory{AuthzURL: ts.URL},
+			}
 			switch test.typ {
 			case "dns":
 				auth, err = cl.Authorize(context.Background(), test.value)
@@ -422,7 +434,11 @@ func TestAuthorizeValid(t *testing.T) {
 		w.Write([]byte(`{"status":"valid"}`))
 	}))
 	defer ts.Close()
-	client := Client{Key: testKey, dir: &Directory{AuthzURL: ts.URL}}
+	client := Client{
+		Key:          testKey,
+		DirectoryURL: ts.URL,
+		dir:          &Directory{AuthzURL: ts.URL},
+	}
 	_, err := client.Authorize(context.Background(), "example.com")
 	if err != nil {
 		t.Errorf("err = %v", err)
@@ -1037,6 +1053,53 @@ func TestNonce_fetchError(t *testing.T) {
 	}
 }
 
+func TestNonce_popWhenEmpty(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != "HEAD" {
+			t.Errorf("r.Method = %q; want HEAD", r.Method)
+		}
+		switch r.URL.Path {
+		case "/dir-with-nonce":
+			w.Header().Set("Replay-Nonce", "dirnonce")
+		case "/new-nonce":
+			w.Header().Set("Replay-Nonce", "newnonce")
+		case "/dir-no-nonce", "/empty":
+			// No nonce in the header.
+		default:
+			t.Errorf("Unknown URL: %s", r.URL)
+		}
+	}))
+	defer ts.Close()
+	ctx := context.Background()
+
+	tt := []struct {
+		dirURL, popURL, nonce string
+		wantOK                bool
+	}{
+		{ts.URL + "/dir-with-nonce", ts.URL + "/new-nonce", "dirnonce", true},
+		{ts.URL + "/dir-no-nonce", ts.URL + "/new-nonce", "newnonce", true},
+		{ts.URL + "/dir-no-nonce", ts.URL + "/empty", "", false},
+	}
+	for _, test := range tt {
+		t.Run(fmt.Sprintf("nonce:%s wantOK:%v", test.nonce, test.wantOK), func(t *testing.T) {
+			c := Client{DirectoryURL: test.dirURL}
+			v, err := c.popNonce(ctx, test.popURL)
+			if !test.wantOK {
+				if err == nil {
+					t.Fatalf("c.popNonce(%q) returned nil error", test.popURL)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("c.popNonce(%q): %v", test.popURL, err)
+			}
+			if v != test.nonce {
+				t.Errorf("c.popNonce(%q) = %q; want %q", test.popURL, v, test.nonce)
+			}
+		})
+	}
+}
+
 func TestNonce_postJWS(t *testing.T) {
 	var count int
 	seen := make(map[string]bool)
@@ -1070,7 +1133,11 @@ func TestNonce_postJWS(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	client := Client{Key: testKey, dir: &Directory{AuthzURL: ts.URL}}
+	client := Client{
+		Key:          testKey,
+		DirectoryURL: ts.URL, // nonces are fetched from here first
+		dir:          &Directory{AuthzURL: ts.URL},
+	}
 	if _, err := client.Authorize(context.Background(), "example.com"); err != nil {
 		t.Errorf("client.Authorize 1: %v", err)
 	}

acme/http_test.go

@@ -106,7 +106,11 @@ func TestPostWithRetries(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	client := &Client{Key: testKey, dir: &Directory{AuthzURL: ts.URL}}
+	client := &Client{
+		Key:          testKey,
+		DirectoryURL: ts.URL,
+		dir:          &Directory{AuthzURL: ts.URL},
+	}
 	// This call will fail with badNonce, causing a retry
 	if _, err := client.Authorize(context.Background(), "example.com"); err != nil {
 		t.Errorf("client.Authorize 1: %v", err)