-
Notifications
You must be signed in to change notification settings - Fork 0
Machinery for loading external Agent (encryptor) out of a shared library #37
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
Changes from all commits
216bd5e
5059384
a35c9ac
a61d2a9
9717f7e
e99e3e9
323e608
30a2b35
958f8a9
6243e77
d391d71
edff151
6c8f265
a62b405
0248ef6
1db72b6
3bf7406
3b09ef3
4d52fc7
e91a805
7d5b847
300ddb3
bdafe6c
c33df5c
be838ef
08b817f
99a0d64
7beec95
4747aef
e975ec6
d2c48f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -238,8 +238,11 @@ endif() | |
|
|
||
| if(PARQUET_REQUIRE_ENCRYPTION) | ||
| list(APPEND PARQUET_SHARED_PRIVATE_LINK_LIBS ${ARROW_OPENSSL_LIBS}) | ||
| list(APPEND PARQUET_STATIC_LINK_LIBS ${ARROW_OPENSSL_LIBS}) | ||
| set(PARQUET_SRCS ${PARQUET_SRCS} encryption/encryption_internal.cc | ||
| encryption/openssl_internal.cc) | ||
| encryption/openssl_internal.cc | ||
| encryption/external/loadable_encryptor_utils.cc | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| encryption/external/dbpa_library_wrapper.cc) | ||
| # Encryption key management | ||
| set(PARQUET_SRCS | ||
| ${PARQUET_SRCS} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,28 @@ | |
|
|
||
| # Headers: public api | ||
| arrow_install_all_headers("parquet/encryption") | ||
|
|
||
| if(ARROW_TESTING) | ||
| add_library(DBPATestAgent SHARED | ||
| external/dbpa_test_agent.cc | ||
| ) | ||
|
|
||
| # DBPATestAgent configuration | ||
| target_link_libraries(DBPATestAgent PUBLIC | ||
| arrow_shared | ||
| ) | ||
|
|
||
| set_target_properties(DBPATestAgent PROPERTIES OUTPUT_NAME "DBPATestAgent") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably answered this already, but the TestAgent is just for the testing flow, right? (as in ARROW_TESTING=True above)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| # Add test for DBPALibraryWrapper | ||
| add_parquet_test(dbpa-library-wrapper-test | ||
| SOURCES external/dbpa_library_wrapper_test.cc | ||
| LABELS "parquet-tests" "encryption-tests") | ||
|
|
||
| # Add test for LoadableEncryptorUtils | ||
| add_parquet_test(loadable-encryptor-utils-test | ||
| SOURCES external/loadable_encryptor_utils_test.cc | ||
| EXTRA_LINK_LIBS DBPATestAgent | ||
| LABELS "parquet-tests" "encryption-tests") | ||
| endif() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| //TODO: figure out the licensing. | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <memory> | ||
| #include "parquet/platform.h" | ||
| #include "arrow/util/span.h" | ||
|
|
||
| using ::arrow::util::span; | ||
|
|
||
| namespace parquet::encryption::external { | ||
|
|
||
| //TODO: this will change once we have a solid defition of interfaces | ||
|
|
||
| class EncryptionResult { | ||
| }; | ||
|
|
||
| class DecryptionResult { | ||
| }; | ||
|
|
||
| class PARQUET_EXPORT DataBatchProtectionAgentInterface { | ||
| public: | ||
| virtual std::unique_ptr<EncryptionResult> Encrypt( | ||
| span<const uint8_t> plaintext, | ||
| span<uint8_t> ciphertext) = 0; | ||
|
|
||
| virtual std::unique_ptr<DecryptionResult> Decrypt( | ||
| span<const uint8_t> ciphertext) = 0; | ||
|
|
||
| virtual ~DataBatchProtectionAgentInterface() = default; | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| //TODO: figure out the licensing. | ||
|
|
||
| #include "parquet/encryption/external/dbpa_library_wrapper.h" | ||
| #include "parquet/encryption/external/dbpa_interface.h" | ||
| #include "arrow/util/span.h" | ||
| #include <dlfcn.h> | ||
| #include <stdexcept> | ||
| #include <functional> | ||
|
|
||
| #include <iostream> | ||
|
|
||
| #include "arrow/util/io_util.h" | ||
|
|
||
| using ::arrow::util::span; | ||
|
|
||
| namespace parquet::encryption::external { | ||
|
|
||
| // Default implementation for handle closing function | ||
| void DefaultSharedLibraryClosingFn(void* library_handle) { | ||
| auto status = arrow::internal::CloseDynamicLibrary(library_handle); | ||
| if (!status.ok()) { | ||
| std::cerr << "Error closing library: " << status.message() << std::endl; | ||
| } | ||
| } | ||
|
|
||
| DBPALibraryWrapper::DBPALibraryWrapper( | ||
| std::unique_ptr<DataBatchProtectionAgentInterface> agent, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These few lines are quite loaded. Could you add an 2-line comment on what this does?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is described in the .h, but I see your point. Will add a comment here as well. |
||
| void* library_handle, | ||
| std::function<void(void*)> handle_closing_fn) | ||
| : wrapped_agent_(std::move(agent)), | ||
| library_handle_(library_handle), | ||
| handle_closing_fn_(std::move(handle_closing_fn)) { | ||
| // Ensure the wrapped agent is not null | ||
| if (!wrapped_agent_) { | ||
| throw std::invalid_argument("DBPAWrapper: Cannot create wrapper with null agent"); | ||
| } | ||
| if (!library_handle_) { | ||
| throw std::invalid_argument("DBPAWrapper: Cannot create wrapper with null library handle"); | ||
| } | ||
| if (!handle_closing_fn_) { | ||
| throw std::invalid_argument("DBPAWrapper: Cannot create wrapper with null handle closing function"); | ||
| } | ||
| } | ||
|
|
||
| // DBPALibraryWrapper destructor | ||
| // This is the main reason for the decorator/wrapper. | ||
| // This will (a) destroy the wrapped agent, and (b) close the shared library. | ||
| // While the wrapped_agent_ would automatically be destroyed when this object is destroyed | ||
| // we need to explicitly destroy **before** we are able to close the shared library. | ||
| // Doing it in a different order, may cause issues, as by unloading the library may cause the class | ||
| // definition to be unloaded before the destructor completes, and that is likely to cause issues | ||
| // (such as a segfault). | ||
| DBPALibraryWrapper::~DBPALibraryWrapper() { | ||
| // Explicitly destroy the wrapped agent first | ||
| if (wrapped_agent_) { | ||
| DataBatchProtectionAgentInterface* wrapped_agent = wrapped_agent_.release(); | ||
| delete wrapped_agent; | ||
| } | ||
|
|
||
| // Now we can close the shared library using the provided function | ||
| handle_closing_fn_(library_handle_); | ||
| library_handle_ = nullptr; | ||
| } | ||
|
|
||
| // Decorator implementation of Encrypt method | ||
| std::unique_ptr<EncryptionResult> DBPALibraryWrapper::Encrypt( | ||
| span<const uint8_t> plaintext, | ||
| span<uint8_t> ciphertext) { | ||
|
|
||
| return wrapped_agent_->Encrypt(plaintext, ciphertext); | ||
| } | ||
|
|
||
| // Decorator implementation of Decrypt method | ||
| std::unique_ptr<DecryptionResult> DBPALibraryWrapper::Decrypt( | ||
| span<const uint8_t> ciphertext) { | ||
|
|
||
| return wrapped_agent_->Decrypt(ciphertext); | ||
| } | ||
|
|
||
| } // namespace parquet::encryption::external | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| //TODO: figure out the licensing. | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <memory> | ||
| #include <functional> | ||
|
|
||
| #include "parquet/encryption/external/dbpa_interface.h" | ||
| #include "arrow/util/span.h" | ||
|
|
||
| using ::arrow::util::span; | ||
|
|
||
| namespace parquet::encryption::external { | ||
|
|
||
| // Default implementation for shared library closing function | ||
| // This is passed into the constructor of DBPALibraryWrapper, | ||
| // and is used as the default function to close the shared library. | ||
| void DefaultSharedLibraryClosingFn(void* library_handle); | ||
|
|
||
| // Decorator/Wrapper class for the DataBatchProtectionAgentInterface | ||
| // Its main purpose is to close the shared library when Arrow is about to destroy | ||
| // an intance of an DPBAgent | ||
| // | ||
| // In the constructor we allow to pass a function that will be used to close the shared library. | ||
| // This simplifies testing, as we can use a mock function to avoid actually closing the shared library. | ||
| class DBPALibraryWrapper : public DataBatchProtectionAgentInterface { | ||
| private: | ||
| std::unique_ptr<DataBatchProtectionAgentInterface> wrapped_agent_; | ||
| void* library_handle_; | ||
| std::function<void(void*)> handle_closing_fn_; | ||
|
|
||
| public: | ||
| // Constructor that takes ownership of the wrapped agent | ||
| explicit DBPALibraryWrapper( | ||
| std::unique_ptr<DataBatchProtectionAgentInterface> agent, | ||
| void* library_handle, | ||
| std::function<void(void*)> handle_closing_fn = &DefaultSharedLibraryClosingFn); | ||
|
|
||
| // Destructor | ||
| // This is the main reason for the decorator/wrapper. | ||
| // This will (a) destroy the wrapped agent, and (b) close the shared library. | ||
| // While the wrapped_agent_ would automatically be destroyed when this object is destroyed | ||
| // we need to explicitly destroy **before** we are able to close the shared library. | ||
| // Doing it in a different order, may cause issues, as by unloading the library may cause the class | ||
| // definition to be unloaded before the destructor completes, and that is likely to cause issues | ||
| // (such as a segfault). | ||
| ~DBPALibraryWrapper() override; | ||
|
|
||
| // Decorator implementation of Encrypt method | ||
| std::unique_ptr<EncryptionResult> Encrypt( | ||
| span<const uint8_t> plaintext, | ||
| span<uint8_t> ciphertext) override; | ||
|
|
||
| // Decorator implementation of Decrypt method | ||
| std::unique_ptr<DecryptionResult> Decrypt( | ||
| span<const uint8_t> ciphertext) override; | ||
| }; | ||
|
|
||
| } // namespace parquet::encryption::external |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method looks a bit cryptic. Could you add a comment at the method level to describe what this does?