From 5fecc2bd87eecf7c1d6f1ee1ef43c9edd0535e7d Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 5 Jul 2024 15:43:40 +0200 Subject: [PATCH] Fix HTTP internal server error when bad attestation object is provided --- acme/challenge.go | 21 ++++- acme/challenge_test.go | 202 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 213 insertions(+), 10 deletions(-) diff --git a/acme/challenge.go b/acme/challenge.go index fcdab7d3e..e6b1fd300 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -1,6 +1,7 @@ package acme import ( + "bytes" "context" "crypto" "crypto/ecdsa" @@ -397,12 +398,26 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose attObj, err := base64.RawURLEncoding.DecodeString(p.AttObj) if err != nil { - return WrapErrorISE(err, "error base64 decoding attObj") + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "failed base64 decoding attObj %q", p.AttObj)) + } + + if len(attObj) == 0 || bytes.Equal(attObj, []byte("{}")) { + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "attObj must not be empty")) + } + + cborDecoderOptions := cbor.DecOptions{} + cborDecoder, err := cborDecoderOptions.DecMode() + if err != nil { + return WrapErrorISE(err, "failed creating CBOR decoder") + } + + if err := cborDecoder.Wellformed(attObj); err != nil { + return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "attObj is not well formed CBOR: %v", err)) } att := attestationObject{} - if err := cbor.Unmarshal(attObj, &att); err != nil { - return WrapErrorISE(err, "error unmarshalling CBOR") + if err := cborDecoder.Unmarshal(attObj, &att); err != nil { + return WrapErrorISE(err, "failed unmarshalling CBOR") } format := att.Format diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 71164739a..9f5c16aa1 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -3268,10 +3268,50 @@ func Test_deviceAttest01Validate(t *testing.T) { AttObj: "?!", }) require.NoError(t, err) - errorCBORPayload, err := json.Marshal(struct { + emptyPayload, err := json.Marshal(struct { AttObj string `json:"attObj"` }{ - AttObj: "AAAA", + AttObj: base64.RawURLEncoding.EncodeToString([]byte("")), + }) + require.NoError(t, err) + emptyObjectPayload, err := json.Marshal(struct { + AttObj string `json:"attObj"` + }{ + AttObj: base64.RawURLEncoding.EncodeToString([]byte("{}")), + }) + require.NoError(t, err) + attObj, err := cbor.Marshal(struct { + Format string `json:"fmt"` + AttStatement map[string]interface{} `json:"attStmt,omitempty"` + }{ + Format: "step", + AttStatement: map[string]interface{}{ + "alg": -7, + "sig": "", + }, + }) + require.NoError(t, err) + errorNonWellformedCBORPayload, err := json.Marshal(struct { + AttObj string `json:"attObj"` + }{ + AttObj: base64.RawURLEncoding.EncodeToString(attObj[:len(attObj)-1]), // cut the CBOR encoded data off + }) + require.NoError(t, err) + unsupportedFormatAttObj, err := cbor.Marshal(struct { + Format string `json:"fmt"` + AttStatement map[string]interface{} `json:"attStmt,omitempty"` + }{ + Format: "unsupported-format", + AttStatement: map[string]interface{}{ + "alg": -7, + "sig": "", + }, + }) + require.NoError(t, err) + errorUnsupportedFormat, err := json.Marshal(struct { + AttObj string `json:"attObj"` + }{ + AttObj: base64.RawURLEncoding.EncodeToString(unsupportedFormatAttObj), }) require.NoError(t, err) type args struct { @@ -3408,7 +3448,7 @@ func Test_deviceAttest01Validate(t *testing.T) { wantErr: nil, } }, - "fail/base64-decode": func(t *testing.T) test { + "ok/base64-decode": func(t *testing.T) test { return test{ args: args{ ch: &Challenge{ @@ -3424,13 +3464,67 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, "azID", id) return &Authorization{ID: "azID"}, nil }, + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equal(t, "chID", updch.ID) + assert.Equal(t, "token", updch.Token) + assert.Equal(t, StatusInvalid, updch.Status) + assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) + assert.Equal(t, "12345678", updch.Value) + + err := NewDetailedError(ErrorBadAttestationStatementType, "failed base64 decoding attObj %q", "?!") + + assert.EqualError(t, updch.Error.Err, err.Err.Error()) + assert.Equal(t, err.Type, updch.Error.Type) + assert.Equal(t, err.Detail, updch.Error.Detail) + assert.Equal(t, err.Status, updch.Error.Status) + assert.Equal(t, err.Subproblems, updch.Error.Subproblems) + + return nil + }, }, payload: errorBase64Payload, }, - wantErr: NewErrorISE("error base64 decoding attObj: illegal base64 data at input byte 0"), } }, - "fail/cbor.Unmarshal": func(t *testing.T) test { + "ok/empty-attobj": func(t *testing.T) test { + return test{ + args: args{ + ch: &Challenge{ + ID: "chID", + AuthorizationID: "azID", + Token: "token", + Type: "device-attest-01", + Status: StatusPending, + Value: "12345678", + }, + db: &MockDB{ + MockGetAuthorization: func(ctx context.Context, id string) (*Authorization, error) { + assert.Equal(t, "azID", id) + return &Authorization{ID: "azID"}, nil + }, + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equal(t, "chID", updch.ID) + assert.Equal(t, "token", updch.Token) + assert.Equal(t, StatusInvalid, updch.Status) + assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) + assert.Equal(t, "12345678", updch.Value) + + err := NewDetailedError(ErrorBadAttestationStatementType, "attObj must not be empty") + + assert.EqualError(t, updch.Error.Err, err.Err.Error()) + assert.Equal(t, err.Type, updch.Error.Type) + assert.Equal(t, err.Detail, updch.Error.Detail) + assert.Equal(t, err.Status, updch.Error.Status) + assert.Equal(t, err.Subproblems, updch.Error.Subproblems) + + return nil + }, + }, + payload: emptyPayload, + }, + } + }, + "ok/empty-json-attobj": func(t *testing.T) test { return test{ args: args{ ch: &Challenge{ @@ -3446,10 +3540,104 @@ func Test_deviceAttest01Validate(t *testing.T) { assert.Equal(t, "azID", id) return &Authorization{ID: "azID"}, nil }, + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equal(t, "chID", updch.ID) + assert.Equal(t, "token", updch.Token) + assert.Equal(t, StatusInvalid, updch.Status) + assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) + assert.Equal(t, "12345678", updch.Value) + + err := NewDetailedError(ErrorBadAttestationStatementType, "attObj must not be empty") + + assert.EqualError(t, updch.Error.Err, err.Err.Error()) + assert.Equal(t, err.Type, updch.Error.Type) + assert.Equal(t, err.Detail, updch.Error.Detail) + assert.Equal(t, err.Status, updch.Error.Status) + assert.Equal(t, err.Subproblems, updch.Error.Subproblems) + + return nil + }, + }, + payload: emptyObjectPayload, + }, + } + }, + "ok/cborDecoder.Wellformed": func(t *testing.T) test { + return test{ + args: args{ + ch: &Challenge{ + ID: "chID", + AuthorizationID: "azID", + Token: "token", + Type: "device-attest-01", + Status: StatusPending, + Value: "12345678", + }, + db: &MockDB{ + MockGetAuthorization: func(ctx context.Context, id string) (*Authorization, error) { + assert.Equal(t, "azID", id) + return &Authorization{ID: "azID"}, nil + }, + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equal(t, "chID", updch.ID) + assert.Equal(t, "token", updch.Token) + assert.Equal(t, StatusInvalid, updch.Status) + assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) + assert.Equal(t, "12345678", updch.Value) + + err := NewDetailedError(ErrorBadAttestationStatementType, "attObj is not well formed CBOR: unexpected EOF") + + assert.EqualError(t, updch.Error.Err, err.Err.Error()) + assert.Equal(t, err.Type, updch.Error.Type) + assert.Equal(t, err.Detail, updch.Error.Detail) + assert.Equal(t, err.Status, updch.Error.Status) + assert.Equal(t, err.Subproblems, updch.Error.Subproblems) + + return nil + }, + }, + payload: errorNonWellformedCBORPayload, + }, + } + }, + "ok/unsupported-attestation-format": func(t *testing.T) test { + ctx := NewProvisionerContext(context.Background(), mustNonAttestationProvisioner(t)) + return test{ + args: args{ + ctx: ctx, + ch: &Challenge{ + ID: "chID", + AuthorizationID: "azID", + Token: "token", + Type: "device-attest-01", + Status: StatusPending, + Value: "12345678", + }, + db: &MockDB{ + MockGetAuthorization: func(ctx context.Context, id string) (*Authorization, error) { + assert.Equal(t, "azID", id) + return &Authorization{ID: "azID"}, nil + }, + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equal(t, "chID", updch.ID) + assert.Equal(t, "token", updch.Token) + assert.Equal(t, StatusInvalid, updch.Status) + assert.Equal(t, ChallengeType("device-attest-01"), updch.Type) + assert.Equal(t, "12345678", updch.Value) + + err := NewDetailedError(ErrorBadAttestationStatementType, "unsupported attestation object format %q", "unsupported-format") + + assert.EqualError(t, updch.Error.Err, err.Err.Error()) + assert.Equal(t, err.Type, updch.Error.Type) + assert.Equal(t, err.Detail, updch.Error.Detail) + assert.Equal(t, err.Status, updch.Error.Status) + assert.Equal(t, err.Subproblems, updch.Error.Subproblems) + + return nil + }, }, - payload: errorCBORPayload, + payload: errorUnsupportedFormat, }, - wantErr: NewErrorISE("error unmarshalling CBOR: cbor:"), } }, "ok/prov.IsAttestationFormatEnabled": func(t *testing.T) test {