Skip to content

Commit baf3287

Browse files
cpuRoland Bracewell Shoemaker
cpu
authored and
Roland Bracewell Shoemaker
committed
Prefix problem type with namespace at runtime. (letsencrypt#3039)
To support having problem types that use either the classic "urn:acme:error" namespace or the new "urn:ietf:params:acme:error" namespace as appropriate we need to prefix the problem type at runtime right before returning it through the WFE to the user as JSON. This commit updates the WFE/WFE2 to do this for both problems sent through sendError as well as problems embedded in challenges. For the latter we do not modify problems with a type that is already prefixed to support backwards compatibility. Resolves letsencrypt#2938 Note: We should cut a follow-up issue to devise a way to share some common code between the WFE and WFE2. For example, the prepChallengeForDisplay should probably be hoisted to a common "web" package
1 parent f193137 commit baf3287

File tree

8 files changed

+286
-141
lines changed

8 files changed

+286
-141
lines changed

‎mocks/mocks.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/letsencrypt/boulder/core"
1616
corepb "github.com/letsencrypt/boulder/core/proto"
1717
berrors "github.com/letsencrypt/boulder/errors"
18+
"github.com/letsencrypt/boulder/probs"
1819
"github.com/letsencrypt/boulder/revocation"
1920
sapb "github.com/letsencrypt/boulder/sa/proto"
2021
)
@@ -494,3 +495,47 @@ func (m *Mailer) Close() error {
494495
func (m *Mailer) Connect() error {
495496
return nil
496497
}
498+
499+
// mockSAWithFailedChallenges is a mocks.StorageAuthority that has
500+
// a `GetAuthorization` implementation that can return authorizations with
501+
// failed challenges.
502+
type SAWithFailedChallenges struct {
503+
StorageAuthority
504+
Clk clock.FakeClock
505+
}
506+
507+
func (sa *SAWithFailedChallenges) GetAuthorization(_ context.Context, id string) (core.Authorization, error) {
508+
authz := core.Authorization{
509+
ID: "valid",
510+
Status: core.StatusValid,
511+
RegistrationID: 1,
512+
Identifier: core.AcmeIdentifier{Type: "dns", Value: "not-an-example.com"},
513+
Challenges: []core.Challenge{
514+
{
515+
ID: 23,
516+
Type: "dns",
517+
},
518+
},
519+
}
520+
prob := &probs.ProblemDetails{
521+
Type: "things:are:whack",
522+
Detail: "whack attack",
523+
HTTPStatus: 555,
524+
}
525+
exp := sa.Clk.Now().AddDate(100, 0, 0)
526+
authz.Expires = &exp
527+
// "oldNS" returns an authz with a failed challenge that has the problem type
528+
// statically prefixed by the V1ErrorNS
529+
if id == "oldNS" {
530+
prob.Type = probs.V1ErrorNS + prob.Type
531+
authz.Challenges[0].Error = prob
532+
return authz, nil
533+
}
534+
// "failed" returns an authz with a failed challenge that has no error
535+
// namespace on the problem type.
536+
if id == "failed" {
537+
authz.Challenges[0].Error = prob
538+
return authz, nil
539+
}
540+
return core.Authorization{}, berrors.NotFoundError("no authorization found with id %q", id)
541+
}

‎probs/probs.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,20 @@ import (
77

88
// Error types that can be used in ACME payloads
99
const (
10-
ConnectionProblem = ProblemType("urn:acme:error:connection")
11-
MalformedProblem = ProblemType("urn:acme:error:malformed")
12-
ServerInternalProblem = ProblemType("urn:acme:error:serverInternal")
13-
TLSProblem = ProblemType("urn:acme:error:tls")
14-
UnauthorizedProblem = ProblemType("urn:acme:error:unauthorized")
15-
UnknownHostProblem = ProblemType("urn:acme:error:unknownHost")
16-
RateLimitedProblem = ProblemType("urn:acme:error:rateLimited")
17-
BadNonceProblem = ProblemType("urn:acme:error:badNonce")
18-
InvalidEmailProblem = ProblemType("urn:acme:error:invalidEmail")
19-
RejectedIdentifierProblem = ProblemType("urn:acme:error:rejectedIdentifier")
20-
21-
v2ErrorNS = "urn:ietf:params:acme:error:"
22-
AccountDoesNotExistProblem = ProblemType(v2ErrorNS + "accountDoesNotExist")
10+
ConnectionProblem = ProblemType("connection")
11+
MalformedProblem = ProblemType("malformed")
12+
ServerInternalProblem = ProblemType("serverInternal")
13+
TLSProblem = ProblemType("tls")
14+
UnauthorizedProblem = ProblemType("unauthorized")
15+
UnknownHostProblem = ProblemType("unknownHost")
16+
RateLimitedProblem = ProblemType("rateLimited")
17+
BadNonceProblem = ProblemType("badNonce")
18+
InvalidEmailProblem = ProblemType("invalidEmail")
19+
RejectedIdentifierProblem = ProblemType("rejectedIdentifier")
20+
AccountDoesNotExistProblem = ProblemType("accountDoesNotExist")
21+
22+
V1ErrorNS = "urn:acme:error:"
23+
V2ErrorNS = "urn:ietf:params:acme:error:"
2324
)
2425

2526
// ProblemType defines the error types in the ACME protocol

‎probs/probs_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func TestProblemDetails(t *testing.T) {
1414
Detail: "Wat? o.O",
1515
HTTPStatus: 403,
1616
}
17-
test.AssertEquals(t, pd.Error(), "urn:acme:error:malformed :: Wat? o.O")
17+
test.AssertEquals(t, pd.Error(), "malformed :: Wat? o.O")
1818
}
1919

2020
func TestProblemDetailsToStatusCode(t *testing.T) {

‎va/va_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ func TestDNSValidationEmpty(t *testing.T) {
813813
"empty-txts.com",
814814
chalDNS,
815815
core.Authorization{})
816-
test.AssertEquals(t, prob.Error(), "urn:acme:error:unauthorized :: No TXT records found for DNS challenge")
816+
test.AssertEquals(t, prob.Error(), "unauthorized :: No TXT records found for DNS challenge")
817817
}
818818

819819
func TestPerformValidationValid(t *testing.T) {

‎wfe/wfe.go‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -603,8 +603,10 @@ func (wfe *WebFrontEndImpl) verifyPOST(ctx context.Context, logEvent *requestEve
603603

604604
// sendError sends an error response represented by the given ProblemDetails,
605605
// and, if the ProblemDetails.Type is ServerInternalProblem, audit logs the
606-
// internal ierr.
606+
// internal ierr. The rendered Problem will have its Type prefixed with the ACME
607+
// v1 namespace.
607608
func (wfe *WebFrontEndImpl) sendError(response http.ResponseWriter, logEvent *requestEvent, prob *probs.ProblemDetails, ierr error) {
609+
// Determine the HTTP status code to use for this problem
608610
code := probs.ProblemDetailsToStatusCode(prob)
609611

610612
// Record details to the log event
@@ -620,22 +622,21 @@ func (wfe *WebFrontEndImpl) sendError(response http.ResponseWriter, logEvent *re
620622
}
621623
}
622624

625+
// Increment a stat for this problem type
626+
wfe.stats.Inc(fmt.Sprintf("HTTP.ProblemTypes.%s", prob.Type), 1)
627+
628+
// Prefix the problem type with the ACME V1 error namespace and marshal to JSON
629+
prob.Type = probs.V1ErrorNS + prob.Type
623630
problemDoc, err := marshalIndent(prob)
624631
if err != nil {
625632
wfe.log.AuditErr(fmt.Sprintf("Could not marshal error message: %s - %+v", err, prob))
626633
problemDoc = []byte("{\"detail\": \"Problem marshalling error message.\"}")
627634
}
628635

629-
// Paraphrased from
630-
// https://golang.org/src/net/http/server.go#L1272
636+
// Write the JSON problem response
631637
response.Header().Set("Content-Type", "application/problem+json")
632638
response.WriteHeader(code)
633639
response.Write(problemDoc)
634-
635-
problemSegments := strings.Split(string(prob.Type), ":")
636-
if len(problemSegments) > 0 {
637-
wfe.stats.Inc(fmt.Sprintf("HTTP.ProblemTypes.%s", problemSegments[len(problemSegments)-1]), 1)
638-
}
639640
}
640641

641642
func link(url, relation string) string {
@@ -1078,12 +1079,20 @@ func (wfe *WebFrontEndImpl) Challenge(
10781079

10791080
// prepChallengeForDisplay takes a core.Challenge and prepares it for display to
10801081
// the client by filling in its URI field and clearing its ID field.
1081-
// TODO: Come up with a cleaner way to do this.
1082-
// https://github.com/letsencrypt/boulder/issues/761
10831082
func (wfe *WebFrontEndImpl) prepChallengeForDisplay(request *http.Request, authz core.Authorization, challenge *core.Challenge) {
1083+
// Update the challenge URI to be relative to the HTTP request Host
10841084
challenge.URI = wfe.relativeEndpoint(request, fmt.Sprintf("%s%s/%d", challengePath, authz.ID, challenge.ID))
1085-
// 0 is considered "empty" for the purpose of the JSON omitempty tag.
1085+
// Ensure the challenge ID isn't written. 0 is considered "empty" for the purpose of the JSON omitempty tag.
10861086
challenge.ID = 0
1087+
1088+
// Historically the Type field of a problem was always prefixed with a static
1089+
// error namespace. To support the V2 API and migrating to the correct IETF
1090+
// namespace we now prefix the Type with the correct namespace at runtime when
1091+
// we write the problem JSON to the user. We skip this process if the
1092+
// challenge error type has already been prefixed with the V1ErrorNS.
1093+
if challenge.Error != nil && !strings.HasPrefix(string(challenge.Error.Type), probs.V1ErrorNS) {
1094+
challenge.Error.Type = probs.V1ErrorNS + challenge.Error.Type
1095+
}
10871096
}
10881097

10891098
// prepAuthorizationForDisplay takes a core.Authorization and prepares it for

0 commit comments

Comments
 (0)