Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MONGOCRYPT-705 Add crypto parameters to payloads #872

Merged
merged 14 commits into from
Jul 31, 2024

Conversation

kevinAlbs
Copy link
Contributor

Summary

Add crypto parameters to payloads mc_FLE2InsertUpdatePayloadV2_t and mc_FLE2FindRangePayloadV2_t.

Updates to expected payloads specification tests are in (mongodb/specifications#1616) and were verified with the C driver in this patch.

Background & Motivation

SERVER-91889 supports receiving optional crypto parameters on the FLE2InsertUpdatePayloadV2 and FLE2FindRangePayloadV2. The crypto parameters describe how the payload was generated. This may enable safeguards proposed in SERVER-91887 to reject client payloads generated with incorrect parameters.

Other changes

_assert_match_bson was changed from a function to a macro. Before, the failure included the line number in the function _assert_match_bson:

test error [...]test-mongocrypt-assert-match-bson.c:904 _assert_match_bson(): ASSERT_MATCH failed

After, a failure includes the line number invoking the macro _assert_match_bson:

test error [...]test-mc-fle2-find-range-payload-v2.c:105 _test_FLE2FindRangePayloadV2_includes_crypto_params(): ASSERT_MATCH failed

_mongocrypt_random_uint64 was modified to read random data as little-endian. This is intended to ease testing on both little and big-endian hosts. Some tests use predetermined random data for deterministic results. If undesirable, random data for tests can be separately handled for big and little endian.

kevinAlbs added 10 commits July 27, 2024 10:42
Add new fields. Note fields that only applicable to range payloads
To include the line number of the macro invocation on error.
Fix documentation for `FLE2FindRangePayloadV2`. `g` is nested inside a `payload` document. See server IDL for reference.
To fix new tests on zSeries. May simplify testing with deterministic random-number generation on big-endian machines.
Payloads now include crypto param fields.
Notably: java bindings test with `trimFactor` of 0. Python tests with `trimFactor` of 1.
@kevinAlbs kevinAlbs requested a review from adriandole July 29, 2024 13:03
@kevinAlbs kevinAlbs marked this pull request as ready for review July 29, 2024 13:27
@kevinAlbs kevinAlbs requested review from jyemin, rozza and a team as code owners July 29, 2024 13:27
@kevinAlbs kevinAlbs removed request for a team, rozza and jyemin July 29, 2024 13:27
@@ -61,6 +71,11 @@ typedef struct {
// secondOperator represents the second query operator for which this payload
// was generated. Only populated for two-sided ranges. It is 0 if unset.
mc_FLE2RangeOperator_t secondOperator;
mc_optional_int64_t sparsity; // sp
mc_optional_uint32_t precision; // pn
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that range v2 isn't backwards-compatible, is there a reason why trimFactor and precision can't be signed int32 to match the server types? Could avoid a lot of bounds checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea of representing sparsity and precision to match the server types. Updated.

To match server representation
…ative

The non-negative check may be redundant with prior checks. The check is intended to be closer to the cast to `size_t`
To match server representation
@kevinAlbs kevinAlbs merged commit bb12cda into mongodb:master Jul 31, 2024
46 of 49 checks passed
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.

2 participants