Skip to content

Commit

Permalink
WFE: Refuse to finalize orders with unrecognized profiles (#7988)
Browse files Browse the repository at this point in the history
The current profiles draft
(https://datatracker.ietf.org/doc/draft-aaron-acme-profiles/00/) says:

> If a server receives a request to finalize an Order whose profile the
> CA is no longer willing to issue under, it MUST respond with a
> problem document of type "invalidProfile".  The server SHOULD attempt
> to avoid this situation, e.g. by ensuring that all Orders for a
> profile have expired before it stops issuing under that profile.

Add types and helper functions representing this new error type to the
berrors, probs, and web packages. Update the WFE code which rejects
new-order requests with unrecognized profiles to use these new types,
and add similar code to the WFE's finalize path. Update the unit and
integration tests to reflect the fact that we now configure at least one
profile in both Staging and Prod (tracked in IN-10574).
  • Loading branch information
aarongable authored Jan 30, 2025
1 parent 1ae1847 commit c5a28cd
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 12 deletions.
9 changes: 8 additions & 1 deletion errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import (
"fmt"
"time"

"github.com/letsencrypt/boulder/identifier"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/letsencrypt/boulder/identifier"
)

// ErrorType provides a coarse category for BoulderErrors.
Expand Down Expand Up @@ -54,6 +55,8 @@ const (
// The certificate being indicated for replacement already has a replacement
// order.
Conflict
// Defined in https://datatracker.ietf.org/doc/draft-aaron-acme-profiles/00/
InvalidProfile
)

func (ErrorType) Error() string {
Expand Down Expand Up @@ -286,3 +289,7 @@ func UnknownSerialError() error {
func ConflictError(msg string, args ...interface{}) error {
return New(Conflict, msg, args...)
}

func InvalidProfileError(msg string, args ...interface{}) error {
return New(InvalidProfile, msg, args...)
}
13 changes: 13 additions & 0 deletions probs/probs.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ const (
UnsupportedContactProblem = ProblemType("unsupportedContact")
UnsupportedIdentifierProblem = ProblemType("unsupportedIdentifier")

// Defined in https://datatracker.ietf.org/doc/draft-aaron-acme-profiles/
InvalidProfileProblem = ProblemType("invalidProfile")

ErrorNS = "urn:ietf:params:acme:error:"
)

Expand Down Expand Up @@ -351,3 +354,13 @@ func NotFound(detail string) *ProblemDetails {
HTTPStatus: http.StatusNotFound,
}
}

// InvalidProfile returns a ProblemDetails with type InvalidProfile, specified
// in https://datatracker.ietf.org/doc/draft-aaron-acme-profiles/.
func InvalidProfile(detail string) *ProblemDetails {
return &ProblemDetails{
Type: InvalidProfileProblem,
Detail: detail,
HTTPStatus: http.StatusBadRequest,
}
}
4 changes: 4 additions & 0 deletions test/config/wfe2.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@
"IncrementRateLimits": true,
"CheckIdentifiersPaused": true
},
"certProfiles": {
"legacy": "The normal profile you know and love",
"modern": "Profile 2: Electric Boogaloo"
},
"unpause": {
"hmacKey": {
"keyFile": "test/secrets/sfe_unpause_key"
Expand Down
2 changes: 2 additions & 0 deletions web/probs.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func problemDetailsForBoulderError(err *berrors.BoulderError, msg string) *probs
outProb = probs.UnsupportedContact(fmt.Sprintf("%s :: %s", msg, err))
case berrors.Conflict:
outProb = probs.Conflict(fmt.Sprintf("%s :: %s", msg, err))
case berrors.InvalidProfile:
outProb = probs.InvalidProfile(fmt.Sprintf("%s :: %s", msg, err))
default:
// Internal server error messages may include sensitive data, so we do
// not include it.
Expand Down
14 changes: 12 additions & 2 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,7 @@ func (wfe *WebFrontEndImpl) validateCertificateProfileName(profile string) error
}
if _, ok := wfe.certProfiles[profile]; !ok {
// The profile name is not in the list of configured profiles.
return errors.New("not a recognized profile name")
return fmt.Errorf("profile name %q not recognized", profile)
}

return nil
Expand Down Expand Up @@ -2335,7 +2335,7 @@ func (wfe *WebFrontEndImpl) NewOrder(
err = wfe.validateCertificateProfileName(newOrderRequest.Profile)
if err != nil {
// TODO(#7392) Provide link to profile documentation.
wfe.sendError(response, logEvent, probs.Malformed("Invalid certificate profile, %q: %s", newOrderRequest.Profile, err), err)
wfe.sendError(response, logEvent, probs.InvalidProfile(err.Error()), err)
return
}

Expand Down Expand Up @@ -2542,6 +2542,16 @@ func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.Req
return
}

// Don't finalize orders with profiles we no longer recognize.
if order.CertificateProfileName != "" {
err = wfe.validateCertificateProfileName(order.CertificateProfileName)
if err != nil {
// TODO(#7392) Provide link to profile documentation.
wfe.sendError(response, logEvent, probs.InvalidProfile(err.Error()), err)
return
}
}

// The authenticated finalize message body should be an encoded CSR
var rawCSR core.RawCertificateRequest
err = json.Unmarshal(body, &rawCSR)
Expand Down
30 changes: 21 additions & 9 deletions wfe2/wfe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ func setupWFE(t *testing.T) (WebFrontEndImpl, clock.FakeClock, requestSigner) {
limiter,
txnBuilder,
100,
nil,
map[string]string{"default": "a test profile"},
unpauseSigner,
unpauseLifetime,
unpauseURL,
Expand Down Expand Up @@ -809,7 +809,10 @@ func TestDirectory(t *testing.T) {
expectedJSON: `{
"keyChange": "http://localhost:4300/acme/key-change",
"meta": {
"termsOfService": "http://example.invalid/terms"
"termsOfService": "http://example.invalid/terms",
"profiles": {
"default": "a test profile"
}
},
"newNonce": "http://localhost:4300/acme/new-nonce",
"newAccount": "http://localhost:4300/acme/new-acct",
Expand All @@ -831,7 +834,10 @@ func TestDirectory(t *testing.T) {
"Radiant Lock"
],
"termsOfService": "http://example.invalid/terms",
"website": "zombo.com"
"website": "zombo.com",
"profiles": {
"default": "a test profile"
}
},
"newAccount": "http://localhost:4300/acme/new-acct",
"newNonce": "http://localhost:4300/acme/new-nonce",
Expand All @@ -852,7 +858,10 @@ func TestDirectory(t *testing.T) {
"Radiant Lock"
],
"termsOfService": "http://example.invalid/terms",
"website": "zombo.com"
"website": "zombo.com",
"profiles": {
"default": "a test profile"
}
},
"newAccount": "http://localhost/acme/new-acct",
"newNonce": "http://localhost/acme/new-nonce",
Expand Down Expand Up @@ -900,7 +909,10 @@ func TestRelativeDirectory(t *testing.T) {
fmt.Fprintf(expected, `"newOrder":"%s/acme/new-order",`, hostname)
fmt.Fprintf(expected, `"revokeCert":"%s/acme/revoke-cert",`, hostname)
fmt.Fprintf(expected, `"AAAAAAAAAAA":"https://community.letsencrypt.org/t/adding-random-entries-to-the-directory/33417",`)
fmt.Fprintf(expected, `"meta":{"termsOfService":"http://example.invalid/terms"}`)
fmt.Fprintf(expected, `"meta":{`)
fmt.Fprintf(expected, `"termsOfService":"http://example.invalid/terms",`)
fmt.Fprintf(expected, `"profiles":{"default":"a test profile"}`)
fmt.Fprintf(expected, "}")
fmt.Fprintf(expected, "}")
return expected.String()
}
Expand Down Expand Up @@ -3994,8 +4006,8 @@ func TestNewOrderWithProfile(t *testing.T) {
var errorResp map[string]interface{}
err := json.Unmarshal(responseWriter.Body.Bytes(), &errorResp)
test.AssertNotError(t, err, "Failed to unmarshal error response")
test.AssertEquals(t, errorResp["type"], "urn:ietf:params:acme:error:malformed")
test.AssertEquals(t, errorResp["detail"], "Invalid certificate profile, \"bad-profile\": not a recognized profile name")
test.AssertEquals(t, errorResp["type"], "urn:ietf:params:acme:error:invalidProfile")
test.AssertEquals(t, errorResp["detail"], "profile name \"bad-profile\" not recognized")

// Test that the newOrder endpoint returns no error if the valid profile is specified.
validOrderBody := `
Expand Down Expand Up @@ -4023,8 +4035,8 @@ func TestNewOrderWithProfile(t *testing.T) {
var errorResp2 map[string]interface{}
err = json.Unmarshal(responseWriter.Body.Bytes(), &errorResp2)
test.AssertNotError(t, err, "Failed to unmarshal error response")
test.AssertEquals(t, errorResp2["type"], "urn:ietf:params:acme:error:malformed")
test.AssertEquals(t, errorResp2["detail"], "Invalid certificate profile, \"test-profile\": not a recognized profile name")
test.AssertEquals(t, errorResp2["type"], "urn:ietf:params:acme:error:invalidProfile")
test.AssertEquals(t, errorResp2["detail"], "profile name \"test-profile\" not recognized")
}

func makeARICertID(leaf *x509.Certificate) (string, error) {
Expand Down

0 comments on commit c5a28cd

Please sign in to comment.