diff --git a/docs/Contributing.md b/docs/Contributing.md index 45dbc59519..a3e0f1447e 100644 --- a/docs/Contributing.md +++ b/docs/Contributing.md @@ -86,6 +86,23 @@ go install github.com/sudorandom/protoc-gen-connect-openapi@latest Make sure your Go bin directory (usually `$HOME/go/bin`) is in your `PATH`. +## Cross-Platform Integration Testing (xtests) + +The [opentdf/tests](https://github.com/opentdf/tests) repo contains cross-SDK compatibility tests that validate platform changes against Go, Java, and JavaScript SDKs. Xtests run automatically on every platform PR via the `platform-xtest` CI job. + +### Running xtests against a platform feature branch + +To manually trigger xtests against your platform changes before merging: + +1. Go to the **Actions** tab in [opentdf/tests](https://github.com/opentdf/tests/actions) +2. Find the **xtest** workflow +3. Click the **"Run workflow"** dropdown on the right +4. Set **"Use workflow from"** to the xtest branch to run (`main`, or a companion xtest branch if you have test changes) +5. Set **"platform ref branch"** to the HEAD commit SHA of your platform feature branch +6. Click **Run workflow** + +This is especially useful when your platform changes affect SDK error handling, KAS behavior, or the rewrap flow — areas where cross-SDK compatibility matters. + ## Advice for Code Contributors * Make sure to run our linters with `make lint` diff --git a/sdk/tdf.go b/sdk/tdf.go index dbfdedabf3..db22208b5f 100644 --- a/sdk/tdf.go +++ b/sdk/tdf.go @@ -1568,11 +1568,26 @@ func createKaoTemplateFromKasInfo(kasInfoArr []KASInfo) []kaoTpl { return kaoTemplate } +// getKasErrorToReturn classifies KAS rewrap errors. KAS intentionally uses a +// generic "bad request" for policy binding and DEK failures to avoid leaking +// information about secret key material. Descriptive messages indicate +// client/configuration issues that are safe to surface as non-tamper errors. func getKasErrorToReturn(err error, defaultError error) error { errToReturn := defaultError - if strings.Contains(err.Error(), codes.InvalidArgument.String()) { - errToReturn = errors.Join(ErrRewrapBadRequest, errToReturn) - } else if strings.Contains(err.Error(), codes.PermissionDenied.String()) { + errStr := err.Error() + + switch { + case strings.Contains(errStr, codes.InvalidArgument.String()): + // Per-KAO errors are serialized as plain strings through the proto + // response, so we match on a substring anchored to the gRPC status + // description. Generic "bad request" = potential tamper; anything + // else = client/configuration error. + if strings.Contains(errStr, kasGenericBadRequest) { + errToReturn = errors.Join(ErrRewrapBadRequest, errToReturn) + } else { + errToReturn = errors.Join(ErrKASRequestError, errToReturn) + } + case strings.Contains(errStr, codes.PermissionDenied.String()): errToReturn = errors.Join(ErrRewrapForbidden, errToReturn) } diff --git a/sdk/tdf_test.go b/sdk/tdf_test.go index b1d5c0acf3..0aa58daa3f 100644 --- a/sdk/tdf_test.go +++ b/sdk/tdf_test.go @@ -3135,17 +3135,62 @@ func TestIsLessThanSemver(t *testing.T) { func TestGetKasErrorToReturn(t *testing.T) { defaultError := errors.New("default KAS error") - t.Run("InvalidArgument error returns ErrRewrapBadRequest", func(t *testing.T) { - inputError := errors.New("rpc error: code = InvalidArgument desc = invalid request") + t.Run("generic InvalidArgument (bad request) is potential tamper", func(t *testing.T) { + inputError := errors.New("rpc error: code = InvalidArgument desc = bad request") result := getKasErrorToReturn(inputError, defaultError) require.ErrorIs(t, result, ErrRewrapBadRequest) + require.ErrorIs(t, result, ErrTampered, "generic bad request may be policy binding tamper") + require.NotErrorIs(t, result, ErrKASRequestError) require.ErrorIs(t, result, defaultError) }) + t.Run("specific InvalidArgument is misconfiguration", func(t *testing.T) { + for _, msg := range []string{ + "unsupported key type", + "key access object is nil", + "ec-wrapped not enabled", + "wrapped key is empty", + "invalid ephemeral public key", + "unsupported EC key size", + "invalid ephemeral public key PEM", + "ephemeral key is not EC", + "invalid EC public key", + "no legacy key IDs found", + "failed to get additional rewrap context", + } { + t.Run(msg, func(t *testing.T) { + inputError := errors.New("rpc error: code = InvalidArgument desc = " + msg) + result := getKasErrorToReturn(inputError, defaultError) + require.ErrorIs(t, result, ErrKASRequestError) + require.NotErrorIs(t, result, ErrTampered, "specific KAS 400 must not match ErrTampered") + require.ErrorIs(t, result, defaultError) + }) + } + }) + + t.Run("message containing bad request as substring is still tamper", func(t *testing.T) { + // "desc = bad request body" contains "desc = bad request" as a substring, + // so it should be classified as potential tamper (conservative approach). + inputError := errors.New("rpc error: code = InvalidArgument desc = bad request body") + result := getKasErrorToReturn(inputError, defaultError) + require.ErrorIs(t, result, ErrRewrapBadRequest) + require.ErrorIs(t, result, ErrTampered, "substring match should err on side of tamper") + }) + + t.Run("middleware injecting bad request without desc prefix is not tamper", func(t *testing.T) { + // A message like "bad request: unsupported key type" without the "desc = " + // prefix should NOT trigger tamper classification. + inputError := errors.New("rpc error: code = InvalidArgument bad request: unsupported key type") + result := getKasErrorToReturn(inputError, defaultError) + require.ErrorIs(t, result, ErrKASRequestError) + require.NotErrorIs(t, result, ErrTampered, "unanchored bad request should not match") + }) + t.Run("PermissionDenied error returns ErrRewrapForbidden", func(t *testing.T) { inputError := errors.New("rpc error: code = PermissionDenied desc = access denied") result := getKasErrorToReturn(inputError, defaultError) require.ErrorIs(t, result, ErrRewrapForbidden) + require.NotErrorIs(t, result, ErrKASRequestError, "403 is authorization, not misconfiguration") require.ErrorIs(t, result, defaultError) }) diff --git a/sdk/tdferrors.go b/sdk/tdferrors.go index f19331fa82..37a69a23ac 100644 --- a/sdk/tdferrors.go +++ b/sdk/tdferrors.go @@ -18,9 +18,28 @@ var ( ErrSegSigValidation = fmt.Errorf("[%w] tdf: failed integrity check on segment hash", ErrTampered) ErrTDFPayloadReadFail = fmt.Errorf("[%w] tdf: fail to read payload from tdf", ErrTampered) ErrTDFPayloadInvalidOffset = fmt.Errorf("[%w] sdk.Reader.ReadAt: negative offset", ErrTampered) - ErrRewrapBadRequest = fmt.Errorf("[%w] tdf: rewrap request 400", ErrTampered) ErrRootSignatureFailure = fmt.Errorf("[%w] tdf: issue verifying root signature", ErrTampered) - ErrRewrapForbidden = errors.New("tdf: rewrap request 403") + ErrRewrapBadRequest = fmt.Errorf("[%w] tdf: rewrap request 400", ErrTampered) + + // kasGenericBadRequest is the substring the SDK looks for in serialized + // KAS 400 errors to identify potential tamper. KAS uses the generic message + // "bad request" for errors involving secret key material (policy binding, + // DEK decryption). Per-KAO errors are serialized as plain strings through + // the proto response (not as gRPC status errors), so substring matching is + // the only classification mechanism available. + // + // The "desc = " prefix anchors the match to the gRPC status description + // field, avoiding false positives from middleware or error wrapping that + // might incidentally contain "bad request". + // + // Must stay in sync with the "bad request" message in + // service/kas/access/rewrap.go — and descriptive KAS messages must NOT + // contain this substring. + kasGenericBadRequest = "desc = bad request" + + // KAS request errors — client/configuration issues, not integrity failures + ErrKASRequestError = errors.New("tdf: KAS request error") + ErrRewrapForbidden = errors.New("tdf: rewrap request 403") ) // Custom error struct for Assertion errors diff --git a/service/kas/access/rewrap.go b/service/kas/access/rewrap.go index 274e09e256..d62975a480 100644 --- a/service/kas/access/rewrap.go +++ b/service/kas/access/rewrap.go @@ -101,6 +101,18 @@ const ( errNoValidKeyAccessObjects = Error("no valid KAOs") ) +// Error helpers for KAS rewrap responses. +// +// SECURITY: Policy binding verification and DEK decryption failures MUST use the +// generic "bad request" message to avoid leaking information about computations +// involving secret key material. Non-secret failures (malformed input, unsupported +// key types, missing fields) SHOULD use descriptive messages so the SDK can +// distinguish misconfiguration from potential tamper. +// +// The SDK matches on the substring "desc = bad request" in serialized per-KAO +// errors to identify potential tamper (see sdk/tdferrors.go kasGenericBadRequest). +// Do not change the generic "bad request" message without updating the SDK +// constant, and do not use "bad request" in descriptive error messages. func err400(s string) error { return connect.NewError(connect.CodeInvalidArgument, errors.Join(ErrUser, status.Error(codes.InvalidArgument, s))) } @@ -432,7 +444,7 @@ func verifyPolicyBinding(ctx context.Context, policy []byte, kao *kaspb.Unsigned if !hmac.Equal(actualHMAC, expectedHMAC) { //nolint:sloglint // usage of camelCase is intentional logger.WarnContext(ctx, "policy hmac mismatch", slog.String("policyBinding", policyBinding)) - return err400("bad request") + return err400("bad request") // Generic: involves secret key material } return nil @@ -566,7 +578,7 @@ func (p *Provider) Rewrap(ctx context.Context, req *connect.Request[kaspb.Rewrap additionalRewrapContext, err := getAdditionalRewrapContext(req.Header()) if err != nil { p.Logger.WarnContext(ctx, "failed to get additional rewrap context", slog.Any("error", err)) - return nil, err400(err.Error()) + return nil, err400("failed to get additional rewrap context") } resp.SessionPublicKey, results, err = p.tdf3Rewrap(ctx, tdf3Reqs, body.GetClientPublicKey(), entityInfo, additionalRewrapContext) if err != nil { @@ -633,14 +645,14 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned for _, kao := range req.GetKeyAccessObjects() { if policyErr != nil { - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("bad request")) // Generic: corrupted policy body may indicate tamper continue } // Check if KeyAccessObject is nil if kao.GetKeyAccessObject() == nil { p.Logger.WarnContext(ctx, "key access object is nil", slog.String("kao_id", kao.GetKeyAccessObjectId())) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("key access object is nil")) continue } @@ -648,7 +660,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned wrappedKey := kao.GetKeyAccessObject().GetWrappedKey() if len(wrappedKey) == 0 { p.Logger.WarnContext(ctx, "wrapped key is empty", slog.String("kao_id", kao.GetKeyAccessObjectId())) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("wrapped key is empty")) continue } @@ -659,7 +671,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned if !p.ECTDFEnabled && !p.Preview.ECTDFEnabled { p.Logger.WarnContext(ctx, "ec-wrapped not enabled") - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("ec-wrapped not enabled")) continue } @@ -674,7 +686,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned slog.Any("kao", kao), slog.Any("error", err), ) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("invalid ephemeral public key")) continue } @@ -685,7 +697,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned slog.Any("kao", kao), slog.Any("error", err), ) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("unsupported EC key size")) continue } @@ -697,7 +709,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned slog.Any("kao", kao), slog.Any("error", err), ) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("invalid ephemeral public key PEM")) continue } @@ -708,14 +720,14 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned slog.Any("kao", kao), slog.Any("error", err), ) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("invalid ephemeral public key")) continue } ecPub, ok := pub.(*ecdsa.PublicKey) if !ok { p.Logger.WarnContext(ctx, "not an EC public key", slog.Any("error", err)) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("ephemeral key is not EC")) continue } @@ -723,7 +735,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned compressedKey, err := ocrypto.CompressedECPublicKey(mode, *ecPub) if err != nil { p.Logger.WarnContext(ctx, "failed to compress public key", slog.Any("error", err)) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("invalid EC public key")) continue } @@ -743,7 +755,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned kidsToCheck = p.listLegacyKeys(ctx) if len(kidsToCheck) == 0 { p.Logger.WarnContext(ctx, "failure to find legacy kids for rsa") - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("no legacy key IDs found")) continue } } @@ -762,12 +774,12 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned p.Logger.WarnContext(ctx, "unsupported key type", slog.String("key_type", keyType), slog.String("kao_id", kao.GetKeyAccessObjectId())) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("unsupported key type")) continue } if err != nil { p.Logger.WarnContext(ctx, "failure to decrypt dek", slog.Any("error", err)) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("bad request")) // Generic: involves secret key material continue } @@ -784,7 +796,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned n, err := base64.StdEncoding.Decode(policyBinding, []byte(policyBindingB64Encoded)) if err != nil { p.Logger.WarnContext(ctx, "invalid policy binding encoding", slog.Any("error", err)) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("bad request")) // Generic: malformed binding may indicate tamper continue } if n == 64 { //nolint:mnd // 32 bytes of hex encoded data = 256 bit sha-2 @@ -800,7 +812,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned // Verify policy binding using the UnwrappedKeyData interface if err := dek.VerifyBinding(ctx, []byte(req.GetPolicy().GetBody()), policyBinding); err != nil { p.Logger.WarnContext(ctx, "failure to verify policy binding", slog.Any("error", err)) - failedKAORewrap(results, kao, err400("bad request")) + failedKAORewrap(results, kao, err400("bad request")) // Generic: involves secret key material continue } @@ -866,12 +878,12 @@ func (p *Provider) tdf3Rewrap(ctx context.Context, requests []*kaspb.UnsignedRew continue } policy, kaoResults, err := p.verifyRewrapRequests(ctx, req) - if err != nil && !errors.Is(err, errNoValidKeyAccessObjects) { - return "", nil, err400("invalid request") - } policyID := req.GetPolicy().GetId() results[policyID] = kaoResults if err != nil { + // Store per-KAO results even on error so tamper signals (e.g. corrupted + // policy body → generic "bad request") reach the SDK rather than being + // replaced by a top-level "invalid request". p.Logger.WarnContext(ctx, "rewrap: verifyRewrapRequests failed", slog.String("policy_id", policyID),