Skip to content

fix(sdk,cmdline): DSPX-1561 pass the default assertion verification key to the SDK - #435

Open
dmihalcik-virtru wants to merge 1 commit into
mainfrom
DSPX-1561-default-key
Open

dmihalcik-virtru wants to merge 1 commit into
mainfrom
DSPX-1561-default-key

Conversation

@dmihalcik-virtru

@dmihalcik-virtru dmihalcik-virtru commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Proposed Changes

  • The decrypt command's --with-assertion-verification-keys JSON now converts defaultKey (or DefaultKey) like the per-ID keys: an HS256 key is the UTF-8 bytes of its string, an RS256 key a PEM public key. The SDK already falls back to the default key for assertions without a per-ID key, but the CLI passed the key on as unconverted text.
  • A keys file that can't be used as written fails with reason=configuration-error instead of silently dropping a key: an unknown top-level member (such as default_key), an alg other than RS256 or HS256, a key with members but no alg, an unreadable key, or PEM text given as an HS256 secret. A null or empty per-ID entry stays an entry, so that assertion fails rather than falling back to another key.
  • Docs: the SDK javadoc for Config.AssertionVerificationKeys and the decrypt help explain that a default key applies to every assertion without a per-ID key, including payload-key assertions such as the system-metadata assertion this SDK writes, so those fail; use per-ID keys for files that carry both.

Split out of #432 (draft; the complete change). Cross-SDK counterparts: opentdf/platform#4304, opentdf/web-sdk#1138.

Compatibility: keys files with unknown top-level members, unsupported algorithms, or PEM text as an HS256 secret are now rejected.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

  • mvn -pl sdk,cmdline -am test (new cases in cmdline/.../AssertionVerificationKeysTest).

…ey to the SDK

The decrypt command's assertion verification keys file now converts
defaultKey (or DefaultKey) the same way as per-ID keys: an HS256 key is
the UTF-8 bytes of its string and an RS256 key is a PEM public key. The
SDK already verifies an assertion with no per-ID key with the default
key, but the CLI passed it on as unconverted text.

A keys file that cannot be used as written is reported with
reason=configuration-error rather than leaving a key out: an unknown
top-level member, an alg other than RS256 or HS256, a key with members
but no alg, a key that cannot be read, or PEM text as an HS256 secret.
A per-ID entry that is null or empty stays an entry, so verifying that
assertion fails instead of falling back to another key.

Document, in the SDK javadoc and the decrypt command's help, that a
default key applies to every assertion without a per-ID key, including
payload-key assertions such as the system-metadata assertion the Java
writer adds, so those fail verification; point to per-ID keys instead.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@dmihalcik-virtru
dmihalcik-virtru requested review from a team as code owners November 9, 2026 14:50
@coderabbitai

coderabbitai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 99170ccb-5a5a-4c22-a64c-6078e22eaef4
Commits

Reviewing files that changed from the base of the PR and between 4092636 and 0be0b33.

Files selected for processing: 3

  • cmdline/src/main/java/io/opentdf/platform/Command.java
  • cmdline/src/test/java/io/opentdf/platform/AssertionVerificationKeysTest.java
  • sdk/src/main/java/io/opentdf/platform/sdk/Config.java
  • 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

@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant