ACME (RFC 8555) § 8.2 Challenge Retries

.gitignore

@@ -19,3 +19,7 @@ coverage.txt
 vendor
 output
 .idea
+
+# vscode
+*.code-workspace
+*_bin

acme/api/handler.go

@@ -149,31 +149,63 @@ func (h *Handler) GetAuthz(w http.ResponseWriter, r *http.Request) {
 	api.JSON(w, authz)
 }
 
-// GetChallenge ACME api for retrieving a Challenge.
+// GetChallenge is the ACME api for retrieving a Challenge resource.
+//
+// Potential Challenges are requested by the client when creating an order.
+// Once the client knows the appropriate validation resources are provisioned,
+// it makes a POST-as-GET request to this endpoint in order to initiate the
+// validation flow.
+//
+// The validation state machine describes the flow for a challenge.
+//
+//   https://tools.ietf.org/html/rfc8555#section-7.1.6
+//
+// Once a validation attempt has completed without error, the challenge's
+// status is updated depending on the result (valid|invalid) of the server's
+// validation attempt. Once this is the case, a challenge cannot be reset.
+//
+// If a challenge cannot be completed because no suitable data can be
+// acquired the server (whilst communicating retry information) and the
+// client (whilst respecting the information from the server) may request
+// retries of the validation.
+//
+//   https://tools.ietf.org/html/rfc8555#section-8.2
+//
+// Retry status is communicated using the error field and by sending a
+// Retry-After header back to the client.
+//
+// The request body is challenge-specific. The current challenges (http-01,
+// dns-01, tls-alpn-01) simply expect an empty object ("{}") in the payload
+// of the JWT sent by the client. We don't gain anything by stricly enforcing
+// nonexistence of unknown attributes, or, in these three cases, enforcing
+// an empty payload. And the spec also says to just ignore it:
+//
+// > The server MUST ignore any fields in the response object
+// > that are not specified as response fields for this type of challenge.
+//
+//    https://tools.ietf.org/html/rfc8555#section-7.5.1
+//
 func (h *Handler) GetChallenge(w http.ResponseWriter, r *http.Request) {
 	prov, err := provisionerFromContext(r)
 	if err != nil {
 		api.WriteError(w, err)
 		return
 	}
+
 	acc, err := accountFromContext(r)
 	if err != nil {
 		api.WriteError(w, err)
 		return
 	}
-	// Just verify that the payload was set, since we're not strictly adhering
-	// to ACME V2 spec for reasons specified below.
+
+	// Just verify that the payload was set since the client is required
+	// to send _something_.
 	_, err = payloadFromContext(r)
 	if err != nil {
 		api.WriteError(w, err)
 		return
 	}
 
-	// NOTE: We should be checking that the request is either a POST-as-GET, or
-	// that the payload is an empty JSON block ({}). However, older ACME clients
-	// still send a vestigial body (rather than an empty JSON block) and
-	// strict enforcement would render these clients broken. For the time being
-	// we'll just ignore the body.
 	var (
 		ch   *acme.Challenge
 		chID = chi.URLParam(r, "chID")
@@ -187,6 +219,13 @@ func (h *Handler) GetChallenge(w http.ResponseWriter, r *http.Request) {
 	getLink := h.Auth.GetLink
 	w.Header().Add("Link", link(getLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, ch.GetAuthzID()), "up"))
 	w.Header().Set("Location", getLink(acme.ChallengeLink, acme.URLSafeProvisionerName(prov), true, ch.GetID()))
+
+	if ch.Status == acme.StatusProcessing {
+		w.Header().Add("Retry-After", ch.RetryAfter)
+		// 200s are cachable. Don't cache this because it will likely change.
+		w.Header().Add("Cache-Control", "no-cache")
+	}
+
 	api.JSON(w, ch)
 }
 

acme/api/handler_test.go

@@ -231,7 +231,7 @@ func TestHandlerGetNonce(t *testing.T) {
 }
 
 func TestHandlerGetDirectory(t *testing.T) {
-	auth, err := acme.NewAuthority(new(db.MockNoSQLDB), "ca.smallstep.com", "acme", nil)
+	auth, err := acme.NewAuthority(new(db.MockNoSQLDB), "ca.smallstep.com", "acme", nil, 0)
 	assert.FatalError(t, err)
 	prov := newProv()
 	url := fmt.Sprintf("http://ca.smallstep.com/acme/%s/directory", acme.URLSafeProvisionerName(prov))
@@ -605,6 +605,7 @@ func TestHandlerGetChallenge(t *testing.T) {
 		ch         acme.Challenge
 		problem    *acme.Error
 	}
+
 	var tests = map[string]func(t *testing.T) test{
 		"fail/no-provisioner": func(t *testing.T) test {
 			return test{
@@ -613,20 +614,23 @@ func TestHandlerGetChallenge(t *testing.T) {
 				problem:    acme.ServerInternalErr(errors.New("provisioner expected in request context")),
 			}
 		},
+
 		"fail/nil-provisioner": func(t *testing.T) test {
 			return test{
 				ctx:        context.WithValue(context.Background(), provisionerContextKey, nil),
 				statusCode: 500,
 				problem:    acme.ServerInternalErr(errors.New("provisioner expected in request context")),
 			}
 		},
+
 		"fail/no-account": func(t *testing.T) test {
 			return test{
 				ctx:        context.WithValue(context.Background(), provisionerContextKey, prov),
 				statusCode: 400,
 				problem:    acme.AccountDoesNotExistErr(nil),
 			}
 		},
+
 		"fail/nil-account": func(t *testing.T) test {
 			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
 			ctx = context.WithValue(ctx, accContextKey, nil)
@@ -636,6 +640,7 @@ func TestHandlerGetChallenge(t *testing.T) {
 				problem:    acme.AccountDoesNotExistErr(nil),
 			}
 		},
+
 		"fail/no-payload": func(t *testing.T) test {
 			acc := &acme.Account{ID: "accID"}
 			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
@@ -646,6 +651,7 @@ func TestHandlerGetChallenge(t *testing.T) {
 				problem:    acme.ServerInternalErr(errors.New("payload expected in request context")),
 			}
 		},
+
 		"fail/nil-payload": func(t *testing.T) test {
 			acc := &acme.Account{ID: "accID"}
 			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
@@ -657,6 +663,7 @@ func TestHandlerGetChallenge(t *testing.T) {
 				problem:    acme.ServerInternalErr(errors.New("payload expected in request context")),
 			}
 		},
+
 		"fail/validate-challenge-error": func(t *testing.T) test {
 			acc := &acme.Account{ID: "accID"}
 			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
@@ -665,39 +672,76 @@ func TestHandlerGetChallenge(t *testing.T) {
 			ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx)
 			return test{
 				auth: &mockAcmeAuthority{
-					err: acme.UnauthorizedErr(nil),
+					err: acme.ServerInternalErr(nil),
 				},
 				ctx:        ctx,
-				statusCode: 401,
-				problem:    acme.UnauthorizedErr(nil),
+				statusCode: 500,
+				problem:    acme.ServerInternalErr(nil),
 			}
 		},
-		"fail/get-challenge-error": func(t *testing.T) test {
-			acc := &acme.Account{ID: "accID"}
+
+		"ok/validate-challenge": func(t *testing.T) test {
+			key, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
+			assert.FatalError(t, err)
+			acc := &acme.Account{ID: "accID", Key: key}
 			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
 			ctx = context.WithValue(ctx, accContextKey, acc)
-			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{isPostAsGet: true})
+			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{isEmptyJSON: true})
 			ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx)
+			ch := ch()
+			ch.Status = "valid"
+			ch.Validated = time.Now().UTC().Format(time.RFC3339)
+			count := 0
 			return test{
 				auth: &mockAcmeAuthority{
-					err: acme.UnauthorizedErr(nil),
+					validateChallenge: func(p provisioner.Interface, accID, id string, jwk *jose.JSONWebKey) (*acme.Challenge, error) {
+						assert.Equals(t, p, prov)
+						assert.Equals(t, accID, acc.ID)
+						assert.Equals(t, id, ch.ID)
+						assert.Equals(t, jwk.KeyID, key.KeyID)
+						return &ch, nil
+					},
+					getLink: func(typ acme.Link, provID string, abs bool, in ...string) string {
+						var ret string
+						switch count {
+						case 0:
+							assert.Equals(t, typ, acme.AuthzLink)
+							assert.Equals(t, provID, acme.URLSafeProvisionerName(prov))
+							assert.True(t, abs)
+							assert.Equals(t, in, []string{ch.AuthzID})
+							ret = fmt.Sprintf("https://ca.smallstep.com/acme/authz/%s", ch.AuthzID)
+						case 1:
+							assert.Equals(t, typ, acme.ChallengeLink)
+							assert.Equals(t, provID, acme.URLSafeProvisionerName(prov))
+							assert.True(t, abs)
+							assert.Equals(t, in, []string{ch.ID})
+							ret = url
+						}
+						count++
+						return ret
+					},
 				},
 				ctx:        ctx,
-				statusCode: 401,
-				problem:    acme.UnauthorizedErr(nil),
+				statusCode: 200,
+				ch:         ch,
 			}
 		},
-		"ok/validate-challenge": func(t *testing.T) test {
+
+		"ok/retry-after": func(t *testing.T) test {
 			key, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
 			assert.FatalError(t, err)
 			acc := &acme.Account{ID: "accID", Key: key}
 			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
 			ctx = context.WithValue(ctx, accContextKey, acc)
-			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{isEmptyJSON: true})
-			ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx)
+			chiCtxInactive := chi.NewRouteContext()
+			chiCtxInactive.URLParams.Add("chID", "chID")
+			ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtxInactive)
 			ch := ch()
-			ch.Status = "valid"
-			ch.Validated = time.Now().UTC().Format(time.RFC3339)
+			ch.Status = "processing"
+			ch.RetryAfter = time.Now().Add(1 * time.Minute).UTC().Format(time.RFC3339)
+			chJSON, err := json.Marshal(ch)
+			assert.FatalError(t, err)
+			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: chJSON})
 			count := 0
 			return test{
 				auth: &mockAcmeAuthority{
@@ -728,12 +772,15 @@ func TestHandlerGetChallenge(t *testing.T) {
 						return ret
 					},
 				},
+
 				ctx:        ctx,
 				statusCode: 200,
 				ch:         ch,
 			}
 		},
 	}
+
+	// Run the tests
 	for name, run := range tests {
 		tc := run(t)
 		t.Run(name, func(t *testing.T) {
@@ -760,13 +807,21 @@ func TestHandlerGetChallenge(t *testing.T) {
 				assert.Equals(t, ae.Identifier, prob.Identifier)
 				assert.Equals(t, ae.Subproblems, prob.Subproblems)
 				assert.Equals(t, res.Header["Content-Type"], []string{"application/problem+json"})
-			} else {
+			} else if res.StatusCode >= 200 {
 				expB, err := json.Marshal(tc.ch)
 				assert.FatalError(t, err)
 				assert.Equals(t, bytes.TrimSpace(body), expB)
-				assert.Equals(t, res.Header["Link"], []string{fmt.Sprintf("<https://ca.smallstep.com/acme/authz/%s>;rel=\"up\"", tc.ch.AuthzID)})
-				assert.Equals(t, res.Header["Location"], []string{url})
 				assert.Equals(t, res.Header["Content-Type"], []string{"application/json"})
+				switch tc.ch.Status {
+				case "processing":
+					assert.Equals(t, res.Header["Cache-Control"], []string{"no-cache"})
+					assert.Equals(t, res.Header["Retry-After"], []string{tc.ch.RetryAfter})
+				case "valid", "invalid":
+					assert.Equals(t, res.Header["Location"], []string{url})
+					assert.Equals(t, res.Header["Link"], []string{fmt.Sprintf("<https://ca.smallstep.com/acme/authz/%s>;rel=\"up\"", tc.ch.AuthzID)})
+				}
+			} else {
+				assert.Fatal(t, false, "Unexpected Status Code")
 			}
 		})
 	}