Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions docs/Contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
21 changes: 18 additions & 3 deletions sdk/tdf.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
49 changes: 47 additions & 2 deletions sdk/tdf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down
23 changes: 21 additions & 2 deletions sdk/tdferrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 32 additions & 20 deletions service/kas/access/rewrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -633,22 +645,22 @@ 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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// 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
}

// Check if wrapped key is empty
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
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -708,22 +720,22 @@ 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
}

// Compress the public key
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
}

Expand All @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
Expand All @@ -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
}

Expand All @@ -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
Comment thread
marythought marked this conversation as resolved.
}
if n == 64 { //nolint:mnd // 32 bytes of hex encoded data = 256 bit sha-2
Expand All @@ -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
}

Expand Down Expand Up @@ -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),
Expand Down
Loading