Skip to content

feat(proxy): accept inbound EQL ciphertext payloads - #458

Merged
freshtonic merged 11 commits into
mainfrom
feat/cip-3822-inbound-eql-payloads
Aug 26, 2026
Merged

feat(proxy): accept inbound EQL ciphertext payloads#458
freshtonic merged 11 commits into
mainfrom
feat/cip-3822-inbound-eql-payloads

Conversation

@freshtonic

@freshtonic freshtonic commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • accept EQL v3 storage payloads and query-only operands in SQL literals and bind parameters before plaintext decoding
  • validate payload version, inferred destination, active keyset metadata, configured scalar and SteVec SEM terms, authenticated descriptors, and SteVec array markers
  • authenticate every stored ciphertext, including every SteVec entry, then independently rederive and compare its searchable metadata; ordinary SELECT decryption remains root-only
  • accept bare selector hashes using the fixed selector-hash shape and document the unavoidable plaintext/hash ambiguity
  • forward valid payloads without re-encryption and fail closed with a generic, documented transaction-aborting error
  • cover application-generated payloads, query-only operands, selector hashes, metadata and term tampering, accepted SteVec storage/readback, non-root SteVec tampering, and transaction recovery in cipherstash-proxy-integration
  • demonstrate storage, query-only, and selector-hash inputs in the executable showcase

Compatibility

On every encrypted column type, Proxy reserves three JSON object shapes for application-generated EQL:

  • storage payloads with top-level v and i plus at least one of c, h, or sv;
  • scalar query payloads with v and i plus at least one of hm, bf, ob, or op; and
  • SteVec query payloads whose only top-level key is sv.

Matching values are validated as EQL rather than encrypted as plaintext, with no opt-out. Query-only shapes are rejected in storage positions. The error documentation includes a complete upgrade audit query and guidance for text-backed source data.

This PR also restores the configured-default-keyset behavior: when a connection has not selected a keyset, Proxy now scopes encryption and decryption to CS_DEFAULT_KEYSET_ID instead of silently using the ZeroKMS credential's account default. Operators whose two defaults differ must decrypt affected existing values with the old account-default keyset and re-encrypt them under the configured default.

Testing

  • mise run test:check
  • cargo nextest run -p cipherstash-proxy --no-fail-fast (151 passed)
  • mise run test:clippy
  • mise run test:format
  • cargo test -p cipherstash-proxy-integration --no-run

Runtime integration and showcase cases use the existing credentialed CI tasks; this checkout lacks Docker access and the application/ZeroKMS credentials needed to start an isolated current-branch Proxy locally.

Linear: CIP-3822

@freshtonic
freshtonic requested a review from tobyhede August 19, 2026 06:59

@tobyhede tobyhede left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the new inbound EQL ciphertext path. One security issue with two exploit paths, one design trade-off to confirm, and one operational gap. Each item was checked against the actual code on this branch and the cipherstash-client 0.42.2 dependency source before being posted here. Two items from an earlier automated pass did not hold up under verification and are not included: the "h" top-level key check is not dead code (it detects SteVec payloads, which carry no top-level c field), and the binary-parameter byte handling in bind.rs does not corrupt data (original bytes stay untouched, and a failed trial parse falls back safely).

See inline comments for details.

Comment thread packages/cipherstash-proxy/src/postgresql/inbound_eql.rs
Comment thread packages/cipherstash-proxy/src/postgresql/inbound_eql.rs Outdated
Comment thread packages/cipherstash-proxy/src/postgresql/inbound_eql.rs Outdated
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs Outdated
Comment thread packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs Outdated
@freshtonic
freshtonic force-pushed the feat/cip-3822-inbound-eql-payloads branch 2 times, most recently from d99891b to dc7a97d Compare August 24, 2026 02:23
@freshtonic
freshtonic requested a review from tobyhede August 24, 2026 02:40

@tobyhede tobyhede left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

This review covers eight candidate findings. Two were not correct, and this review removes them. Six remain.

The security work in this PR is good. The proxy now decrypts every SteVec entry. This finds forged ciphertext, changed entry order, duplicate entries, and swapped entry ciphertext. The comments below do not change that result.


1. High — the new decrypt path also changes every SELECT

File: packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs:413

The decrypt function now makes one ZeroKMS record for each SteVec entry. Before this PR it made one record for the root entry only.

EncryptionService::decrypt has two callers:

  • postgresql/frontend.rs:1273merge_inbound_eql, the new inbound path.
  • postgresql/backend.rs:497 — the DataRow flush, the usual SELECT path.

The change therefore applies to every read of an encrypted JSONB column, not only to inbound payloads.

cipherstash-client makes two entries for each JSON node. A document with 50 nodes gives approximately 100 records. MessageBuffer holds 4096 rows. One SELECT of 4096 rows can thus send approximately 409,600 records in place of 4,096.

The client divides the keys into groups of 500, and sends 5 requests at the same time. There is no hard limit to reach. But the number of requests increases from approximately 9 to approximately 820. Each entry also gets its own key-retrieval payload, and the client does not remove the duplicates.

This is not only a cost. The AAD contains the selector. Decryption of the entries on read therefore finds a graft of value entries in the database. That is a true safety gain.

Confirm the intent:

  • If the read-path change is intentional, add a CHANGELOG entry. Give the read cost for JSON columns.
  • If it is not intentional, expand the entries only in merge_inbound_eql. The trait has one real implementation and two test mocks. The change is small.

No test covers the read path for this behaviour.


2. Medium — the keyset change has no CHANGELOG entry

File: packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs:172

init_cipher now uses the configured default_keyset_id when the connection sets no keyset. Before this PR it passed None. None selects the default keyset of the client credential.

The history shows a repair, not a new decision. Commit 8df6bee7 passed the configured id. Commit 6cbb7119 removed it on 2025-08-13. The releases v2.1.20 to v3.0.1 contain that regression. This PR puts the original behaviour back.

The effect on users is small, because stash setup --proxy makes the two ids the same. But if the two ids are different, these things occur after the upgrade:

  • Rows written before the upgrade do not decrypt.
  • Search terms for old rows do not agree with new terms.
  • A wrong default_keyset_id now fails at cipher init with UnknownKeysetIdentifier. Before, the proxy ignored it.

Only the commit message records this change. Add a CHANGELOG entry and a short migration note.


3. Medium — two rejected JSON shapes have no documentation and no audit query

File: packages/cipherstash-proxy/src/postgresql/inbound_eql.rs:57

The proxy rejects three JSON shapes in a storage position:

  • v and i and one of c, h, sv — documented.
  • v and i and one of hm, bf, ob, op — not documented.
  • sv as the only key — not documented.

The audit query in docs/errors.md needs v and i:

WHERE value ? 'v' AND value ? 'i' AND value ?| ARRAY['c', 'h', 'sv']

The document {"sv": ["a", "b"]} has no v and no i. The query does not find it, but the proxy rejects it. An operator can run the audit, get zero rows, upgrade, and then get a write failure.

{"sv": ...} alone is a possible document. sv is the ISO-639 code for Swedish.

parse also runs before any check of the column type. See frontend.rs:659 and bind.rs:157. This applies to all encrypted columns, not only to jsonb columns. An encrypted text column that receives '{"sv":"Hej"}' also fails.

Change the audit query to find all three shapes. Say in the compatibility note that the reservation includes the query-operand shapes, and that it applies to every encrypted column type.


4. Low — the SteVec check does not compare the a marker

File: packages/cipherstash-proxy/src/postgresql/inbound_eql.rs:195

For two SteVec payloads, sem_terms_match does not use the fast path. It compares into_query_operand() on the two sides.

In cipherstash-client 0.42.2, into_query_operand() maps SteVecEntryV3 to SteVecQueryEntryV3 { selector, term }. It removes is_array. The doc comment on that function states this.

The AAD is the descriptor, the key tag, and the 16-byte selector. The encrypted plaintext is the node value. is_array is in neither. The a field is therefore not authenticated and not compared. The proxy stores it as it arrives.

An attacker who holds a legitimate payload can change a and send the payload again. In EQL 3.0.4, a controls eql_v3_internal.is_ste_vec_array. That function controls eql_v3.jsonb_array_length and eql_v3.jsonb_array_elements. EQL Mapper sends the standard SQL functions to these. A changed a makes one of these results:

  • An error becomes a wrong answer with one element.
  • A correct answer becomes an error.

There is no effect on the keys or on the plaintext. The severity is low. But the fix is easy: compare is_array, or compare the sv array directly.

The doc comment at inbound_eql.rs:190 is also not correct. It says the comparison keeps "every scalar or SteVec SEM term". It does not keep a.


5. Low — the inbound keyset_id has no check

File: packages/cipherstash-proxy/src/postgresql/inbound_eql.rs:94

validate_storage_metadata checks the version, the identifier, the descriptor, and the term shape. It does not check EncryptedRecord.keyset_id. It does not check KeyHeader.keyset_id for SteVec payloads. The AAD contains neither field.

This is not a failure of keyset scope. ScopedCipher::decrypt always gives an explicit keyset, and that keyset replaces the value in the record. See vitur_client/mod.rs:785. The proxy does not use decrypt_fallible. A payload that is truly encrypted under a different keyset fails the AEAD check.

But a false value stays in the record. The client groups the payloads by keyset_id before it replaces the value. Rows with many different false values therefore divide one key-retrieval request into many requests. The false value also goes into the stored data and into the logs. An external tool that decrypts the same column without an explicit keyset fails.

Add an equality check against the cipher keyset, or set the field to the cipher keyset. This is defence in depth.


6. Low — literals_to_plaintext has no callers

File: packages/cipherstash-proxy/src/postgresql/frontend.rs:1524

Line 668 now calls literals_to_plaintext_skipping. Nothing calls literals_to_plaintext. The function is a simple call to the _skipping version with an empty slice. The behaviour of the two functions is the same.

CI does not fail. packages/cipherstash-proxy/src/lib.rs:1 has #![allow(dead_code)]. This hides the function from clippy -D warnings.

Remove the function. Also examine the crate-wide allow(dead_code). It hides all dead code from CI.


Items checked and cleared

These items looked wrong, but they are correct. No action is necessary.

The containment cast to eql_v3.query_json does not break a GIN index.

rewrite_containment_ops.rs:121 changes the cast, and the test comment about GIN index use is gone. The change is safe:

  • cipherstash-encrypt.sql has no GIN operator class. The only operator class is a B-tree class for ORE.
  • The rewrite makes a function call to eql_v3.jsonb_contains. That function takes plain jsonb on both sides. Both domains are domains over jsonb.
  • The documented GIN expressions apply to the column, not to the needle. This PR does not change the column side.
  • The old test asserted a cast to a text domain on a JSON column. That cast cannot work against a real database.
  • EQL 3.0.4 names $1::jsonb::eql_v3.query_json as the correct needle shape.

Query-only operands must carry all configured terms. This is correct.

validate_query_metadata uses validate_scalar_term_presence without a change. This agrees with the eql_v3.query_* domain CHECK, and PostgreSQL rejects a short operand in any case. cipherstash-client documents into_query_operand() as the way to make a multi-term operand. The tests use a column with unique, ope, and match indexes, and the tests pass.


Test coverage gap

No integration test stores an inbound SteVec document with success. The tests cover text storage, query-only scalar operands, query-only SteVec needles, bare selectors, and SteVec rejection. They do not cover an accepted inbound SteVec store.

The SteVec comparison uses positional equality of a JSON array. The result therefore depends on agreement between the entry order from the application and the entry order from the re-encryption of the proxy. No test proves that these two orders agree.

Add a test that stores an inbound SteVec document and reads it back.


Not checked

EncryptedRecord.decryption_policy comes from the same untrusted source, and it goes to retrieve_key_payload. This review did not establish whether the server-generated ZeroKMS MAC makes that field safe.

@cipherstash-bot cipherstash-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage review

Strong coverage overall. inbound_eql.rs ships 20 unit tests and inbound_ciphertext.rs adds 11 end-to-end tests spanning store/query/selector acceptance plus destination-mismatch, tampered-non-root, and spliced-scalar-term rejection. The eql-mapper containment changes (@>/<@ now query operands) carry updated assertions in lib.rs, and the empty-batch encrypt short-circuit and error-response mapping both got unit tests.

Gaps below are all negative/branch coverage that is lopsided against the positive paths already tested — the classic "one tamper axis tested, the sibling axis not" pattern. None block; each is a cheap paste-in.

Out-of-scope note (not an inline comment)

The init_cipher change in zerokms.rs (fall back to Proxy's configured default_keyset_id instead of None) is a meaningful security-relevant behaviour change — passing None would derive different searchable-encryption terms from the account default. It resists pure unit testing (needs a live ScopedCipher) so I'm not filing an inline gap, but it's worth confirming an integration path exercises the no-connection-keyset case.

Additional coverage gaps not posted inline

  • validate_scalar_term_presence (inbound_eql.rs:246): the IndexType::SteVec => Err arm — a scalar Encrypted payload validated against a column that carries a SteVec index — has no direct test.
  • merge_inbound_eql (frontend.rs:1246): the InboundEql::Store with column == None early error return is unexercised.

⚠️ 1 review job(s) failed and were excluded from this review: codex (gpt-5.5)/test-gap.

Comment thread packages/cipherstash-proxy/src/postgresql/inbound_eql.rs
Comment thread packages/cipherstash-proxy/src/postgresql/inbound_eql.rs
Comment thread packages/cipherstash-proxy/src/postgresql/inbound_eql.rs
Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
Scope the ZeroKMS cipher to CS_DEFAULT_KEYSET_ID whenever a connection has not selected an override. Previously Proxy only checked that the setting existed, then passed no identifier to ScopedCipher and could silently use the client's account default instead.

Application-encrypted payloads use the configured keyset explicitly. When the account and configured defaults differ, Proxy derived searchable-encryption metadata with another index key and rejected valid inbound ciphertext during authentication. Preserve connection-level keyset precedence while making the configured fallback effective.

Signed-off-by: James Sadler <james@cipherstash.com>
Inbound EQL authentication independently re-encrypts plaintext and compares its searchable-encryption metadata with the supplied payload. Match-index generation does not guarantee a stable ordering for Bloom-filter bit positions, so comparing serialized query operands rejected valid ciphertext whenever equivalent positions were emitted in another order.

Compare scalar metadata field by field and normalize Bloom-filter positions before equality. Continue comparing identifiers, exact-match terms, ordered terms, versions, and structured SteVec operands exactly so altered metadata still fails closed. Add a regression test covering reordered equivalent Bloom-filter terms.

Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
Signed-off-by: James Sadler <james@cipherstash.com>
@freshtonic
freshtonic force-pushed the feat/cip-3822-inbound-eql-payloads branch from dc7a97d to 96f22cd Compare August 26, 2026 03:06
Signed-off-by: James Sadler <james@cipherstash.com>
@freshtonic

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review feedback in 7b58470e:

  • scoped all-entry SteVec authentication to inbound storage validation; ordinary DataRow/SELECT decryption remains root-only, with direct regression coverage for both modes;
  • documented the restored CS_DEFAULT_KEYSET_ID behavior and migration implications;
  • expanded the compatibility note and audit SQL to cover storage, scalar-query, and bare-SteVec query shapes on every encrypted column type;
  • compare SteVec a/array metadata during independent derivation and reject tampering end to end;
  • require embedded scalar and SteVec keyset metadata to match the active cipher keyset;
  • removed the new dead literals_to_plaintext wrapper. I also checked the crate-wide allow(dead_code): removing it exposes 11 pre-existing warnings across unrelated CLI, connection, diagnostics, context, and data helpers, so this focused change leaves that existing policy intact;
  • added successful inbound SteVec storage/readback integration coverage plus the requested SteVec term-splice, Bloom presence-mismatch, negative query-shape, scalar-vs-SteVec, and missing-destination tests;
  • renamed the shared error to InvalidInboundEqlPayload, since query-only payloads contain no ciphertext;
  • confirmed decryption_policy is authenticated by ZeroKMS's tag-version-1 policy MAC during key retrieval and documented that invariant in the decrypt path.

The PR description now carries the full compatibility and migration notes. Static checks, formatting, Clippy, and all 151 proxy tests pass locally; the credentialed PostgreSQL matrix is running in CI.

@freshtonic
freshtonic requested a review from tobyhede August 26, 2026 04:44
@tobyhede

tobyhede commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review

Head 7b58470e. Three low-priority points. No defect found.

1. Low — SteVec order safety depends on a dependency feature flag

packages/cipherstash-proxy/src/postgresql/inbound_eql.rs:205

sem_terms_match compares SteVec entries by position. This is correct today, on one condition: the preserve_order feature of serde_json stays off. With that feature off, serde_json::Map is a BTreeMap, so key order is sorted. The client walk is a sequential depth-first traversal (json_indexer/path_values.rs:90). Both sides encode the plaintext through Value (plaintext/mod.rs:332, :428), which removes the key order of the original text. Equal documents therefore give equal entry order.

Two events break the condition, and neither changes this file:

  • A new dependency turns on serde_json/preserve_order. Cargo unifies features across the graph, so Proxy gets IndexMap order.
  • A client in another language makes the payload and its core turns on that feature. napi supplies such a feature. That client sends insertion order, Proxy derives sorted order, and Proxy rejects every document from it.

The second event matters most, because this feature exists to accept payloads that other SDKs make.

Options: record the condition in a comment, or sort entries by selector before the comparison. The length check already limits the cost.

2. Low — the exact-term rule for query operands is not documented

docs/errors.md:365

The "How to fix" text gives the exact-term rule for storage payloads only. Query operands obey the same rule: the terms must equal the configured indexes of the column. The error is deliberately generic, so a user who debugs a rejected operand has no other source. Add one sentence.

3. Low — the sem_terms_match doc comment omits k

packages/cipherstash-proxy/src/postgresql/inbound_eql.rs:190

The comment lists what the comparison covers, but does not mention k. State that the type constrains k to one value, and that the comparison therefore excludes it.

@tobyhede tobyhede left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work

@freshtonic
freshtonic merged commit d4475db into main Aug 26, 2026
6 checks passed
@freshtonic
freshtonic deleted the feat/cip-3822-inbound-eql-payloads branch August 26, 2026 05:34
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.

3 participants