diff --git a/acme/challenge.go b/acme/challenge.go index e43b15b4..687cc680 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -30,6 +30,7 @@ import ( "golang.org/x/exp/slices" "github.com/smallstep/go-attestation/attest" + "go.step.sm/crypto/jose" "go.step.sm/crypto/keyutil" "go.step.sm/crypto/pemutil" @@ -379,13 +380,18 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose return WrapErrorISE(err, "error unmarshalling CBOR") } + format := att.Format prov := MustProvisionerFromContext(ctx) - if !prov.IsAttestationFormatEnabled(ctx, provisioner.ACMEAttestationFormat(att.Format)) { + if !prov.IsAttestationFormatEnabled(ctx, provisioner.ACMEAttestationFormat(format)) { + if format != "apple" && format != "step" && format != "tpm" { + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "unsupported attestation object format %q", format)) + } + return storeError(ctx, db, ch, true, - NewError(ErrorBadAttestationStatementType, "attestation format %q is not enabled", att.Format)) + NewError(ErrorBadAttestationStatementType, "attestation format %q is not enabled", format)) } - switch att.Format { + switch format { case "apple": data, err := doAppleAttestationFormat(ctx, prov, ch, &att) if err != nil { @@ -398,11 +404,12 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose } return WrapErrorISE(err, "error validating attestation") } + // Validate nonce with SHA-256 of the token. if len(data.Nonce) != 0 { sum := sha256.Sum256([]byte(ch.Token)) if subtle.ConstantTimeCompare(data.Nonce, sum[:]) != 1 { - return storeError(ctx, db, ch, true, NewError(ErrorBadAttestationStatementType, "challenge token does not match")) + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "challenge token does not match")) } } @@ -411,7 +418,12 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose // // Note: We might want to use an external service for this. if data.UDID != ch.Value && data.SerialNumber != ch.Value { - return storeError(ctx, db, ch, true, NewError(ErrorBadAttestationStatementType, "permanent identifier does not match")) + subproblem := NewSubproblemWithIdentifier( + ErrorRejectedIdentifierType, + Identifier{Type: "permanent-identifier", Value: ch.Value}, + "challenge identifier %q doesn't match any of the attested hardware identifiers %q", ch.Value, []string{data.UDID, data.SerialNumber}, + ) + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "permanent identifier does not match").AddSubproblems(subproblem)) } // Update attestation key fingerprint to compare against the CSR @@ -435,11 +447,11 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose // Note: We might want to use an external service for this. if data.SerialNumber != ch.Value { subproblem := NewSubproblemWithIdentifier( - ErrorMalformedType, + ErrorRejectedIdentifierType, Identifier{Type: "permanent-identifier", Value: ch.Value}, "challenge identifier %q doesn't match the attested hardware identifier %q", ch.Value, data.SerialNumber, ) - return storeError(ctx, db, ch, true, NewError(ErrorBadAttestationStatementType, "permanent identifier does not match").AddSubproblems(subproblem)) + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "permanent identifier does not match").AddSubproblems(subproblem)) } // Update attestation key fingerprint to compare against the CSR @@ -448,8 +460,6 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose case "tpm": data, err := doTPMAttestationFormat(ctx, prov, ch, jwk, &att) if err != nil { - // TODO(hs): we should provide more details in the error reported to the client; - // "Attestation statement cannot be verified" is VERY generic. Also holds true for the other formats. var acmeError *Error if errors.As(err, &acmeError) { if acmeError.Status == 500 { @@ -467,17 +477,17 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose // still fail if the challenge value isn't equal to the CSR subject. if len(data.PermanentIdentifiers) > 0 && !slices.Contains(data.PermanentIdentifiers, ch.Value) { // TODO(hs): add support for HardwareModuleName subproblem := NewSubproblemWithIdentifier( - ErrorMalformedType, + ErrorRejectedIdentifierType, Identifier{Type: "permanent-identifier", Value: ch.Value}, "challenge identifier %q doesn't match any of the attested hardware identifiers %q", ch.Value, data.PermanentIdentifiers, ) - return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType, "permanent identifier does not match").AddSubproblems(subproblem)) + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "permanent identifier does not match").AddSubproblems(subproblem)) } // Update attestation key fingerprint to compare against the CSR az.Fingerprint = data.Fingerprint default: - return storeError(ctx, db, ch, true, NewError(ErrorBadAttestationStatementType, "unexpected attestation object format")) + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "unsupported attestation object format %q", format)) } // Update and store the challenge. @@ -523,38 +533,38 @@ const ( func doTPMAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, jwk *jose.JSONWebKey, att *attestationObject) (*tpmAttestationData, error) { ver, ok := att.AttStatement["ver"].(string) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "ver not present") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "ver not present") } if ver != "2.0" { - return nil, NewError(ErrorBadAttestationStatementType, "version %q is not supported", ver) + return nil, NewDetailedError(ErrorBadAttestationStatementType, "version %q is not supported", ver) } x5c, ok := att.AttStatement["x5c"].([]interface{}) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c not present") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c not present") } if len(x5c) == 0 { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is empty") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is empty") } akCertBytes, ok := x5c[0].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is malformed") } akCert, err := x509.ParseCertificate(akCertBytes) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is malformed") } intermediates := x509.NewCertPool() for _, v := range x5c[1:] { intCertBytes, vok := v.([]byte) if !vok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is malformed") } intCert, err := x509.ParseCertificate(intCertBytes) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is malformed") } intermediates.AddCert(intCert) } @@ -592,19 +602,19 @@ func doTPMAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, }) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is not valid") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is not valid") } // validate additional AK certificate requirements if err := validateAKCertificate(akCert); err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "AK certificate is not valid") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "AK certificate is not valid") } // TODO(hs): implement revocation check; Verify() doesn't perform CRL check nor OCSP lookup. sans, err := x509util.ParseSubjectAlternativeNames(akCert) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed parsing AK certificate Subject Alternative Names") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "failed parsing AK certificate Subject Alternative Names") } permanentIdentifiers := make([]string, len(sans.PermanentIdentifiers)) @@ -615,37 +625,37 @@ func doTPMAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, // extract and validate pubArea, sig, certInfo and alg properties from the request body pubArea, ok := att.AttStatement["pubArea"].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "invalid pubArea in attestation statement") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "invalid pubArea in attestation statement") } if len(pubArea) == 0 { - return nil, NewError(ErrorBadAttestationStatementType, "pubArea is empty") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "pubArea is empty") } sig, ok := att.AttStatement["sig"].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "invalid sig in attestation statement") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "invalid sig in attestation statement") } if len(sig) == 0 { - return nil, NewError(ErrorBadAttestationStatementType, "sig is empty") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "sig is empty") } certInfo, ok := att.AttStatement["certInfo"].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "invalid certInfo in attestation statement") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "invalid certInfo in attestation statement") } if len(certInfo) == 0 { - return nil, NewError(ErrorBadAttestationStatementType, "certInfo is empty") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "certInfo is empty") } alg, ok := att.AttStatement["alg"].(int64) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "invalid alg in attestation statement") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "invalid alg in attestation statement") } // only RS256 and ES256 are allowed coseAlg := coseAlgorithmIdentifier(alg) if coseAlg != coseAlgRS256 && coseAlg != coseAlgES256 { - return nil, NewError(ErrorBadAttestationStatementType, "invalid alg %d in attestation statement", alg) + return nil, NewDetailedError(ErrorBadAttestationStatementType, "invalid alg %d in attestation statement", alg) } // set the hash algorithm to use to SHA256 @@ -663,36 +673,36 @@ func doTPMAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, Hash: hash, } if err = certificationParameters.Verify(verifyOpts); err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "invalid certification parameters") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "invalid certification parameters") } // decode the "certInfo" data. This won't fail, as it's also done as part of Verify(). tpmCertInfo, err := tpm2.DecodeAttestationData(certInfo) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed decoding attestation data") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "failed decoding attestation data") } keyAuth, err := KeyAuthorization(ch.Token, jwk) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed creating key auth digest") + return nil, WrapErrorISE(err, "failed creating key auth digest") } hashedKeyAuth := sha256.Sum256([]byte(keyAuth)) // verify the WebAuthn object contains the expect key authorization digest, which is carried // within the encoded `certInfo` property of the attestation statement. if subtle.ConstantTimeCompare(hashedKeyAuth[:], []byte(tpmCertInfo.ExtraData)) == 0 { - return nil, NewError(ErrorBadAttestationStatementType, "key authorization does not match") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "key authorization invalid") } // decode the (attested) public key and determine its fingerprint. This won't fail, as it's also done as part of Verify(). pub, err := tpm2.DecodePublic(pubArea) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed decoding pubArea") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "failed decoding pubArea") } publicKey, err := pub.Key() if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed getting public key") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "failed getting public key") } data := &tpmAttestationData{ @@ -838,30 +848,30 @@ func doAppleAttestationFormat(_ context.Context, prov Provisioner, _ *Challenge, x5c, ok := att.AttStatement["x5c"].([]interface{}) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c not present") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c not present") } if len(x5c) == 0 { - return nil, NewError(ErrorRejectedIdentifierType, "x5c is empty") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is empty") } der, ok := x5c[0].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is malformed") } leaf, err := x509.ParseCertificate(der) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is malformed") } intermediates := x509.NewCertPool() for _, v := range x5c[1:] { der, ok = v.([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is malformed") } cert, err := x509.ParseCertificate(der) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is malformed") } intermediates.AddCert(cert) } @@ -872,7 +882,7 @@ func doAppleAttestationFormat(_ context.Context, prov Provisioner, _ *Challenge, CurrentTime: time.Now().Truncate(time.Second), KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, }); err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is not valid") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is not valid") } data := &appleAttestationData{ @@ -944,28 +954,28 @@ func doStepAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, // Extract x5c and verify certificate x5c, ok := att.AttStatement["x5c"].([]interface{}) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c not present") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c not present") } if len(x5c) == 0 { - return nil, NewError(ErrorRejectedIdentifierType, "x5c is empty") + return nil, NewDetailedError(ErrorRejectedIdentifierType, "x5c is empty") } der, ok := x5c[0].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is malformed") } leaf, err := x509.ParseCertificate(der) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is malformed") } intermediates := x509.NewCertPool() for _, v := range x5c[1:] { der, ok = v.([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "x5c is malformed") } cert, err := x509.ParseCertificate(der) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is malformed") } intermediates.AddCert(cert) } @@ -975,7 +985,7 @@ func doStepAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, CurrentTime: time.Now().Truncate(time.Second), KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, }); err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is not valid") + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "x5c is not valid") } // Verify proof of possession of private key validating the key @@ -985,10 +995,10 @@ func doStepAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, var sig []byte csig, ok := att.AttStatement["sig"].([]byte) if !ok { - return nil, NewError(ErrorBadAttestationStatementType, "sig not present") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "sig not present") } if err := cbor.Unmarshal(csig, &sig); err != nil { - return nil, NewError(ErrorBadAttestationStatementType, "sig is malformed") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "sig is malformed") } keyAuth, err := KeyAuthorization(ch.Token, jwk) if err != nil { @@ -998,23 +1008,23 @@ func doStepAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge, switch pub := leaf.PublicKey.(type) { case *ecdsa.PublicKey: if pub.Curve != elliptic.P256() { - return nil, WrapError(ErrorBadAttestationStatementType, err, "unsupported elliptic curve %s", pub.Curve) + return nil, WrapDetailedError(ErrorBadAttestationStatementType, err, "unsupported elliptic curve %s", pub.Curve) } sum := sha256.Sum256([]byte(keyAuth)) if !ecdsa.VerifyASN1(pub, sum[:], sig) { - return nil, NewError(ErrorBadAttestationStatementType, "failed to validate signature") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "failed to validate signature") } case *rsa.PublicKey: sum := sha256.Sum256([]byte(keyAuth)) if err := rsa.VerifyPKCS1v15(pub, crypto.SHA256, sum[:], sig); err != nil { - return nil, NewError(ErrorBadAttestationStatementType, "failed to validate signature") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "failed to validate signature") } case ed25519.PublicKey: if !ed25519.Verify(pub, []byte(keyAuth), sig) { - return nil, NewError(ErrorBadAttestationStatementType, "failed to validate signature") + return nil, NewDetailedError(ErrorBadAttestationStatementType, "failed to validate signature") } default: - return nil, NewError(ErrorBadAttestationStatementType, "unsupported public key type %T", pub) + return nil, NewDetailedError(ErrorBadAttestationStatementType, "unsupported public key type %T", pub) } // Parse attestation data: diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 74ff363c..f14249d2 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -3532,7 +3532,7 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "12345678", updch.Value) - err := NewError(ErrorBadAttestationStatementType, "x5c not present") + err := NewDetailedError(ErrorBadAttestationStatementType, "x5c not present") assert.EqualError(t, updch.Error.Err, err.Err.Error()) assert.Equal(t, err.Type, updch.Error.Type) @@ -3579,7 +3579,7 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "serial-number", updch.Value) - err := NewError(ErrorBadAttestationStatementType, "challenge token does not match") + err := NewDetailedError(ErrorBadAttestationStatementType, "challenge token does not match") assert.EqualError(t, updch.Error.Err, err.Err.Error()) assert.Equal(t, err.Type, updch.Error.Type) @@ -3625,7 +3625,12 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "non-matching-value", updch.Value) - err := NewError(ErrorBadAttestationStatementType, "permanent identifier does not match") + subproblem := NewSubproblemWithIdentifier( + ErrorRejectedIdentifierType, + Identifier{Type: "permanent-identifier", Value: "non-matching-value"}, + `challenge identifier "non-matching-value" doesn't match any of the attested hardware identifiers ["udid" "serial-number"]`, + ) + err := NewDetailedError(ErrorBadAttestationStatementType, "permanent identifier does not match").AddSubproblems(subproblem) assert.EqualError(t, updch.Error.Err, err.Err.Error()) assert.Equal(t, err.Type, updch.Error.Type) @@ -3698,7 +3703,7 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "12345678", updch.Value) - err := NewError(ErrorBadAttestationStatementType, "x5c not present") + err := NewDetailedError(ErrorBadAttestationStatementType, "x5c not present") assert.EqualError(t, updch.Error.Err, err.Err.Error()) assert.Equal(t, err.Type, updch.Error.Type) @@ -3752,9 +3757,9 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "12345678", updch.Value) - err := NewError(ErrorBadAttestationStatementType, "permanent identifier does not match"). + err := NewDetailedError(ErrorBadAttestationStatementType, "permanent identifier does not match"). AddSubproblems(NewSubproblemWithIdentifier( - ErrorMalformedType, + ErrorRejectedIdentifierType, Identifier{Type: "permanent-identifier", Value: "12345678"}, "challenge identifier \"12345678\" doesn't match the attested hardware identifier \"87654321\"", )) @@ -3847,7 +3852,7 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "12345678", updch.Value) - err := NewError(ErrorBadAttestationStatementType, "unexpected attestation object format") + err := NewDetailedError(ErrorBadAttestationStatementType, `unsupported attestation object format "bogus-format"`) assert.EqualError(t, updch.Error.Err, err.Err.Error()) assert.Equal(t, err.Type, updch.Error.Type) diff --git a/acme/challenge_tpmsimulator_test.go b/acme/challenge_tpmsimulator_test.go index cb893b14..87db8631 100644 --- a/acme/challenge_tpmsimulator_test.go +++ b/acme/challenge_tpmsimulator_test.go @@ -237,7 +237,7 @@ func Test_deviceAttest01ValidateWithTPMSimulator(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "device.id.12345678", updch.Value) - err := NewError(ErrorBadAttestationStatementType, `version "bogus" is not supported`) + err := NewDetailedError(ErrorBadAttestationStatementType, `version "bogus" is not supported`) assert.EqualError(t, updch.Error.Err, err.Err.Error()) assert.Equal(t, err.Type, updch.Error.Type) @@ -282,9 +282,9 @@ func Test_deviceAttest01ValidateWithTPMSimulator(t *testing.T) { assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) assert.Equal(t, "device.id.99999999", updch.Value) - err := NewError(ErrorRejectedIdentifierType, `permanent identifier does not match`). + err := NewDetailedError(ErrorBadAttestationStatementType, `permanent identifier does not match`). AddSubproblems(NewSubproblemWithIdentifier( - ErrorMalformedType, + ErrorRejectedIdentifierType, Identifier{Type: "permanent-identifier", Value: "device.id.99999999"}, `challenge identifier "device.id.99999999" doesn't match any of the attested hardware identifiers ["device.id.12345678"]`, )) @@ -828,7 +828,7 @@ func Test_doTPMAttestationFormat(t *testing.T) { "certInfo": params.CreateAttestation, "pubArea": params.Public, }, - }}, nil, newBadAttestationStatementError("key authorization does not match")}, + }}, nil, newBadAttestationStatementError("key authorization invalid")}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/acme/errors.go b/acme/errors.go index 44f367a0..658ec6e0 100644 --- a/acme/errors.go +++ b/acme/errors.go @@ -293,6 +293,27 @@ type Subproblem struct { Identifier *Identifier `json:"identifier,omitempty"` } +// NewError creates a new Error. +func NewError(pt ProblemType, msg string, args ...interface{}) *Error { + return newError(pt, errors.Errorf(msg, args...)) +} + +// NewDetailedError creates a new Error that includes the error +// message in the details, providing more information to the +// ACME client. +func NewDetailedError(pt ProblemType, msg string, args ...interface{}) *Error { + return NewError(pt, msg, args...).withDetail() +} + +func (e *Error) withDetail() *Error { + if e == nil || e.Status >= 500 || e.Err == nil { + return e + } + + e.Detail = fmt.Sprintf("%s: %s", e.Detail, e.Err) + return e +} + // AddSubproblems adds the Subproblems to Error. It // returns the Error, allowing for fluent addition. func (e *Error) AddSubproblems(subproblems ...Subproblem) *Error { @@ -300,11 +321,6 @@ func (e *Error) AddSubproblems(subproblems ...Subproblem) *Error { return e } -// NewError creates a new Error type. -func NewError(pt ProblemType, msg string, args ...interface{}) *Error { - return newError(pt, errors.Errorf(msg, args...)) -} - // NewSubproblem creates a new Subproblem. The msg and args // are used to create a new error, which is set as the Detail, allowing // for more detailed error messages to be returned to the ACME client. @@ -368,6 +384,10 @@ func WrapError(typ ProblemType, err error, msg string, args ...interface{}) *Err } } +func WrapDetailedError(typ ProblemType, err error, msg string, args ...interface{}) *Error { + return WrapError(typ, err, msg, args...).withDetail() +} + // WrapErrorISE shortcut to wrap an internal server error type. func WrapErrorISE(err error, msg string, args ...interface{}) *Error { return WrapError(ErrorServerInternalType, err, msg, args...) diff --git a/acme/errors_test.go b/acme/errors_test.go new file mode 100644 index 00000000..8e586a12 --- /dev/null +++ b/acme/errors_test.go @@ -0,0 +1,54 @@ +package acme + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func mustJSON(t *testing.T, m map[string]interface{}) string { + t.Helper() + + b, err := json.Marshal(m) + require.NoError(t, err) + + return string(b) +} + +func TestError_WithAdditionalErrorDetail(t *testing.T) { + internalJSON := mustJSON(t, map[string]interface{}{ + "detail": "The server experienced an internal error", + "type": "urn:ietf:params:acme:error:serverInternal", + }) + malformedErr := NewError(ErrorMalformedType, "malformed error") // will result in Err == nil behavior + malformedJSON := mustJSON(t, map[string]interface{}{ + "detail": "The request message was malformed", + "type": "urn:ietf:params:acme:error:malformed", + }) + withDetailJSON := mustJSON(t, map[string]interface{}{ + "detail": "Attestation statement cannot be verified: invalid property", + "type": "urn:ietf:params:acme:error:badAttestationStatement", + }) + tests := []struct { + name string + err *Error + want string + }{ + {"internal", NewDetailedError(ErrorServerInternalType, ""), internalJSON}, + {"nil err", malformedErr, malformedJSON}, + {"detailed", NewDetailedError(ErrorBadAttestationStatementType, "invalid property"), withDetailJSON}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := json.Marshal(tt.err) + require.NoError(t, err) + + // tests if the additional error detail is included in the JSON representation + // of the ACME error. This is what is returned to ACME clients and being logged + // by the CA. + assert.JSONEq(t, tt.want, string(b)) + }) + } +}