Check authorization status at the WFE/2 instead of RA

This commit is contained in:
Roland Shoemaker 2018-11-13 12:04:56 -08:00
parent 465be64f3f
commit ad1843c47f
4 changed files with 71 additions and 57 deletions

View File

@ -1072,28 +1072,39 @@ func (wfe *WebFrontEndImpl) postChallenge(
return
}
var challengeUpdate core.Challenge
if err := json.Unmarshal(body, &challengeUpdate); err != nil {
wfe.sendError(response, logEvent, probs.Malformed("Error unmarshaling challenge response"), err)
return
}
// We can expect some clients to try and update a challenge for an authorization
// that is already valid. In this case we don't need to process the challenge
// update. It wouldn't be helpful, the overall authorization is already good! We
// increment a stat for this case and return early.
var returnAuthz core.Authorization
if authz.Status == core.StatusValid {
// ra.stats.Inc("ReusedValidAuthzChallenge", 1)
returnAuthz = authz
} else {
var challengeUpdate core.Challenge
if err := json.Unmarshal(body, &challengeUpdate); err != nil {
wfe.sendError(response, logEvent, probs.Malformed("Error unmarshaling challenge response"), err)
return
}
// Ask the RA to update this authorization
updatedAuthorization, err := wfe.RA.UpdateAuthorization(ctx, authz, challengeIndex, challengeUpdate)
if err != nil {
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update challenge"), err)
return
// Ask the RA to update this authorization
var err error
returnAuthz, err = wfe.RA.UpdateAuthorization(ctx, authz, challengeIndex, challengeUpdate)
if err != nil {
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update challenge"), err)
return
}
}
// assumption: UpdateAuthorization does not modify order of challenges
challenge := updatedAuthorization.Challenges[challengeIndex]
challenge := returnAuthz.Challenges[challengeIndex]
wfe.prepChallengeForDisplay(request, authz, &challenge)
authzURL := web.RelativeEndpoint(request, authzPath+string(authz.ID))
response.Header().Add("Location", challenge.URI)
response.Header().Add("Link", link(authzURL, "up"))
err = wfe.writeJsonResponse(response, logEvent, http.StatusAccepted, challenge)
err := wfe.writeJsonResponse(response, logEvent, http.StatusAccepted, challenge)
if err != nil {
// ServerInternal because we made the challenges, they should be OK
wfe.sendError(response, logEvent, probs.ServerInternal("Failed to marshal challenge"), err)

View File

@ -1167,23 +1167,18 @@ func TestChallenge(t *testing.T) {
}
// MockRAStrictUpdateAuthz is a mock RA that enforces authz status in `UpdateAuthorization`
// MockRAUpdateAuthorizationError is a mock RA that just returns an error on UpdateAuthorization
type MockRAStrictUpdateAuthz struct {
MockRegistrationAuthority
}
// UpdateAuthorization for a MockRAStrictUpdateAuthz returns a
// berrors.WrongAuthorizationStateError when told to update a non-pending authz.
// It returns the authz unchanged for all other cases.
func (ra *MockRAStrictUpdateAuthz) UpdateAuthorization(_ context.Context, authz core.Authorization, _ int, _ core.Challenge) (core.Authorization, error) {
if authz.Status != core.StatusPending {
return core.Authorization{}, berrors.WrongAuthorizationStateError("authorization is not pending")
}
return authz, nil
return core.Authorization{}, errors.New("broken on purpose")
}
// TestUpdateChallengeFinalizedAuthz tests that POSTing a challenge associated
// with an already valid authorization returns the expected Malformed problem.
// with an already valid authorization just returns the challenge without calling
// the RA.
func TestUpdateChallengeFinalizedAuthz(t *testing.T) {
wfe, _ := setupWFE(t)
wfe.RA = &MockRAStrictUpdateAuthz{}
@ -1195,11 +1190,11 @@ func TestUpdateChallengeFinalizedAuthz(t *testing.T) {
signRequest(t, `{"resource":"challenge"}`, wfe.nonceService)))
body := responseWriter.Body.String()
fmt.Println(body)
test.AssertUnmarshaledEquals(t, body, `{
"type": "`+probs.V1ErrorNS+`malformed",
"detail": "Unable to update challenge :: authorization is not pending",
"status": 400
}`)
"type": "dns",
"uri": "http://localhost/acme/challenge/valid/23"
}`)
}
func TestBadNonce(t *testing.T) {

View File

@ -982,35 +982,47 @@ func (wfe *WebFrontEndImpl) postChallenge(
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
// that the POST body is valid JSON. Any data/fields included are ignored to
// be kind to ACMEv2 implementations that still send a key authorization.
var challengeUpdate struct{}
if err := json.Unmarshal(body, &challengeUpdate); err != nil {
wfe.sendError(response, logEvent, probs.Malformed("Error unmarshaling challenge response"), err)
return
}
// We can expect some clients to try and update a challenge for an authorization
// that is already valid. In this case we don't need to process the challenge
// update. It wouldn't be helpful, the overall authorization is already good! We
// increment a stat for this case and return early.
var returnAuthz core.Authorization
if authz.Status == core.StatusValid {
// ra.stats.Inc("ReusedValidAuthzChallenge", 1)
returnAuthz = authz
} else {
// Send the authorization to the RA for validation (the name of this RPC is somewhat
// misleading, the RA sends the authorization to the VA for validation. Once the validation
// is complete the VA returns back to the RA to finalize the authorization)
updatedAuthorization, err := wfe.RA.UpdateAuthorization(ctx, authz, challengeIndex, core.Challenge{})
if err != nil {
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update challenge"), err)
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
// that the POST body is valid JSON. Any data/fields included are ignored to
// be kind to ACMEv2 implementations that still send a key authorization.
var challengeUpdate struct{}
if err := json.Unmarshal(body, &challengeUpdate); err != nil {
wfe.sendError(response, logEvent, probs.Malformed("Error unmarshaling challenge response"), err)
return
}
// Send the authorization to the RA for validation (the name of this RPC is somewhat
// misleading, the RA sends the authorization to the VA for validation. Once the validation
// is complete the VA returns back to the RA to finalize the authorization)
var err error
returnAuthz, err = wfe.RA.UpdateAuthorization(ctx, authz, challengeIndex, core.Challenge{})
if err != nil {
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update challenge"), err)
return
}
}
// assumption: UpdateAuthorization does not modify order of challenges
challenge := updatedAuthorization.Challenges[challengeIndex]
challenge := returnAuthz.Challenges[challengeIndex]
wfe.prepChallengeForDisplay(request, authz, &challenge)
authzURL := web.RelativeEndpoint(request, authzPath+string(authz.ID))
response.Header().Add("Location", challenge.URL)
response.Header().Add("Link", link(authzURL, "up"))
err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, challenge)
err := wfe.writeJsonResponse(response, logEvent, http.StatusOK, challenge)
if err != nil {
// ServerInternal because we made the challenges, they should be OK
wfe.sendError(response, logEvent, probs.ServerInternal("Failed to marshal challenge"), err)

View File

@ -9,6 +9,7 @@ import (
"crypto/x509"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"io"
"io/ioutil"
@ -1097,23 +1098,18 @@ func TestChallenge(t *testing.T) {
}
}
// MockRAStrictUpdateAuthz is a mock RA that enforces authz status in `UpdateAuthorization`
// MockRAUpdateAuthorizationError is a mock RA that just returns an error on UpdateAuthorization
type MockRAStrictUpdateAuthz struct {
MockRegistrationAuthority
}
// UpdateAuthorization for a MockRAStrictUpdateAuthz returns a
// berrors.WrongAuthorizationStateError when told to update a non-pending authz. It
// returns the authz unchanged for all other cases.
func (ra *MockRAStrictUpdateAuthz) UpdateAuthorization(_ context.Context, authz core.Authorization, _ int, _ core.Challenge) (core.Authorization, error) {
if authz.Status != core.StatusPending {
return core.Authorization{}, berrors.WrongAuthorizationStateError("authorization is not pending")
}
return authz, nil
return core.Authorization{}, errors.New("broken on purpose")
}
// TestUpdateChallengeFinalizedAuthz tests that POSTing a challenge associated
// with an already valid authorization returns the expected Malformed problem.
// with an already valid authorization just returns the challenge without calling
// the RA.
func TestUpdateChallengeFinalizedAuthz(t *testing.T) {
wfe, _ := setupWFE(t)
wfe.RA = &MockRAStrictUpdateAuthz{}
@ -1125,11 +1121,11 @@ func TestUpdateChallengeFinalizedAuthz(t *testing.T) {
wfe.Challenge(ctx, newRequestEvent(), responseWriter, request)
body := responseWriter.Body.String()
fmt.Println(body)
test.AssertUnmarshaledEquals(t, body, `{
"type": "`+probs.V2ErrorNS+`malformed",
"detail": "Unable to update challenge :: authorization is not pending",
"status": 400
}`)
"type": "dns",
"url": "http://localhost/acme/challenge/valid/23"
}`)
}
func TestBadNonce(t *testing.T) {