Repository navigation
GH-53776: [C++][Gandiva] Add AES-CBC and AES-GCM support via binary encrypt/decrypt functions - #53815
Open
dmitry-chirkov-dremio wants to merge 7 commits into
Open
dmitry-chirkov-dremio wants to merge 7 commits into
dmitry-chirkov-dremio wants to merge 7 commits into
Conversation
|
|
|
|
github-actions
Bot
added
Component: C++
Component: Gandiva
awaiting review
…mpty varlen entries
dmitry-chirkov-dremio
force-pushed
the
gandiva-encrypt-decrypt-cbc-gcm
branch
from
October 15, 2026 02:50
2c58f9e to
a4c81f1
Compare
|
|
3 similar comments
|
|
|
|
|
|
|
|
| namespace { | ||
|
|
||
| // ERR_raise() only exists since OpenSSL 3.0 | ||
| void RaiseOpenSSLError(int reason) { |
Contributor
Author
There was a problem hiding this comment.
I'd love a feedback if this fallback to an earlier OpenSSL version is needed. The oldest OpenSSL in CI is 3.0.2 (Ubuntu 22.04 ARM64).
github-actions
Bot
added
awaiting committer review
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
1 participant
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.
Rationale for this change
Gandiva's
aes_encrypt/aes_decryptare ECB-only and VARCHAR-based. This adds binaryencrypt/decryptfunctions with CBC and authenticated GCM modes. See #53776.What changes are included in this PR?
New
encrypt(data, key, mode[, iv[, aad]])and matchingdecryptoverloads. All arguments and results arebinaryexceptmode, which isutf8:AES-ECB,AES-CBC,AES-GCM, plus-PKCS7/-NONEvariants for ECB and CBC. Keys are 16, 24 or 32 bytes.aes_encrypt/aes_decryptandencrypt_utils.*are not modified.The commits are layered: ECB, CBC, GCM, functions, projector tests. Start with
encrypt_utils_gcm.cc(the IV length must be set before the IV, or OpenSSL uses only the first 12 bytes) and the output buffer sizing in the stubs ingdv_function_stubs.cc.Are these changes tested?
Yes: unit tests per module, GCM known-answer tests from the GCM specification (test cases 4-6, which fail without the IV fix), stub tests including block-aligned inputs, and projector round-trip and error tests. The full Gandiva internals and projector suites pass locally with clang on macOS.
Are there any user-facing changes?
Yes, two new Gandiva functions,
encryptanddecrypt, are available to expression builders. Nothing existing changes.Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
PR code and description written by:
The original implementation was written by a human engineer in an internal codebase. AI tooling was used to port it onto current main, review it and fix what the review found (GCM IV-length ordering, a context leak on invalid key lengths, length validation and buffer sizing), write the tests, and write this description.
Reviewed before submission by: