Machinery for loading external Agent (encryptor) out of a shared library #37

argmarco-tkd · 2025-08-08T04:09:00Z

Rationale for this change

In this PR, I'm creating the machinery (utils classes) for

Loading an implementation of the DataBatchProtectionAgent from a shared library (e.g. *.so file)
Wrapping that implementation in a Decorator which will ensure the shared library handle will be properly disposed of.

These changes are described in this doc

(An earlier PR was prepared on the same change. The earlier PR was closed because it had undesired changes/files in it - however I have tried to address the comments from that one in this one).

What changes are included in this PR?

Mostly new files + their respective tests.
A relatively important, but hopefully not controversial change is the addition of the "CloseDynamicLibrary" function to Arrow's io_util.cc - the same file which already contained functionality for dealing with dynamic libraries.

Are these changes tested?

Yes - unit tests have been created.

Additional Notes

The interface for DataBatchProtectionAgentInterface is still being worked on. I have created a temporary interface (parquet/encryption/external/DBPAInterface.h), which hopefully is close-enough to the final version. Once the interface is finalized, we will incorporate it here, and send the final PR.
This PR is of the changes between two branches: dev_dll_work (feature branch) and dev-miniapp

…cryptor*

…yptor related stuff. Begin moving things around.

…. It has its own .so file now.

…s is done to facilitate testing

…ry_wrapper now pass.

…ader stuff - needed for refactoring

…of DBPALibraryWrapper

…f LoadableEncryptorInterface

…brary stuff with arrow-provided libraries.

github-actions · 2025-08-08T04:09:24Z

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:

avalerio-tkd

LGTM overall, just a few comments. I mostly skipped the unittests.

avalerio-tkd · 2025-08-08T15:55:14Z

cpp/src/arrow/util/io_util.cc

 #endif
 }

+Status CloseDynamicLibrary(void* handle) {


The method looks a bit cryptic. Could you add a comment at the method level to describe what this does?

avalerio-tkd · 2025-08-08T15:57:12Z

cpp/src/parquet/CMakeLists.txt

  set(PARQUET_SRCS ${PARQUET_SRCS} encryption/encryption_internal.cc
-                   encryption/openssl_internal.cc)
+                   encryption/openssl_internal.cc
+                   encryption/external/loadable_encryptor_utils.cc


For my understanding, do these modules need to be specified somehow differently (with additional flags maybe) during compilation? Or these are treated the same as any other source?

avalerio-tkd · 2025-08-08T15:59:04Z

cpp/src/parquet/encryption/CMakeLists.txt

+    arrow_shared
+  )
+
+  set_target_properties(DBPATestAgent PROPERTIES OUTPUT_NAME "DBPATestAgent")


You probably answered this already, but the TestAgent is just for the testing flow, right? (as in ARROW_TESTING=True above)

correct, this is just for the testing flow. The generated library (*.so, *.dll) is not "installed" (i.e. copied) anywhere other than where needed for unit testing

avalerio-tkd · 2025-08-08T16:00:45Z

cpp/src/parquet/encryption/external/dbpa_library_wrapper.cc

+}
+
+DBPALibraryWrapper::DBPALibraryWrapper(
+    std::unique_ptr<DataBatchProtectionAgentInterface> agent,


These few lines are quite loaded. Could you add an 2-line comment on what this does?

It is described in the .h, but I see your point. Will add a comment here as well.

avalerio-tkd

Since these are mostly comment and nits will approve for you to merge when ready. Thanks.

github-actions · 2025-08-08T21:44:43Z

❌ GitHub issue #41 could not be retrieved.

github-actions · 2025-08-08T21:45:48Z

❌ GitHub issue #41 could not be retrieved.

avalerio-tkd · 2025-08-09T16:55:46Z

@argmarco-tkd if there's anything pending from my side on this PR, let me know. Thanks!

argmarco-tkd · 2025-08-09T20:54:44Z

@argmarco-tkd if there's anything pending from my side on this PR, let me know. Thanks!

Nope. I was initially planning to have the finalized DataBatchProtectionAgent.h before merging this in - but on second thought, I'll merge it as is, THEN apply the changes to DataBatchProtectionAgent.h

argmarco-tkd and others added 30 commits July 8, 2025 10:47

Added DLLEncryptor. Currently unused.

216bd5e

creating empty DLL Encryptor. Some wiring.

5059384

DLLEncryptor succesfully loaded (via the Loader) from the mini-app.

a35c9ac

Refactored and separated the DLLEncryptorLoader from DLLEncryptor impl

a61d2a9

Refactoring. Began the renaming of DLLEncryptorLoader into LoadableEn…

9717f7e

…cryptor*

Extracted LoadableEncryptorInterface into its own file.

e99e3e9

adding a few utils to Dockerfile.miniApp

323e608

adding parallelism to ./ninja to the helper build scripts

30a2b35

created the parquet/encryption/[external] folder to put external encr…

958f8a9

…yptor related stuff. Begin moving things around.

First pass at implementing DBPALibraryWrapper - tests not added yet.

6243e77

cleaning up the impl of LoadFromLibrary()

d391d71

Defined DBPATestAgent as a simple agent for testing loading machinery…

edff151

…. It has its own .so file now.

moving DBPALibraryWrapper into its own .h and .cc files. No tests yet.

6c8f265

Adding the option to pass a function to close the shared library. Thi…

a62b405

…s is done to facilitate testing

Reviewed and improved the testing suite. All the tests for dbpa_libra…

0248ef6

…ry_wrapper now pass.

Reviewed and improved the testing suite. All the tests for dbpa_libra…

1db72b6

…ry_wrapper now pass.

ensuring that WIN32 stuff throws an exception (unimplemented)

3bf7406

commented out the references to loadble_encryptor_utils and to DLL lo…

3b09ef3

…ader stuff - needed for refactoring

Added a default value (function) for the handle_closing_fn parameter …

4d52fc7

…of DBPALibraryWrapper

Modified LoadableEncryptorUtils to work with DBPAInterface, instead o…

e91a805

…f LoadableEncryptorInterface

Adding tests for loadable_encryptor_utils

7d5b847

Added CloseDynamicLibrary() to arrow/util/io_util.cc

300ddb3

Replaced the "custom" usage of dlopen, dlsym, etc to handle Shared Li…

bdafe6c

…brary stuff with arrow-provided libraries.

better casting

c33df5c

Added better heuristics to find the *.so file during tests

be838ef

Removed references to DLLEncryptor (sources, CMake configs)

08b817f

working

99a0d64

closing decryption loop in miniapp. super hacky code, pls ignore

7beec95

Removing unnecessary prints

4747aef

Merge branch 'dev-miniapp' into dll_work

e975ec6

Some cleanup after feedback in draft PR: #36

d2c48f8

argmarco-tkd requested review from avalerio-tkd, cristekdatum and sofia-tekdatum August 8, 2025 04:09

github-actions bot added Component: C++ Component: Parquet awaiting review labels Aug 8, 2025

argmarco-tkd changed the title ~~Dev dll work~~ [Draft] Machinery for loading external Agent (encryptor) out of a shared library Aug 8, 2025

argmarco-tkd mentioned this pull request Aug 8, 2025

[Abandoned-Replaced] Machinery for loading of dynamic libraries. #36

Closed

avalerio-tkd reviewed Aug 8, 2025

View reviewed changes

avalerio-tkd approved these changes Aug 8, 2025

View reviewed changes

github-actions bot added awaiting committer review and removed awaiting review labels Aug 8, 2025

argmarco-tkd changed the title ~~[Draft] Machinery for loading external Agent (encryptor) out of a shared library~~ GH-41 -- Machinery for loading external Agent (encryptor) out of a shared library Aug 8, 2025

argmarco-tkd changed the title ~~GH-41 -- Machinery for loading external Agent (encryptor) out of a shared library~~ GH-41: Machinery for loading external Agent (encryptor) out of a shared library Aug 8, 2025

argmarco-tkd changed the title ~~GH-41: Machinery for loading external Agent (encryptor) out of a shared library~~ Machinery for loading external Agent (encryptor) out of a shared library Aug 8, 2025

argmarco-tkd mentioned this pull request Aug 8, 2025

DLL Loading: implement LoadableEncryptorUtils() (includes Agent wrapper for safe/proper dll unloading) protegrity/DataBatchProtectionService#41

Closed

argmarco-tkd marked this pull request as ready for review August 9, 2025 20:56

argmarco-tkd merged commit d2c48f8 into dev-miniapp Aug 9, 2025
34 of 77 checks passed

argmarco-tkd mentioned this pull request Aug 9, 2025

Toy implementation of DataBatchProtectionAgent (for testing) protegrity/DataBatchProtectionService#43

Closed

This was referenced Sep 2, 2025

Adding CloseDynamicLibrary() to arrow/util/io_util.h #109

Merged

Adding external dependencies to parquet/encryption/external/third_party #111

Closed

Machinery for loading external Agent (encryptor) out of a shared library #37

Machinery for loading external Agent (encryptor) out of a shared library #37

Uh oh!

Conversation

argmarco-tkd commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Additional Notes

Uh oh!

github-actions bot commented Aug 8, 2025

Uh oh!

avalerio-tkd left a comment

Choose a reason for hiding this comment

Uh oh!

avalerio-tkd Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

avalerio-tkd Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

avalerio-tkd Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

argmarco-tkd Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

avalerio-tkd Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

argmarco-tkd Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

avalerio-tkd left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 8, 2025

Uh oh!

github-actions bot commented Aug 8, 2025

Uh oh!

avalerio-tkd commented Aug 9, 2025

Uh oh!

argmarco-tkd commented Aug 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

argmarco-tkd commented Aug 8, 2025 •

edited

Loading