[WIP] Feature: Digital Signature for filters #6158
Conversation
* Add working code to verify signed plugins. * Committing clang-format changes * Adding changes to test branch * Committing clang-format changes * Moving to repo * Committing clang-format changes * Make fixes * Committing clang-format changes * Add * Committing clang-format changes * Finish up security changes * Committing clang-format changes * Fix bad conflict * Add last 2 snprintfs * Add changes to code * Committing clang-format changes * Remove bad file validation check * Committing clang-format changes --------- Co-authored-by: Glenn Song <[email protected]> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
brtnfld
requested review from
bmribler,
byrnHDF,
derobins,
fortnern,
glennsong09,
jhendersonHDF,
lrknox,
mattjala,
qkoziol and
vchoi-hdfgroup
as code owners
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| add_compile_definitions(H5_REQUIRE_DIGITAL_SIGNATURE) | ||
| # link_libraries(gpgme) | ||
| # link_libraries(gpg-error) | ||
| link_libraries(ssl) |
There was a problem hiding this comment.
Consider using target_link_libraries() on specific targets instead of the global link_libraries() command for better modern CMake integration and dependency propagation.
| HGOTO_ERROR(H5E_PLUGIN, H5E_CANTGET, FAIL, "verification check failed"); | ||
| } | ||
| #ifdef H5_HAVE_PARALLEL | ||
| MPI_Finalize(); |
There was a problem hiding this comment.
Calling MPI_Finalize() within the plugin open routine can prematurely shut down MPI. Finalizing MPI should be done at program termination, not within plugin loading.
| MPI_Finalize(); |
| signature = H5PL__get_sig_name_from_path(path, "sig"); | ||
| publickey = H5PL__get_sig_name_from_path(path, "key"); | ||
| verify_result = H5PL__openssl_verify_signature(path, signature, publickey); | ||
| free(signature); |
There was a problem hiding this comment.
Memory allocated in H5PL__get_sig_name_from_path() is done via H5MM_calloc, so it should be freed with H5MM_xfree rather than free() to maintain consistency with HDF5’s memory management.
| free(signature); | |
| signature = (char *)H5MM_xfree(signature); |
| H5_DLL int H5PL__RSA_verify_signature(RSA *rsa, unsigned char *msg_hash, size_t msg_hash_len, const char *msg, | ||
| size_t msg_len, int *authentic); | ||
| H5_DLL RSA *H5PL__create_public_RSA(const char *key); | ||
| H5_DLL char *H5PL__openSSL_read_file(const char *file_path, int *file_length); |
There was a problem hiding this comment.
Typographical suggestion: The function name H5PL__openSSL_read_file uses 'openSSL' with mixed capitalization, whereas H5PL__openssl_verify_signature uses all lowercase for 'openssl'. For consistency, consider using a uniform naming convention (e.g. H5PL__openssl_read_file).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
1 similar comment
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
Adds a new build option HDF5_REQUIRE_SIGNED_PLUGINS (defaulting to ON).
Links the HDF5 library against OpenSSL (libssl, libcrypto) when this option is enabled.
Defines the H5_REQUIRE_DIGITAL_SIGNATURE preprocessor macro.
Interception: Modifies the H5PL__open function to intercept the plugin loading process.
MPI Coordination: In parallel builds, Rank 0 performs the verification and broadcasts the result to other ranks using MPI_Bcast.
Verification Workflow:
It relies on external shell commands (system()) and GNU objcopy to copy the plugin file and extract a signature section named "sig".
It reads the extracted signature and the "clean" plugin binary into memory.
It uses OpenSSL routines (EVP_DigestVerify, SHA-256) to verify the digital signature against a public key file.
Helper Functions: Adds several internal functions to handle file reading, RSA key parsing, and filename validation.
Updates private headers to include OpenSSL dependencies and declare the new internal verification functions.
Important
Adds digital signature verification for HDF5 plugins using OpenSSL, with build system and plugin loading logic updates.
HDF5_REQUIRE_SIGNED_PLUGINSoption inCMakeLists.txt(default OFF).libsslandlibcryptowhen enabled.H5_REQUIRE_DIGITAL_SIGNATUREmacro.H5PL__open()inH5PLint.cto verify plugin signatures using OpenSSL.MPI_Bcast.H5PLprivate.hto include OpenSSL headers and declare new functions.H5pubconf.h.in.This description was created by
for 579ce87. You can customize this summary. It will automatically update as commits are pushed.