Skip to content

Modifying ExternalDBPAEncryptor/DecryptorAdapter to obtain agent instance from shared lib (DLL) - #119

Merged
argmarco-tkd merged 2 commits into
dev_phase2from
dev_merge_p2_dll
Sep 4, 2025
Merged

Modifying ExternalDBPAEncryptor/DecryptorAdapter to obtain agent instance from shared lib (DLL)#119
argmarco-tkd merged 2 commits into
dev_phase2from
dev_merge_p2_dll

Conversation

@argmarco-tkd

@argmarco-tkd argmarco-tkd commented Sep 3, 2025

Copy link
Copy Markdown

Modifying ExternalDBPAEncryptor/DecryptorAdapter to obtain agent instance from shared lib (DLL)

This is the last piece which connects together the base work in Arrow (dev_phase2), and DLL-based agent loading.

Both ExternalDBPAEncryptorAdapter and ExternalDBPADecryptorAdapter have been modified to abandon the simple, pass-through encryption/decryption, and to make use of the DLL-loaded agent (which must be specific in the connection_config object passed in. The modifications include (a) looking for the DLL-file name in the properties, (b) instantiating a new agent from the DLL, and (c) forwarding the encrypt/decrypt operations to the DLL-based agent.

Testing

  • Tests inside external_dbpa_encryption_tests.cc had to be modified to account for the need of DLL. For these tests, we're making use of libDBPATestAgent.so, which is built out of external/dbpa_test_agent.* (and used in a few different test scenarios).
  • Newly modified tests pass.
  • All existing Parquet (ctest -L parquet) tests pass.
  • All other tests were unaffected.
  • Manually verified that the end-to-end workflow in base_app.py completes succesfully.

Notes

  • A similar change was performed in the dev-miniApp codebase: Wiring DBPAgent into ExternalEncryptorImpl/ExternalDecryptor impl #68, however bear in mind that the structure of encryptors/decryptors is/was quite different in the miniApp
  • There will be another PR with some modifications to base_app.py (it needs to account for DLL-based agent loading)

@github-actions

github-actions Bot commented Sep 3, 2025

Copy link
Copy Markdown

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:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I made this name change so source files are easier to navigate. Sharing the same name among encryptor and decryptor was confusing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this is just formatting, so that the encryptor and decryptor construction are similar in the code (no functional changes)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We missed this in an earlier PR. Because the code was not being used at the time, there were no compilation errors.

agent_instance_ = LoadAndInitializeAgent(
column_name, connection_config, app_context, key_id, data_type, compression_type);

//TODO: what to do if agent was not initialized?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Added an Issue to track agent init errors since these are in a couple places. protegrity/DataBatchProtectionService#100

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thank you!

}

int32_t ExternalDBPAEncryptorAdapter::CallExternalDBPA(
int32_t ExternalDBPAEncryptorAdapter::InvokeExternalEncrypt(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is where the "executor" would be hooked? (for my understanding)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The calls to InvokeExternalEncrypt() and InvokeExternalDecrypt() will definitely be controlled by the "executor". But the overall behavioral reach of the "executor" is TBD. It will really depend on the decision we make on the behavior for init() and how those errors propagate up (e.g. we will likely want to protect against a remote DBPA's init() that takes 10 mins to verify server connections)

@argmarco-tkd
argmarco-tkd merged commit 3e332fc into dev_phase2 Sep 4, 2025
19 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Synch Point] Getting Arrow ready for ExternalEncryptorImpl (merging two work streams)

2 participants