fix(kas): accept spec-compliant policy bindings by pflynn-virtru · Pull Request #4223 · opentdf/platform

pflynn-virtru · 2026-10-14T19:02:58Z

KAS now accepts both Base64(HMAC) (spec) and legacy Base64(hex(HMAC)). Service-only; no output changes.

Fixes the raw-binding path broken since #2095, which compared an untrimmed 33-byte buffer against the 32-byte HMAC, so no released KAS accepted the spec form.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Corrected handling of policy-binding hashes in both standard and legacy formats, reducing errors when validating rewrap requests.
- Malformed policy-binding values now receive a clear bad-request response, while processing continues for other requests.

The spec defines the policy binding as Base64(HMAC), but no released KAS can decode that form. #2095 added a dual-accept branch and it has never worked: base64.DecodedLen over-allocates, so a 44-character binding decodes 32 bytes into a 33-byte buffer. Only the hex branch replaced that buffer with a right-sized slice; the raw branch passed the untrimmed 33 bytes to VerifyBinding, which compares with hmac.Equal -- length sensitive, so it always failed. Extract decodePolicyBinding, trimmed to the bytes actually decoded, and use it on the rewrap path. Legacy hex stays accepted indefinitely; TDFs are archival. This only widens what KAS accepts and changes no output, so it is safe to deploy on its own. It is a prerequisite for fixing the SDK writer, which is deliberately left for a follow-up: nothing may emit raw bindings until a KAS that can read them is deployed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

coderabbitai · 2026-10-14T19:03:08Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: e42866c9-4010-45f3-b9df-3833c02ca52f

📥 Commits

Reviewing files that changed from the base of the PR and between beba549 and cae7caa.

📒 Files selected for processing: 2

service/kas/access/rewrap.go
service/kas/access/rewrap_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The change adds a policy-binding decoder that supports raw HMAC bytes and legacy hexadecimal values. Rewrap verification uses the decoder and records a per-KAO bad request error when decoding fails. Tests cover both encodings, invalid Base64, and decoded output length.

Changes: Policy-binding decoding

Layer / File(s)	Summary
Decode and verify policy bindings `service/kas/access/rewrap.go`, `service/kas/access/rewrap_test.go`	A new helper decodes Base64 policy bindings and decodes 64-byte results as hexadecimal when valid. Rewrap verification records a per-KAO `bad request` error on decode failure and continues processing. Tests cover raw HMAC bytes, legacy hexadecimal values, invalid Base64, and exact decoded length.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: dmihalcik-virtru

Merge Risk: ⚪ Minimal · up to cae7c

The supported binding formats and reported regression are covered; no identified issue prevents merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: KAS now accepts spec-compliant policy bindings while preserving legacy support.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{A rabbit checks each encoded byte,
Raw HMACs decode just right.
Old hex paths still pass the test,
Bad input gets a clear request.
Then onward hops each KAO,
While carrots wait below.}

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-10-14T19:05:16Z

X-Test Failure Report

govulncheck-failure-7
govulncheck-failure-0
govulncheck-failure-1
govulncheck-failure-4
govulncheck-failure-2

github-actions · 2026-10-14T19:06:46Z

Benchmark results, click to expand

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	247.686469ms
Throughput	403.74 requests/second

TDF3 Benchmark Results

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	27.884015912s
Average Latency	278.324873ms
Throughput	179.31 requests/second

github-actions · 2026-10-14T19:09:34Z

Benchmark results, click to expand

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	356.290517ms
Throughput	280.67 requests/second

TDF3 Benchmark Results

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	40.067344293s
Average Latency	399.85585ms
Throughput	124.79 requests/second

github-actions · 2026-10-14T19:11:34Z

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

otdfctl
sdk
service
lib/fixtures
tests-bdd

See the workflow run for details.

github-actions · 2026-10-14T19:18:26Z

The point is to show that a real KAS -- the current release and main -- rejects an on-spec policy binding. A force-only gate skipped exactly those cells, so drop it. The test fails until a platform carrying opentdf/platform#4223 is under test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

KAS must accept a policy binding in the spec's Base64(HMAC) form as well as the legacy Base64(hex(HMAC)) every SDK writes. No released KAS did (opentdf/platform#2095 compared an untrimmed buffer); fixed in opentdf/platform#4223. test_tdf_with_spec_policy_binding re-encodes the binding to the spec form, which needs no key, and expects decrypt to succeed. It is gated on the new platform-only feature spec_policy_binding, force-only until a release carries the fix. test_tdf_with_altered_spec_policy_binding expects a well-formed spec binding with a wrong HMAC to be rejected, so the new path cannot fail open. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

The point is to show that a real KAS -- the current release and main -- rejects an on-spec policy binding. A force-only gate skipped exactly those cells, so drop it. The test fails until a platform carrying opentdf/platform#4223 is under test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

github-actions Bot added comp:sdk A software development kit, including library, for client applications and inter-service communicati comp:kas Key Access Server size/m labels Oct 14, 2026

pflynn-virtru force-pushed the fix/policy-binding-backcompat branch from d73c7fa to cae7caa Compare October 14, 2026 19:04

pflynn-virtru changed the title ~~fix(core): backwards-compatible spec-compliant policy bindings~~ fix(kas): accept spec-compliant policy bindings Oct 14, 2026

pflynn-virtru marked this pull request as ready for review October 14, 2026 19:07

pflynn-virtru requested review from a team as code owners October 14, 2026 19:07

pflynn-virtru mentioned this pull request Oct 14, 2026

feat(xtest): cover hexless policy bindings, Base64(HMAC) opentdf/tests#648

Open

dmihalcik-virtru approved these changes Oct 15, 2026

View reviewed changes

pflynn-virtru added this pull request to the merge queue Oct 15, 2026

Merged via the queue into main with commit d937028 Oct 15, 2026
50 checks passed

pflynn-virtru deleted the fix/policy-binding-backcompat branch October 15, 2026 14:08

arkavo-com mentioned this pull request Oct 28, 2026

fix(xtest): align community SDKs with TDF container naming arkavo-org/opentdf-tests#15

Merged

Navigation Menu

Repository navigation

fix(kas): accept spec-compliant policy bindings - #4223

fix(kas): accept spec-compliant policy bindings#4223
pflynn-virtru merged 1 commit into
mainfrom
fix/policy-binding-backcompat

pflynn-virtru commented Oct 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 14, 2026

Uh oh!

github-actions Bot commented Oct 14, 2026

Bulk Benchmark Results

TDF3 Benchmark Results

Uh oh!

github-actions Bot commented Oct 14, 2026

Bulk Benchmark Results

TDF3 Benchmark Results

Uh oh!

github-actions Bot commented Oct 14, 2026

Uh oh!

github-actions Bot commented Oct 14, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pflynn-virtru commented Oct 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 14, 2026

X-Test Failure Report

Uh oh!

github-actions Bot commented Oct 14, 2026

Bulk Benchmark Results

TDF3 Benchmark Results

Uh oh!

github-actions Bot commented Oct 14, 2026

Bulk Benchmark Results

TDF3 Benchmark Results

Uh oh!

github-actions Bot commented Oct 14, 2026

⚠️ Govulncheck found vulnerabilities ⚠️

Uh oh!

github-actions Bot commented Oct 14, 2026

X-Test Results

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pflynn-virtru commented Oct 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading