diff --git a/mocks/mocks.go b/mocks/mocks.go index c4b72a39767..09170653fc1 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "net" + "strings" "time" "github.com/jmhodges/clock" @@ -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) @@ -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) diff --git a/wfe2/verify.go b/wfe2/verify.go index a2d9d2df4ad..f2af03cc8fa 100644 --- a/wfe2/verify.go +++ b/wfe2/verify.go @@ -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") } @@ -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, @@ -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 diff --git a/wfe2/verify_test.go b/wfe2/verify_test.go index 95fa082e17a..ad8c6155ebb 100644 --- a/wfe2/verify_test.go +++ b/wfe2/verify_test.go @@ -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 } diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 25b4ce1c04d..a7bd63beab6 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -295,20 +295,30 @@ func (wfe *WebFrontEndImpl) relativeDirectory(request *http.Request, directory m // various ACME-specified paths. func (wfe *WebFrontEndImpl) Handler() http.Handler { m := http.NewServeMux() + // Boulder specific endpoints + wfe.HandleFunc(m, issuerPath, wfe.Issuer, "GET") + wfe.HandleFunc(m, buildIDPath, wfe.BuildID, "GET") + + // GETable ACME endpoints wfe.HandleFunc(m, directoryPath, wfe.Directory, "GET") + wfe.HandleFunc(m, newNoncePath, wfe.Nonce, "GET") + + // POSTable ACME endpoints wfe.HandleFunc(m, newAcctPath, wfe.NewAccount, "POST") wfe.HandleFunc(m, acctPath, wfe.Account, "POST") - wfe.HandleFunc(m, authzPath, wfe.Authorization, "GET", "POST") - wfe.HandleFunc(m, challengePath, wfe.Challenge, "GET", "POST") - wfe.HandleFunc(m, certPath, wfe.Certificate, "GET") wfe.HandleFunc(m, revokeCertPath, wfe.RevokeCertificate, "POST") - wfe.HandleFunc(m, issuerPath, wfe.Issuer, "GET") - wfe.HandleFunc(m, buildIDPath, wfe.BuildID, "GET") wfe.HandleFunc(m, rolloverPath, wfe.KeyRollover, "POST") - wfe.HandleFunc(m, newNoncePath, wfe.Nonce, "GET") wfe.HandleFunc(m, newOrderPath, wfe.NewOrder, "POST") - wfe.HandleFunc(m, orderPath, wfe.GetOrder, "GET") wfe.HandleFunc(m, finalizeOrderPath, wfe.FinalizeOrder, "POST") + + // POST-as-GETable ACME endpoints + // TODO(@cpu): After November 1st, 2019 support for "GET" to the following + // endpoints will be removed, leaving only POST-as-GET support. + wfe.HandleFunc(m, orderPath, wfe.GetOrder, "GET", "POST") + wfe.HandleFunc(m, authzPath, wfe.Authorization, "GET", "POST") + wfe.HandleFunc(m, challengePath, wfe.Challenge, "GET", "POST") + wfe.HandleFunc(m, certPath, wfe.Certificate, "GET", "POST") + // We don't use our special HandleFunc for "/" because it matches everything, // meaning we can wind up returning 405 when we mean to return 404. See // https://github.com/letsencrypt/boulder/issues/717 @@ -965,6 +975,14 @@ func (wfe *WebFrontEndImpl) postChallenge( return } + // If the JWS body is empty then this POST is a POST-as-GET to retreive + // challenge details, not a POST to initiate a challenge + if string(body) == "" { + challenge := authz.Challenges[challengeIndex] + wfe.getChallenge(ctx, response, request, authz, &challenge, logEvent) + return + } + // NOTE(@cpu): Historically a challenge update needed to include // a KeyAuthorization field. This is no longer the case, since both sides can // calculate the key authorization as needed. We unmarshal here only to check @@ -1032,6 +1050,45 @@ func (wfe *WebFrontEndImpl) Account( return } + // If the body was not empty, then this is an account update request. + if string(body) != "" { + currAcct, prob = wfe.updateAccount(ctx, body, currAcct) + if prob != nil { + wfe.sendError(response, logEvent, prob, nil) + return + } + } + + if len(wfe.SubscriberAgreementURL) > 0 { + response.Header().Add("Link", link(wfe.SubscriberAgreementURL, "terms-of-service")) + } + + // We populate the account Agreement field when creating a new response to + // track which terms-of-service URL was in effect when an account with + // "termsOfServiceAgreed":"true" is created. That said, we don't want to send + // this value back to a V2 client. The "Agreement" field of an + // account/registration is a V1 notion so we strip it here in the WFE2 before + // returning the account. + currAcct.Agreement = "" + + err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, currAcct) + if err != nil { + // ServerInternal because we just generated the account, it should be OK + wfe.sendError(response, logEvent, + probs.ServerInternal("Failed to marshal account"), err) + return + } +} + +// updateAccount unmarshals an account update request from the provided +// requestBody to update the given registration. Important: It is assumed the +// request has already been authenticated by the caller. If the request is +// a valid update the resulting updated account is returned, otherwise a problem +// is returned. +func (wfe *WebFrontEndImpl) updateAccount( + ctx context.Context, + requestBody []byte, + currAcct *core.Registration) (*core.Registration, *probs.ProblemDetails) { // Only the Contact and Status fields of an account may be updated this way. // For key updates clients should be using the key change endpoint. var accountUpdateRequest struct { @@ -1039,10 +1096,9 @@ func (wfe *WebFrontEndImpl) Account( Status core.AcmeStatus `json:"status"` } - err = json.Unmarshal(body, &accountUpdateRequest) + err := json.Unmarshal(requestBody, &accountUpdateRequest) if err != nil { - wfe.sendError(response, logEvent, probs.Malformed("Error unmarshaling account"), err) - return + return nil, probs.Malformed("Error unmarshaling account") } // Copy over the fields from the request to the registration object used for @@ -1062,11 +1118,13 @@ func (wfe *WebFrontEndImpl) Account( // return before an update would be performed. if update.Status != "" && update.Status != currAcct.Status { if update.Status != core.StatusDeactivated { - wfe.sendError(response, logEvent, probs.Malformed("Invalid value provided for status field"), nil) - return + return nil, probs.Malformed("Invalid value provided for status field") } - wfe.deactivateAccount(ctx, *currAcct, response, request, logEvent) - return + if err := wfe.RA.DeactivateRegistration(ctx, *currAcct); err != nil { + return nil, web.ProblemDetailsForError(err, "Unable to deactivate account") + } + currAcct.Status = core.StatusDeactivated + return currAcct, nil } // Account objects contain a JWK object which are merged in UpdateRegistration @@ -1078,49 +1136,22 @@ func (wfe *WebFrontEndImpl) Account( updatedAcct, err := wfe.RA.UpdateRegistration(ctx, *currAcct, update) if err != nil { - wfe.sendError(response, logEvent, - web.ProblemDetailsForError(err, "Unable to update account"), err) - return - } - - if len(wfe.SubscriberAgreementURL) > 0 { - response.Header().Add("Link", link(wfe.SubscriberAgreementURL, "terms-of-service")) - } - - // We populate the account Agreement field when creating a new response to - // track which terms-of-service URL was in effect when an account with - // "termsOfServiceAgreed":"true" is created. That said, we don't want to send - // this value back to a V2 client. The "Agreement" field of an - // account/registration is a V1 notion so we strip it here in the WFE2 before - // returning the account. - updatedAcct.Agreement = "" - - err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, updatedAcct) - if err != nil { - // ServerInternal because we just generated the account, it should be OK - wfe.sendError(response, logEvent, - probs.ServerInternal("Failed to marshal account"), err) - return + return nil, web.ProblemDetailsForError(err, "Unable to update account") } + return &updatedAcct, nil } +// deactivateAuthorization processes the given JWS POST body as a request to +// deactivate the provided authorization. If an error occurs it is written to +// the response writer. Important: `deactivateAuthorization` does not check that +// the requester is authorized to deactivate the given authorization. It is +// assumed that this check is performed prior to calling deactivateAuthorzation. func (wfe *WebFrontEndImpl) deactivateAuthorization( ctx context.Context, authz *core.Authorization, logEvent *web.RequestEvent, response http.ResponseWriter, - request *http.Request) bool { - body, _, acct, prob := wfe.validPOSTForAccount(request, ctx, logEvent) - addRequesterHeader(response, logEvent.Requester) - if prob != nil { - wfe.sendError(response, logEvent, prob, nil) - return false - } - if acct.ID != authz.RegistrationID { - wfe.sendError(response, logEvent, - probs.Unauthorized("Account ID doesn't match ID for authorization"), nil) - return false - } + body []byte) bool { var req struct { Status core.AcmeStatus } @@ -1148,6 +1179,23 @@ func (wfe *WebFrontEndImpl) deactivateAuthorization( // Authorization is used by clients to submit an update to one of their // authorizations. func (wfe *WebFrontEndImpl) Authorization(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { + var requestAccount *core.Registration + var requestBody []byte + // If the request is a POST it is either: + // A) an update to an authorization to deactivate it + // B) a POST-as-GET to query the authorization details + if request.Method == "POST" { + // Both POST options need to be authenticated by an account + body, _, acct, prob := wfe.validPOSTForAccount(request, ctx, logEvent) + addRequesterHeader(response, logEvent.Requester) + if prob != nil { + wfe.sendError(response, logEvent, prob, nil) + return + } + requestAccount = acct + requestBody = body + } + // Requests to this handler should have a path that leads to a known authz id := request.URL.Path authz, err := wfe.SA.GetAuthorization(ctx, id) @@ -1168,11 +1216,22 @@ func (wfe *WebFrontEndImpl) Authorization(ctx context.Context, logEvent *web.Req return } - if wfe.AllowAuthzDeactivation && request.Method == "POST" { + // If this was a POST that has an associated requestAccount and that account + // doesn't own the authorization, abort before trying to deactivate the authz + // or return its details + if requestAccount != nil && requestAccount.ID != authz.RegistrationID { + wfe.sendError(response, logEvent, + probs.Unauthorized("Account ID doesn't match ID for authorization"), nil) + return + } + + // If the body isn't empty we know it isn't a POST-as-GET and must be an + // attempt to deactivate an authorization. + if string(requestBody) != "" && wfe.AllowAuthzDeactivation { // If the deactivation fails return early as errors and return codes // have already been set. Otherwise continue so that the user gets // sent the deactivated authorization. - if !wfe.deactivateAuthorization(ctx, &authz, logEvent, response, request) { + if !wfe.deactivateAuthorization(ctx, &authz, logEvent, response, requestBody) { return } } @@ -1192,6 +1251,17 @@ var allHex = regexp.MustCompile("^[0-9a-f]+$") // Certificate is used by clients to request a copy of their current certificate, or to // request a reissuance of the certificate. func (wfe *WebFrontEndImpl) Certificate(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { + var requesterAccount *core.Registration + // Any POSTs to the Certificate endpoint should be POST-as-GET requests. There are + // no POSTs with a body allowed for this endpoint. + if request.Method == "POST" { + acct, prob := wfe.validPOSTAsGETForAccount(request, ctx, logEvent) + if prob != nil { + wfe.sendError(response, logEvent, prob, nil) + return + } + requesterAccount = acct + } serial := request.URL.Path // Certificate paths consist of the CertBase path, plus exactly sixteen hex @@ -1219,6 +1289,14 @@ func (wfe *WebFrontEndImpl) Certificate(ctx context.Context, logEvent *web.Reque return } + // If there was a requesterAccount (e.g. because it was a POST-as-GET request) + // then the requesting account must be the owner of the certificate, otherwise + // return an unauthorized error. + if requesterAccount != nil && requesterAccount.ID != cert.RegistrationID { + wfe.sendError(response, logEvent, probs.Unauthorized("Account in use did not issue specified certificate"), nil) + return + } + leafPEM := pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: cert.DER, @@ -1450,29 +1528,6 @@ func (wfe *WebFrontEndImpl) KeyRollover( } } -func (wfe *WebFrontEndImpl) deactivateAccount( - ctx context.Context, - acct core.Registration, - response http.ResponseWriter, - request *http.Request, - logEvent *web.RequestEvent) { - err := wfe.RA.DeactivateRegistration(ctx, acct) - if err != nil { - wfe.sendError(response, logEvent, - web.ProblemDetailsForError(err, "Error deactivating account"), err) - return - } - acct.Status = core.StatusDeactivated - - err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, acct) - if err != nil { - // ServerInternal because account is from DB and should be fine - wfe.sendError(response, logEvent, - probs.ServerInternal("Failed to marshal account"), err) - return - } -} - type orderJSON struct { Status core.AcmeStatus `json:"status"` Expires time.Time `json:"expires"` @@ -1597,6 +1652,18 @@ func (wfe *WebFrontEndImpl) NewOrder( // GetOrder is used to retrieve a existing order object func (wfe *WebFrontEndImpl) GetOrder(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { + var requesterAccount *core.Registration + // Any POSTs to the Order endpoint should be POST-as-GET requests. There are + // no POSTs with a body allowed for this endpoint. + if request.Method == "POST" { + acct, prob := wfe.validPOSTAsGETForAccount(request, ctx, logEvent) + if prob != nil { + wfe.sendError(response, logEvent, prob, nil) + return + } + requesterAccount = acct + } + // Path prefix is stripped, so this should be like "/" fields := strings.SplitN(request.URL.Path, "/", 2) if len(fields) != 2 { @@ -1629,6 +1696,14 @@ func (wfe *WebFrontEndImpl) GetOrder(ctx context.Context, logEvent *web.RequestE return } + // If the requesterAccount is not nil then this was an authenticated + // POST-as-GET request and we need to verify the requesterAccount is the + // order's owner. + if requesterAccount != nil && *order.RegistrationID != requesterAccount.ID { + wfe.sendError(response, logEvent, probs.NotFound("No order found for account ID %d", acctID), nil) + return + } + respObj := wfe.orderToOrderJSON(request, order) err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, respObj) if err != nil { diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index 8be975b93cf..49a30f37649 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -875,20 +875,23 @@ func TestHTTPMethods(t *testing.T) { Path: acctPath, Allowed: postOnly, }, + // TODO(@cpu): Remove GET authz support, support only POST-as-GET { Name: "Authz path should be GET or POST only", Path: authzPath, Allowed: getOrPost, }, + // TODO(@cpu): Remove GET challenge support, support only POST-as-GET { Name: "Challenge path should be GET or POST only", Path: challengePath, Allowed: getOrPost, }, + // TODO(@cpu): Remove GET certificate support, support only POST-as-GET { - Name: "Certificate path should be GET only", + Name: "Certificate path should be GET or POST only", Path: certPath, - Allowed: getOnly, + Allowed: getOrPost, }, { Name: "RevokeCert path should be POST only", @@ -915,6 +918,7 @@ func TestHTTPMethods(t *testing.T) { Path: newOrderPath, Allowed: postOnly, }, + // TODO(@cpu): Remove GET order support, support only POST-as-GET { Name: "Order path should be GET or POST only", Path: orderPath, @@ -1016,18 +1020,28 @@ func TestGetChallenge(t *testing.T) { func TestChallenge(t *testing.T) { wfe, _ := setupWFE(t) + post := func(path string) *http.Request { + signedURL := fmt.Sprintf("http://localhost/%s", path) + _, _, jwsBody := signRequestKeyID(t, 1, nil, signedURL, `{}`, wfe.nonceService) + return makePostRequestWithPath(path, jwsBody) + } + postAsGet := func(keyID int64, path, body string) *http.Request { + _, _, jwsBody := signRequestKeyID(t, keyID, nil, fmt.Sprintf("http://localhost/%s", path), body, wfe.nonceService) + return makePostRequestWithPath(path, jwsBody) + } + // See mocks/mocks.go StorageAuthority.GetAuthorization for the "expired/" // "error_result/" path handling. testCases := []struct { Name string - Path string + Request *http.Request ExpectedStatus int ExpectedHeaders map[string]string ExpectedBody string }{ { Name: "Valid challenge", - Path: "valid/23", + Request: post("valid/23"), ExpectedStatus: http.StatusOK, ExpectedHeaders: map[string]string{ "Location": "http://localhost/acme/challenge/valid/23", @@ -1037,34 +1051,40 @@ func TestChallenge(t *testing.T) { }, { Name: "Expired challenge", - Path: "expired/23", + Request: post("expired/23"), ExpectedStatus: http.StatusNotFound, ExpectedBody: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Expired authorization","status":404}`, }, { Name: "Missing challenge", - Path: "", + Request: post(""), ExpectedStatus: http.StatusNotFound, ExpectedBody: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"No such challenge","status":404}`, }, { Name: "Unspecified database error", - Path: "error_result/24", + Request: post("error_result/24"), ExpectedStatus: http.StatusInternalServerError, ExpectedBody: `{"type":"` + probs.V2ErrorNS + `serverInternal","detail":"Problem getting authorization","status":500}`, }, + { + Name: "POST-as-GET, wrong owner", + Request: postAsGet(1, "diff_acct/23", ""), + ExpectedStatus: http.StatusForbidden, + ExpectedBody: `{"type":"` + probs.V2ErrorNS + `unauthorized","detail":"User account ID doesn't match account ID in authorization","status":403}`, + }, + { + Name: "Valid POST-as-GET", + Request: postAsGet(1, "valid/23", ""), + ExpectedStatus: http.StatusOK, + ExpectedBody: `{"type":"dns", "url": "http://localhost/acme/challenge/valid/23"}`, + }, } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { responseWriter := httptest.NewRecorder() - - // Make a signed request to the Challenge endpoint - signedURL := fmt.Sprintf("http://localhost/%s", tc.Path) - _, _, jwsBody := signRequestKeyID(t, 1, nil, signedURL, `{}`, wfe.nonceService) - request := makePostRequestWithPath(tc.Path, jwsBody) - wfe.Challenge(ctx, newRequestEvent(), responseWriter, request) - + wfe.Challenge(ctx, newRequestEvent(), responseWriter, tc.Request) // Check the reponse code, headers and body match expected headers := responseWriter.Header() body := responseWriter.Body.String() @@ -1352,6 +1372,30 @@ func TestGetAuthorization(t *testing.T) { }) test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{"type":"`+probs.V2ErrorNS+`malformed","detail":"No such authorization","status":404}`) + + _, _, jwsBody := signRequestKeyID(t, 1, nil, "http://localhost/valid", "", wfe.nonceService) + postAsGet := makePostRequestWithPath("http://localhost/valid", jwsBody) + + responseWriter = httptest.NewRecorder() + // Ensure that a POST-as-GET to an authorization works + wfe.Authorization(ctx, newRequestEvent(), responseWriter, postAsGet) + test.AssertEquals(t, responseWriter.Code, http.StatusOK) + body := responseWriter.Body.String() + test.AssertUnmarshaledEquals(t, body, ` + { + "identifier": { + "type": "dns", + "value": "not-an-example.com" + }, + "status": "valid", + "expires": "2070-01-01T00:00:00Z", + "challenges": [ + { + "type": "dns", + "url": "http://localhost/acme/challenge/valid/23" + } + ] + }`) } // An SA mock that always returns a berrors.ServerInternal error for @@ -1503,6 +1547,32 @@ func TestAccount(t *testing.T) { test.AssertContains(t, responseWriter.Body.String(), "400") test.AssertContains(t, responseWriter.Body.String(), probs.V2ErrorNS+"malformed") responseWriter.Body.Reset() + + // Test valid POST-as-GET request + responseWriter = httptest.NewRecorder() + _, _, body = signRequestKeyID(t, 1, nil, "http://localhost/1", "", wfe.nonceService) + request = makePostRequestWithPath("1", body) + wfe.Account(ctx, newRequestEvent(), responseWriter, request) + // It should not error + test.AssertNotContains(t, responseWriter.Body.String(), probs.V2ErrorNS) + test.AssertEquals(t, responseWriter.Code, http.StatusOK) + + altKey := loadKey(t, []byte(test2KeyPrivatePEM)) + _, ok = altKey.(*rsa.PrivateKey) + test.Assert(t, ok, "Couldn't load altKey RSA key") + + // Test POST-as-GET request signed with wrong account key + responseWriter = httptest.NewRecorder() + _, _, body = signRequestKeyID(t, 2, altKey, "http://localhost/1", "", wfe.nonceService) + request = makePostRequestWithPath("1", body) + wfe.Account(ctx, newRequestEvent(), responseWriter, request) + // It should error + test.AssertEquals(t, responseWriter.Code, http.StatusForbidden) + test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{ + "type": "urn:ietf:params:acme:error:unauthorized", + "detail": "Request signing key did not match account key", + "status": 403 + }`) } func TestIssuer(t *testing.T) { @@ -1522,6 +1592,19 @@ func TestGetCertificate(t *testing.T) { wfe, _ := setupWFE(t) mux := wfe.Handler() + makeGet := func(path string) *http.Request { + return &http.Request{URL: &url.URL{Path: path}, Method: "GET"} + } + + makePost := func(keyID int64, key interface{}, path, body string) *http.Request { + _, _, jwsBody := signRequestKeyID(t, keyID, key, fmt.Sprintf("http://localhost%s", path), body, wfe.nonceService) + return makePostRequestWithPath(path, jwsBody) + } + + altKey := loadKey(t, []byte(test2KeyPrivatePEM)) + _, ok := altKey.(*rsa.PrivateKey) + test.Assert(t, ok, "Couldn't load RSA key") + certPemBytes, _ := ioutil.ReadFile("test/178.crt") pkixContent := "application/pem-certificate-chain" @@ -1534,7 +1617,7 @@ func TestGetCertificate(t *testing.T) { testCases := []struct { Name string - Path string + Request *http.Request ExpectedStatus int ExpectedHeaders map[string]string ExpectedBody string @@ -1542,28 +1625,57 @@ func TestGetCertificate(t *testing.T) { }{ { Name: "Valid serial", - Path: goodSerial, + Request: makeGet(goodSerial), ExpectedStatus: http.StatusOK, ExpectedHeaders: map[string]string{ "Content-Type": pkixContent, }, ExpectedCert: append(certPemBytes, append([]byte("\n"), chainPemBytes...)...), }, + { + Name: "Valid serial, POST-as-GET", + Request: makePost(1, nil, goodSerial, ""), + ExpectedStatus: http.StatusOK, + ExpectedHeaders: map[string]string{ + "Content-Type": pkixContent, + }, + ExpectedCert: append(certPemBytes, append([]byte("\n"), chainPemBytes...)...), + }, + { + Name: "Valid serial, bad POST-as-GET", + Request: makePost(1, nil, goodSerial, "{}"), + ExpectedStatus: http.StatusBadRequest, + ExpectedBody: `{ + "type": "urn:ietf:params:acme:error:malformed", + "status": 400, + "detail": "POST-as-GET requests must have an empty payload" + }`, + }, + { + Name: "Valid serial, POST-as-GET from wrong account", + Request: makePost(2, altKey, goodSerial, ""), + ExpectedStatus: http.StatusForbidden, + ExpectedBody: `{ + "type": "urn:ietf:params:acme:error:unauthorized", + "status": 403, + "detail": "Account in use did not issue specified certificate" + }`, + }, { Name: "Unused serial, no cache", - Path: "/acme/cert/0000000000000000000000000000000000ff", + Request: makeGet("/acme/cert/0000000000000000000000000000000000ff"), ExpectedStatus: http.StatusNotFound, ExpectedBody: notFound, }, { Name: "Invalid serial, no cache", - Path: "/acme/cert/nothex", + Request: makeGet("/acme/cert/nothex"), ExpectedStatus: http.StatusNotFound, ExpectedBody: notFound, }, { Name: "Another invalid serial, no cache", - Path: "/acme/cert/00000000000000", + Request: makeGet("/acme/cert/00000000000000"), ExpectedStatus: http.StatusNotFound, ExpectedBody: notFound, }, @@ -1576,9 +1688,7 @@ func TestGetCertificate(t *testing.T) { mockLog.Clear() // Mux a request for a certificate - req, _ := http.NewRequest("GET", tc.Path, nil) - req.RemoteAddr = "192.168.0.1" - mux.ServeHTTP(responseWriter, req) + mux.ServeHTTP(responseWriter, tc.Request) headers := responseWriter.Header() // Assert that the status code written is as expected @@ -1609,10 +1719,10 @@ func TestGetCertificate(t *testing.T) { test.AssertUnmarshaledEquals(t, body, tc.ExpectedBody) // Unsuccessful requests should be logged as such - reqlogs := mockLog.GetAllMatching(`INFO: JSON=.*"Code":404.*`) + reqlogs := mockLog.GetAllMatching(fmt.Sprintf(`INFO: JSON=.*"Code":%d.*`, tc.ExpectedStatus)) if len(reqlogs) != 1 { - t.Errorf("Didn't find info logs with code 404. Instead got:\n%s\n", - strings.Join(mockLog.GetAllMatching(`.*`), "\n")) + t.Errorf("Didn't find info logs with code %d. Instead got:\n%s\n", + tc.ExpectedStatus, strings.Join(mockLog.GetAllMatching(`.*`), "\n")) } } }) @@ -1759,7 +1869,6 @@ func TestDeactivateAccount(t *testing.T) { "contact": [ "mailto:person@mail.com" ], - "agreement": "http://example.invalid/terms", "initialIp": "", "createdAt": "0001-01-01T00:00:00Z", "status": "deactivated" @@ -1782,7 +1891,6 @@ func TestDeactivateAccount(t *testing.T) { "contact": [ "mailto:person@mail.com" ], - "agreement": "http://example.invalid/terms", "initialIp": "", "createdAt": "0001-01-01T00:00:00Z", "status": "deactivated" @@ -2289,55 +2397,79 @@ func TestKeyRollover(t *testing.T) { } } -func TestOrder(t *testing.T) { +func TestGetOrder(t *testing.T) { wfe, _ := setupWFE(t) + makeGet := func(path string) *http.Request { + return &http.Request{URL: &url.URL{Path: path}, Method: "GET"} + } + + makePost := func(keyID int64, path, body string) *http.Request { + _, _, jwsBody := signRequestKeyID(t, keyID, nil, fmt.Sprintf("http://localhost/%s", path), body, wfe.nonceService) + return makePostRequestWithPath(path, jwsBody) + } + testCases := []struct { Name string - Path string + Request *http.Request Response string }{ { Name: "Good request", - Path: "1/1", + Request: makeGet("1/1"), Response: `{"status": "valid","expires": "1970-01-01T00:00:00.9466848Z","identifiers":[{"type":"dns", "value":"example.com"}], "authorizations":["http://localhost/acme/authz/hello"],"finalize":"http://localhost/acme/finalize/1/1","certificate":"http://localhost/acme/cert/serial"}`, }, { Name: "404 request", - Path: "1/2", + Request: makeGet("1/2"), Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"No order for ID 2", "status":404}`, }, { Name: "Invalid request path", - Path: "asd", + Request: makeGet("asd"), Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Invalid request path","status":404}`, }, { Name: "Invalid account ID", - Path: "asd/asd", + Request: makeGet("asd/asd"), Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Invalid account ID","status":400}`, }, { Name: "Invalid order ID", - Path: "1/asd", + Request: makeGet("1/asd"), Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Invalid order ID","status":400}`, }, { Name: "Real request, wrong account", - Path: "2/1", + Request: makeGet("2/1"), Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"No order found for account ID 2", "status":404}`, }, { Name: "Internal error request", - Path: "1/3", + Request: makeGet("1/3"), Response: `{"type":"` + probs.V2ErrorNS + `serverInternal","detail":"Failed to retrieve order for ID 3","status":500}`, }, + { + Name: "Invalid POST-as-GET", + Request: makePost(1, "1/1", "{}"), + Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"POST-as-GET requests must have an empty payload", "status":400}`, + }, + { + Name: "Valid POST-as-GET, wrong account", + Request: makePost(1, "2/1", ""), + Response: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"No order found for account ID 2", "status":404}`, + }, + { + Name: "Valid POST-as-GET", + Request: makePost(1, "1/1", ""), + Response: `{"status": "valid","expires": "1970-01-01T00:00:00.9466848Z","identifiers":[{"type":"dns", "value":"example.com"}], "authorizations":["http://localhost/acme/authz/hello"],"finalize":"http://localhost/acme/finalize/1/1","certificate":"http://localhost/acme/cert/serial"}`, + }, } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { responseWriter := httptest.NewRecorder() - wfe.GetOrder(ctx, newRequestEvent(), responseWriter, &http.Request{URL: &url.URL{Path: tc.Path}, Method: "GET"}) + wfe.GetOrder(ctx, newRequestEvent(), responseWriter, tc.Request) test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), tc.Response) }) }