From 83a5a9bb288b4c9523e182ed4f621411a11de3df Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Sun, 10 May 2020 14:28:56 -0700 Subject: [PATCH] acme: add support for subproblems Add support for RFC 8555 subproblems. The type naming is real bike-shed territory, but I think I've mostly matched the existing style of the package. In a similar vein the format of how to print subproblems when stringing an acme.Error is up for debate (it could just be completely ignored, and require clients to inspect Error.Subproblems themselves). Fixes golang/go#38978 Change-Id: Ice803079bab621ae9410de79e7e75e11c1af21b6 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/233165 Trust: Roland Shoemaker Run-TryBot: Roland Shoemaker TryBot-Result: Go Bot Reviewed-by: Russ Cox --- acme/types.go | 52 ++++++++++++++++++++--- acme/types_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 6 deletions(-) diff --git a/acme/types.go b/acme/types.go index e751bf5..eaae452 100644 --- a/acme/types.go +++ b/acme/types.go @@ -57,6 +57,32 @@ var ( ErrNoAccount = errors.New("acme: account does not exist") ) +// A Subproblem describes an ACME subproblem as reported in an Error. +type Subproblem struct { + // Type is a URI reference that identifies the problem type, + // typically in a "urn:acme:error:xxx" form. + Type string + // Detail is a human-readable explanation specific to this occurrence of the problem. + Detail string + // Instance indicates a URL that the client should direct a human user to visit + // in order for instructions on how to agree to the updated Terms of Service. + // In such an event CA sets StatusCode to 403, Type to + // "urn:ietf:params:acme:error:userActionRequired", and adds a Link header with relation + // "terms-of-service" containing the latest TOS URL. + Instance string + // Identifier may contain the ACME identifier that the error is for. + Identifier *AuthzID +} + +func (sp Subproblem) String() string { + str := fmt.Sprintf("%s: ", sp.Type) + if sp.Identifier != nil { + str += fmt.Sprintf("[%s: %s] ", sp.Identifier.Type, sp.Identifier.Value) + } + str += sp.Detail + return str +} + // Error is an ACME error, defined in Problem Details for HTTP APIs doc // http://tools.ietf.org/html/draft-ietf-appsawg-http-problem. type Error struct { @@ -76,10 +102,21 @@ type Error struct { // Header is the original server error response headers. // It may be nil. Header http.Header + // Subproblems may contain more detailed information about the individual problems + // that caused the error. This field is only sent by RFC 8555 compatible ACME + // servers. Defined in RFC 8555 Section 6.7.1. + Subproblems []Subproblem } func (e *Error) Error() string { - return fmt.Sprintf("%d %s: %s", e.StatusCode, e.ProblemType, e.Detail) + str := fmt.Sprintf("%d %s: %s", e.StatusCode, e.ProblemType, e.Detail) + if len(e.Subproblems) > 0 { + str += fmt.Sprintf("; subproblems:") + for _, sp := range e.Subproblems { + str += fmt.Sprintf("\n\t%s", sp) + } + } + return str } // AuthorizationError indicates that an authorization for an identifier @@ -533,20 +570,23 @@ func (c *wireChallenge) challenge() *Challenge { // wireError is a subset of fields of the Problem Details object // as described in https://tools.ietf.org/html/rfc7807#section-3.1. type wireError struct { - Status int - Type string - Detail string - Instance string + Status int + Type string + Detail string + Instance string + Subproblems []Subproblem } func (e *wireError) error(h http.Header) *Error { - return &Error{ + err := &Error{ StatusCode: e.Status, ProblemType: e.Type, Detail: e.Detail, Instance: e.Instance, Header: h, + Subproblems: e.Subproblems, } + return err } // CertOption is an optional argument type for the TLS ChallengeCert methods for diff --git a/acme/types_test.go b/acme/types_test.go index 9ed7753..59ce7e7 100644 --- a/acme/types_test.go +++ b/acme/types_test.go @@ -7,6 +7,7 @@ package acme import ( "errors" "net/http" + "reflect" "testing" "time" ) @@ -116,3 +117,103 @@ func TestAuthorizationError(t *testing.T) { } } } + +func TestSubproblems(t *testing.T) { + tests := []struct { + wire wireError + expectedOut Error + }{ + { + wire: wireError{ + Status: 1, + Type: "urn:error", + Detail: "it's an error", + }, + expectedOut: Error{ + StatusCode: 1, + ProblemType: "urn:error", + Detail: "it's an error", + }, + }, + { + wire: wireError{ + Status: 1, + Type: "urn:error", + Detail: "it's an error", + Subproblems: []Subproblem{ + { + Type: "urn:error:sub", + Detail: "it's a subproblem", + }, + }, + }, + expectedOut: Error{ + StatusCode: 1, + ProblemType: "urn:error", + Detail: "it's an error", + Subproblems: []Subproblem{ + { + Type: "urn:error:sub", + Detail: "it's a subproblem", + }, + }, + }, + }, + { + wire: wireError{ + Status: 1, + Type: "urn:error", + Detail: "it's an error", + Subproblems: []Subproblem{ + { + Type: "urn:error:sub", + Detail: "it's a subproblem", + Identifier: &AuthzID{Type: "dns", Value: "example"}, + }, + }, + }, + expectedOut: Error{ + StatusCode: 1, + ProblemType: "urn:error", + Detail: "it's an error", + Subproblems: []Subproblem{ + { + Type: "urn:error:sub", + Detail: "it's a subproblem", + Identifier: &AuthzID{Type: "dns", Value: "example"}, + }, + }, + }, + }, + } + + for _, tc := range tests { + out := tc.wire.error(nil) + if !reflect.DeepEqual(*out, tc.expectedOut) { + t.Errorf("Unexpected error: wanted %v, got %v", tc.expectedOut, *out) + } + } +} + +func TestErrorStringerWithSubproblems(t *testing.T) { + err := Error{ + StatusCode: 1, + ProblemType: "urn:error", + Detail: "it's an error", + Subproblems: []Subproblem{ + { + Type: "urn:error:sub", + Detail: "it's a subproblem", + }, + { + Type: "urn:error:sub", + Detail: "it's a subproblem", + Identifier: &AuthzID{Type: "dns", Value: "example"}, + }, + }, + } + expectedStr := "1 urn:error: it's an error; subproblems:\n\turn:error:sub: it's a subproblem\n\turn:error:sub: [dns: example] it's a subproblem" + if err.Error() != expectedStr { + t.Errorf("Unexpected error string: wanted %q, got %q", expectedStr, err.Error()) + } +}