wfe2: Return Status 200 for HEAD to new-nonce endpoint. (#3992)

Previously we mistakenly returned status 204 (no content) for all
requests to new-nonce, including HEAD. This status should only be used
for GET requests.

When the `HeadNonceStatusOK` feature flag is enabled we will now return
the correct status for HEAD requests. When the flag is disabled we return
status 204 to preserve backwards compatibility.
This commit is contained in:
Daniel McCarney 2019-01-07 12:58:30 -05:00 committed by GitHub
parent 11433e1ea0
commit e5b8d530b7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 63 additions and 18 deletions

View File

@ -4,9 +4,9 @@ package features
import "strconv"
const _FeatureFlag_name = "unusedReusePendingAuthzCancelCTSubmissionsCountCertificatesExactIPv6FirstEnforceChallengeDisableEmbedSCTsWildcardDomainsForceConsistentStatusRPCHeadroomVAChecksGSBEnforceV2ContentTypeEnforceOverlappingWildcardsOrderReadyStatusAllowRenewalFirstRLTLSSNIRevalidationCAAValidationMethodsCAAAccountURIACME13KeyRolloverProbeCTLogsSimplifiedVAHTTPPerformValidationRPC"
const _FeatureFlag_name = "unusedReusePendingAuthzCancelCTSubmissionsCountCertificatesExactIPv6FirstEnforceChallengeDisableEmbedSCTsWildcardDomainsForceConsistentStatusRPCHeadroomVAChecksGSBEnforceV2ContentTypeEnforceOverlappingWildcardsOrderReadyStatusAllowRenewalFirstRLTLSSNIRevalidationCAAValidationMethodsCAAAccountURIACME13KeyRolloverProbeCTLogsSimplifiedVAHTTPPerformValidationRPCHeadNonceStatusOK"
var _FeatureFlag_index = [...]uint16{0, 6, 23, 42, 64, 73, 96, 105, 120, 141, 152, 163, 183, 210, 226, 245, 263, 283, 296, 313, 324, 340, 360}
var _FeatureFlag_index = [...]uint16{0, 6, 23, 42, 64, 73, 96, 105, 120, 141, 152, 163, 183, 210, 226, 245, 263, 283, 296, 313, 324, 340, 360, 377}
func (i FeatureFlag) String() string {
if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

View File

@ -44,6 +44,9 @@ const (
// PerformValidationRPC enables the WFE/WFE2 to use the RA's PerformValidation
// RPC instead of the deprecated UpdateAuthorization RPC.
PerformValidationRPC
// HEAD requests to the WFE2 new-nonce endpoint should return HTTP StatusOK
// instead of HTTP StatusNoContent.
HeadNonceStatusOK
)
// List of features and their default value, protected by fMu
@ -70,6 +73,7 @@ var features = map[FeatureFlag]bool{
ProbeCTLogs: false,
SimplifiedVAHTTP: false,
PerformValidationRPC: false,
HeadNonceStatusOK: false,
}
var fMu = new(sync.RWMutex)

View File

@ -39,7 +39,8 @@
"EnforceV2ContentType": true,
"RPCHeadroom": true,
"ACME13KeyRollover": true,
"PerformValidationRPC": true
"PerformValidationRPC": true,
"HeadNonceStatusOK": true
}
},

View File

@ -424,14 +424,22 @@ func (wfe *WebFrontEndImpl) Directory(
}
// Nonce is an endpoint for getting a fresh nonce with an HTTP GET or HEAD
// request. This endpoint only returns a no content header - the `HandleFunc`
// request. This endpoint only returns a status code header - the `HandleFunc`
// wrapper ensures that a nonce is written in the correct response header.
func (wfe *WebFrontEndImpl) Nonce(
ctx context.Context,
logEvent *web.RequestEvent,
response http.ResponseWriter,
request *http.Request) {
response.WriteHeader(http.StatusNoContent)
statusCode := http.StatusNoContent
// The ACME specification says GET requets should receive http.StatusNoContent
// and HEAD requests should receive http.StatusOK. We gate this with the
// HeadNonceStatusOK feature flag because it may break clients that are
// programmed to expect StatusOK.
if features.Enabled(features.HeadNonceStatusOK) && request.Method == "HEAD" {
statusCode = http.StatusOK
}
response.WriteHeader(statusCode)
}
// sendError wraps web.SendError

View File

@ -825,24 +825,56 @@ func TestRelativeDirectory(t *testing.T) {
}
}
// TestNonceEndpoint tests the WFE2's new-nonce endpoint
func TestNonceEndpoint(t *testing.T) {
// TestNonceEndpoint tests requests to the WFE2's new-nonce endpoint
func TestNonceEndpointGET(t *testing.T) {
wfe, _ := setupWFE(t)
mux := wfe.Handler()
responseWriter := httptest.NewRecorder()
testCases := []struct {
Name string
Method string
ExpectedStatus int
HeadNonceStatusOK bool
}{
{
Name: "GET new-nonce request",
Method: "GET",
ExpectedStatus: http.StatusNoContent,
},
{
Name: "HEAD new-nonce request (legacy)",
Method: "HEAD",
ExpectedStatus: http.StatusNoContent,
},
{
Name: "HEAD new-nonce request (feature flag)",
Method: "HEAD",
ExpectedStatus: http.StatusOK,
HeadNonceStatusOK: true,
},
}
mux.ServeHTTP(responseWriter, &http.Request{
Method: "GET",
URL: mustParseURL(newNoncePath),
})
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
if tc.HeadNonceStatusOK {
if err := features.Set(map[string]bool{"HeadNonceStatusOK": true}); err != nil {
t.Fatalf("Failed to enable HeadNonceStatusOK feature: %v", err)
}
defer features.Reset()
}
// Sending a GET request to the nonce endpoint should produce a HTTP response
// with the correct status code
test.AssertEquals(t, responseWriter.Code, http.StatusNoContent)
// And the response should contain a valid nonce in the Replay-Nonce header
nonce := responseWriter.Header().Get("Replay-Nonce")
test.AssertEquals(t, wfe.nonceService.Valid(nonce), true)
responseWriter := httptest.NewRecorder()
mux.ServeHTTP(responseWriter, &http.Request{
Method: tc.Method,
URL: mustParseURL(newNoncePath),
})
// The response should have the expected HTTP status code
test.AssertEquals(t, responseWriter.Code, tc.ExpectedStatus)
// And the response should contain a valid nonce in the Replay-Nonce header
nonce := responseWriter.Header().Get("Replay-Nonce")
test.AssertEquals(t, wfe.nonceService.Valid(nonce), true)
})
}
}
func TestHTTPMethods(t *testing.T) {