Skip to content

Commit

Permalink
ACME v2: Optional POST-as-GET support. (#3883)
Browse files Browse the repository at this point in the history
This allows POST-as-GET requests to Orders, Authorizations, Challenges, Certificates and Accounts. Legacy GET support remains for Orders, Authorizations, Challenges and Certificates. Legacy "POST {}" support for Accounts remains.

Resolves #3871
  • Loading branch information
cpu committed Oct 22, 2018
1 parent 7fd3d30 commit 1398377
Show file tree
Hide file tree
Showing 5 changed files with 399 additions and 115 deletions.
9 changes: 9 additions & 0 deletions mocks/mocks.go
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io/ioutil"
"net"
"strings"
"time"

"github.com/jmhodges/clock"
Expand Down Expand Up @@ -247,6 +248,8 @@ func (sa *StorageAuthority) GetAuthorization(_ context.Context, id string) (core
},
},
}
// Strip a leading `/` to make working with fake URLs in signed JWS tests easier.
id = strings.TrimPrefix(id, "/")

if id == "valid" {
exp := sa.clk.Now().AddDate(100, 0, 0)
Expand All @@ -260,6 +263,12 @@ func (sa *StorageAuthority) GetAuthorization(_ context.Context, id string) (core
return authz, nil
} else if id == "error_result" {
return core.Authorization{}, fmt.Errorf("Unspecified database error")
} else if id == "diff_acct" {
exp := sa.clk.Now().AddDate(100, 0, 0)
authz.RegistrationID = 2
authz.Expires = &exp
authz.Challenges[0].URI = "http://localhost:4300/acme/challenge/valid/23"
return authz, nil
}

return core.Authorization{}, berrors.NotFoundError("no authorization found with id %q", id)
Expand Down
34 changes: 30 additions & 4 deletions wfe2/verify.go
Expand Up @@ -499,10 +499,10 @@ func (wfe *WebFrontEndImpl) validJWSForKey(
// In the WFE1 package the check for the request URL required unmarshalling
// the payload JSON to check the "resource" field of the protected JWS body.
// This caught invalid JSON early and so we preserve this check by explicitly
// trying to unmarshal the payload as part of the verification and failing
// early if it isn't valid JSON.
// trying to unmarshal the payload (when it is non-empty to allow POST-as-GET
// behaviour) as part of the verification and failing early if it isn't valid JSON.
var parsedBody struct{}
if err := json.Unmarshal(payload, &parsedBody); err != nil {
if err := json.Unmarshal(payload, &parsedBody); string(payload) != "" && err != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSBodyUnmarshalFailed"}).Inc()
return nil, probs.Malformed("Request payload did not parse as JSON")
}
Expand Down Expand Up @@ -538,7 +538,10 @@ func (wfe *WebFrontEndImpl) validJWSForAccount(
}

// validPOSTForAccount checks that a given POST request has a valid JWS
// using `validJWSForAccount`.
// using `validJWSForAccount`. If valid, the authenticated JWS body and the
// registration that authenticated the body are returned. Otherwise a problem is
// returned. The returned JWS body may be empty if the request is a POST-as-GET
// request.
func (wfe *WebFrontEndImpl) validPOSTForAccount(
request *http.Request,
ctx context.Context,
Expand All @@ -551,6 +554,29 @@ func (wfe *WebFrontEndImpl) validPOSTForAccount(
return wfe.validJWSForAccount(jws, request, ctx, logEvent)
}

// validPOSTAsGETForAccount checks that a given POST request is valid using
// `validPOSTForAccount`. It additionally validates that the JWS request payload
// is empty, indicating that it is a POST-as-GET request per ACME draft 15+
// section 6.3 "GET and POST-as-GET requests". If a non empty payload is
// provided in the JWS the invalidPOSTAsGETErr problem is returned. This
// function is useful only for endpoints that do not need to handle both POSTs
// with a body and POST-as-GET requests (e.g. Order, Certificate).
func (wfe *WebFrontEndImpl) validPOSTAsGETForAccount(
request *http.Request,
ctx context.Context,
logEvent *web.RequestEvent) (*core.Registration, *probs.ProblemDetails) {
// Call validPOSTForAccount to verify the JWS and extract the body.
body, _, reg, prob := wfe.validPOSTForAccount(request, ctx, logEvent)
if prob != nil {
return nil, prob
}
// Verify the POST-as-GET payload is empty
if string(body) != "" {
return nil, probs.Malformed("POST-as-GET requests must have an empty payload")
}
return reg, prob
}

// validSelfAuthenticatedJWS checks that a given JWS verifies with the JWK
// embedded in the JWS itself (e.g. self-authenticated). This type of JWS
// is only used for creating new accounts or revoking a certificate by signing
Expand Down
42 changes: 42 additions & 0 deletions wfe2/verify_test.go
Expand Up @@ -1337,6 +1337,48 @@ func TestValidPOSTForAccount(t *testing.T) {
}
}

// TestValidPOSTAsGETForAccount tests POST-as-GET processing. Because
// wfe.validPOSTAsGETForAccount calls `wfe.validPOSTForAccount` to do all
// processing except the empty body test we do not duplicate the
// `TestValidPOSTForAccount` testcases here.
func TestValidPOSTAsGETForAccount(t *testing.T) {
wfe, _ := setupWFE(t)

// an invalid POST-as-GET request contains a non-empty payload. In this case
// we test with the empty JSON payload ("{}")
_, _, invalidPayloadRequest := signRequestKeyID(t, 1, nil, "http://localhost/test", "{}", wfe.nonceService)
// a valid POST-as-GET request contains an empty payload.
_, _, validRequest := signRequestKeyID(t, 1, nil, "http://localhost/test", "", wfe.nonceService)

testCases := []struct {
Name string
Request *http.Request
ExpectedProblem *probs.ProblemDetails
}{
{
Name: "Non-empty JWS payload",
Request: makePostRequestWithPath("test", invalidPayloadRequest),
ExpectedProblem: probs.Malformed("POST-as-GET requests must have an empty payload"),
},
{
Name: "Valid POST-as-GET",
Request: makePostRequestWithPath("test", validRequest),
},
}

for _, tc := range testCases {
_, prob := wfe.validPOSTAsGETForAccount(
tc.Request,
context.Background(),
newRequestEvent())
if tc.ExpectedProblem == nil && prob != nil {
t.Fatalf("Expected nil problem, got %#v\n", prob)
} else if tc.ExpectedProblem != nil {
test.AssertMarshaledEquals(t, prob, tc.ExpectedProblem)
}
}
}

type mockSADifferentStoredKey struct {
core.StorageGetter
}
Expand Down

0 comments on commit 1398377

Please sign in to comment.