Repository navigation
fix(sdk): DSPX-1561 align TDF manifest parsing and assertion verification across SDKs - #432
Draft
dmihalcik-virtru wants to merge 6 commits into
Draft
dmihalcik-virtru wants to merge 6 commits into
dmihalcik-virtru wants to merge 6 commits into
Conversation
…eceived Apply the JSON profile of the ZTDF assertions specification (Section 3) while the manifest streams: duplicate member names anywhere, case variants of modelled member names at the root, assertion, statement and binding levels, and non-I-JSON assertion content are rejected before a typed field or JSON tree can keep only one value. The manifest is read as strict JSON, decoded as strict UTF-8, and may not start with a byte order mark. Each assertion keeps the JSON object it was parsed from. assertionHash is computed over that object minus only its top-level binding (Section 4), never over a re-serialization of the typed model, and the object is written back unchanged when a manifest is serialized. Writers hash the wire form they write, emit binding method "jws", and refuse, as a configuration error, an assertion that a reader would reject while parsing. Add SDK.AssertionFailureException, carrying the assertion id and an AssertionFailureReason with the Section 9 codes, and the public Manifest.Assertion.computeAssertionHash(String). Ids and details in its message are escaped and length-limited. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…tion only Each assertion is verified with exactly one key, selected once by its exact id: the per-ID key, else the default key, else the payload key with HS256 (spec Section 6.1). A failure is final, and an empty or invalid configured key, including PEM text given as an HS256 secret, is a configuration error. Each binding is verified only with the key selected from configuration and that key's algorithm; an embedded jwk or x5c must match it (Section 6.2). Embedded keys that are symmetric, private, malformed for their key type, not canonically encoded, or contradictory, and any embedded key in HMAC mode, are rejected. The JWS alg must equal the selected key's algorithm, crit headers and duplicate JOSE members are rejected, base64 is decoded strictly, claims other than assertionHash and assertionSig are ignored, and both binding forms of Section 5.1 are recognized, with binding objects reported as unsupported. Checks follow the Section 8 order, and every failure is reported as an SDK.AssertionFailureException with its reason. JSON that does not parse inside an assertion is reported as invalid-json. Key selection stays inside loadTDF, and Manifest.Assertion.verify has no side effects. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…o 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. 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. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Vendor xtest/vectors/assertions-phase0.json from opentdf/tests (the DSPX-5520 shared vectors) byte for byte, and run every vector: assertion hashing and Section 3 parsing, manifest-level parsing, binding forms, and Section 8 verification with a selected key. Required vectors must pass; proposed ones are logged. The assertion vectors also run through the manifest reader, spliced into a TDF. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
The payload key is assembled only from key access objects. A manifest whose encryptionInformation.keyAccess is empty or absent is now refused as a malformed TDF when it is loaded, before any KAS, integrity or assertion step, with a message that says it has no key access objects. Key reconstruction also fails if no split is found. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…fication key applies to A default assertion verification 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. Say so in the SDK javadoc and the decrypt command's help, and point to per-ID keys instead. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. |
This was referenced Nov 9, 2026
|
Contributor
This was referenced Nov 9, 2026
dmihalcik-virtru
added a commit
that referenced
this pull request
Nov 12, 2026
### Proposed Changes
* `loadTDF` refuses a manifest whose `encryptionInformation.keyAccess`
is empty or absent with `SDK.MalformedTDFException` ("no key access
objects"), before any KAS request, integrity check, or assertion check.
The payload key is assembled only from key access objects, so such a
file can't be decrypted.
* Key reconstruction also fails with `SDK.SplitKeyException` when no
split was found, so no path continues with the all-zero starting value
of the payload key.
* `SDK.readManifest`, `isTDF`, and manifest inspection are unchanged.
Split out of #432 (draft; the complete change). Cross-SDK counterparts:
opentdf/platform#4302, opentdf/web-sdk#1136; cross-SDK test:
opentdf/tests#703.
**Compatibility:** a TDF with no key access objects now fails at load
with a malformed-TDF error instead of a later, less specific failure.
### Checklist
- [x] 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 `TDFKeyAccessTest`).
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* TDF files with missing or empty key access information are now
rejected with a clearer error before key access or integrity checks
begin.
* Encrypted files that cannot reconstruct a split key now fail instead
of continuing with an invalid key.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This aligns the Java SDK and cmdline with the Go and JS SDKs on how TDF manifests are parsed and how assertions are hashed and verified. All three SDKs run the same shared conformance vectors from opentdf/tests.
Manifest parsing.
StrictJsonReaderchecks the manifest in the same single Gson pass that reads it. Each of the following is rejected:Manifests are read as strict JSON. One historical spelling,
IsStreamable, is accepted. The checks apply whether or not assertion verification is enabled.Assertion hashing. Each assertion keeps the JSON it was read from. The hash covers that object minus its top-level
binding.Manifest.Assertion.computeAssertionHash(String)exposes the calculation. Unknown members and object statement values round-trip unchanged.Writer. The writer emits
"method": "jws"and hashes the object it writes. It refuses an invalid assertion id, or content that would not parse back, withIllegalArgumentException.Verification keys. Each assertion is verified with one key from configuration: its per-ID key, otherwise the default key, otherwise the payload key with HS256.
algmust match that key's algorithm.jwkorx5cis accepted only when it matches that key. Assertions from the Java writer with an embeddedjwkorx5cverify when the reader configures the same key.assertionHashandassertionSigare not validated.Errors. Assertion failures throw
SDK.AssertionFailureException, a subclass ofSDK.AssertionExceptionand therefore still aTamperException. It hasgetAssertionId()andgetReason()(AssertionFailureReason), and its message includesreason=<code>.Key access.
loadTDFrejects a manifest with no key access objects withMalformedTDFExceptionbefore any KAS request.cmdline. The assertion verification keys file supports
defaultKeyand is parsed strictly. Unknown members, unknown algorithms, and PEM text as an HS256 key are configuration errors. The help text and javadoc note that a default key also applies to payload-key assertions.Compatibility
reason=binding-mismatch.system-metadata, so those files fail verification when only a default key is configured.invalid-json.Testing
mvn -pl cmdline -am packagepasses: sdk 959 tests (9 skipped), sdk-pqc-bc 54, cmdline 42.AssertionVectorsTestruns the shared vectors vendored assdk/src/test/resources/assertion-vectors-phase0.json.Related pull requests