Skip to content

Conversation

@timhurskidremio
Copy link

@timhurskidremio timhurskidremio commented Nov 12, 2025

Before adding the new modes, we need to reorganize the existing code to make room for the new stuff

This PR:

  1. Moves the existing AES-EBC functions into their own modules (cpp/src/gandiva/encrypt_utils.*cpp/src/gandiva/encrypt_utils_ecb.*)
  2. Enhances error messages to better communicate what went wrong (it captures OpenSSL exception details where possible as well)
  3. Adds missing resource release to fix memory leaks
  4. Adds a new signature for the ECB mode that operates on VARBINARY instead of VARCHAR as VARCHAR is vulnerable to non-UTF8-compliant values. For example, this query errors out with unexpected byte \24 encountered while decoding utf8 string:
SELECT
  'Test 1: Null byte at position 8' AS test_name,
  AES_ENCRYPT(
    '2sp_`f pO:Jzy{Rd',
    'Nn4n6[s0MqsLG%<7'
  ) AS encrypted_result,
  LENGTH(AES_ENCRYPT(
    '2sp_`f pO:Jzy{Rd',
    'Nn4n6[s0MqsLG%<7'
  )) AS result_length;

AES_ENCRYPT and AES_DECRYPT signatures after this change:
(utf8, utf8) → utf8 - the original signature for backward compatibility
(varbinary, varbinary, 'ECB') → varbinary - the new signature that will be consistent with signatures for CBC and GCM, and isn't vulnerable to non-UTF8-compliant data

Both signatures ultimately are handled by the same implementation

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@timhurskidremio
Copy link
Author

@timhurskidremio timhurskidremio force-pushed the move-ecb-encryption-mode branch 3 times, most recently from c4529af to 57fb0ab Compare November 13, 2025 16:24
@timhurskidremio timhurskidremio changed the title Move ECB encryption mode into its own module DX-108149: Move ECB encryption mode into its own module Nov 13, 2025
@lriggs
Copy link

lriggs commented Nov 17, 2025

Is there a plan to make these changes upstream in apache/arrow?

@timhurskidremio
Copy link
Author

Yes, once all of the encryption has been implemented

@timhurskidremio timhurskidremio merged commit 8817691 into dremio:dremio_26.1_18.1.0 Nov 20, 2025
7 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants