From 1164f441bdeefa612d54e13c3966e4161399dcfa Mon Sep 17 00:00:00 2001
From: Daniel McCarney <daniel@binaryparadox.net>
Date: Mon, 29 Oct 2018 12:35:49 -0400
Subject: [PATCH] Client: Do not send a JWS body when POSTing challenges.
 (#689)

* Client: Do not send a JWS body when POSTing challenges.

In legacy ACME there was a requirement to send a JWS body that contained
a key authorization as part of all challenge initiation POSTs. Since
both the client and server can reconstitute the key authorization there
is no need to send it and modern ACME expects challenges to be initiated
with a JWS carrying the trivial empty JSON object (`{}`).  Some ACME
servers (e.g. Pebble in `-strict` mode) will reject all challenge POSTs
that have a legacy JWS body.

This commit updates the LEGO `acme/client.go`'s `validate` function to
send the correct JWS payload for challenge POSTs.
---
 acme/client.go      |  5 ++++-
 acme/client_test.go | 42 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/acme/client.go b/acme/client.go
index 4908d592..d8b2187e 100644
--- a/acme/client.go
+++ b/acme/client.go
@@ -914,7 +914,10 @@ func parseLinks(links []string) map[string]string {
 func validate(j *jws, domain, uri string, c challenge) error {
 	var chlng challenge
 
-	hdr, err := postJSON(j, uri, c, &chlng)
+	// Challenge initiation is done by sending a JWS payload containing the
+	// trivial JSON object `{}`. We use an empty struct instance as the postJSON
+	// payload here to achieve this result.
+	hdr, err := postJSON(j, uri, struct{}{}, &chlng)
 	if err != nil {
 		return err
 	}
diff --git a/acme/client_test.go b/acme/client_test.go
index e14444ee..51b57596 100644
--- a/acme/client_test.go
+++ b/acme/client_test.go
@@ -5,6 +5,8 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"encoding/json"
+	"fmt"
+	"io/ioutil"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -13,6 +15,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/square/go-jose.v2"
 )
 
 func TestNewClient(t *testing.T) {
@@ -149,6 +152,38 @@ func TestNotHoldingLockWhileMakingHTTPRequests(t *testing.T) {
 func TestValidate(t *testing.T) {
 	var statuses []string
 
+	privKey, err := rsa.GenerateKey(rand.Reader, 512)
+	require.NoError(t, err)
+
+	// validateNoBody reads the http.Request POST body, parses the JWS and
+	// validates it to read the body. If there is an error doing this, or if the
+	// JWS body is not the empty JSON payload "{}" an error is returned. We use
+	// this to verify challenge POSTs to the ts below do not send a JWS body.
+	validateNoBody := func(r *http.Request) error {
+		reqBody, err := ioutil.ReadAll(r.Body)
+		if err != nil {
+			return err
+		}
+
+		jws, err := jose.ParseSigned(string(reqBody))
+		if err != nil {
+			return err
+		}
+
+		body, err := jws.Verify(&jose.JSONWebKey{
+			Key:       privKey.Public(),
+			Algorithm: "RSA",
+		})
+		if err != nil {
+			return err
+		}
+
+		if bodyStr := string(body); bodyStr != "{}" {
+			return fmt.Errorf(`expected JWS POST body "{}", got %q`, bodyStr)
+		}
+		return nil
+	}
+
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Minimal stub ACME server for validation.
 		w.Header().Add("Replay-Nonce", "12345")
@@ -157,6 +192,11 @@ func TestValidate(t *testing.T) {
 		switch r.Method {
 		case http.MethodHead:
 		case http.MethodPost:
+			if err := validateNoBody(r); err != nil {
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return
+			}
+
 			st := statuses[0]
 			statuses = statuses[1:]
 			writeJSONResponse(w, &challenge{Type: "http-01", Status: st, URL: "http://example.com/", Token: "token"})
@@ -172,8 +212,6 @@ func TestValidate(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	privKey, err := rsa.GenerateKey(rand.Reader, 512)
-	require.NoError(t, err)
 	j := &jws{privKey: privKey, getNonceURL: ts.URL}
 
 	testCases := []struct {