Feature: Digital Signature Verification for HDF5 Plugins#6198
Open
Feature: Digital Signature Verification for HDF5 Plugins#6198
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 <gsong@hdfgroup.org> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Implements a signature verification cache to avoid redundant cryptographic
operations when loading the same plugin multiple times. The cache stores
verification results (success/failure) indexed by plugin path and file
modification time, enabling instant verification on cache hits while
maintaining security through mtime-based invalidation.
Implementation Details:
1. Cache Structure (H5PLsig.c):
- H5PL_signature_cache_entry_t: path, mtime, verified status
- Dynamic array with doubling growth strategy (initial capacity: 8)
- Stores both positive and negative verification results
2. Cache Operations:
- H5PL__check_signature_cache(): Check cache, validate mtime
- H5PL__update_signature_cache(): Add/update cache entries
- Integrated into H5PL__verify_signature_appended()
- Cache invalidation on file modification (mtime check)
3. Code Cleanup:
- Removed commented GPG/GPGME configuration from H5pubconf.h.in
- Eliminated dead code since implementation uses OpenSSL
4. Test Infrastructure:
- New test executable: h5signverifytest
- 6 comprehensive test cases:
* Verify signed plugin (positive test)
* Verify unsigned plugin (negative test)
* Verify tampered plugin (security test)
* Cache basic functionality (performance test)
* Cache invalidation on modification (security test)
* Cache negative results (DoS prevention)
- CMake integration with dependency management
- CreateTamperedPlugin.cmake: Generate tampered plugins for testing
Files Modified:
- src/H5PLsig.c: Cache implementation and integration
- src/H5pubconf.h.in: Removed GPG comments
- tools/test/h5sign/CMakeLists.txt: Added h5signverifytest build
- tools/test/h5sign/CMakeTests.cmake: Added verification tests
Files Added:
- tools/test/h5sign/h5signverifytest.c: Comprehensive test suite
- tools/test/h5sign/CreateTamperedPlugin.cmake: Tamper test helper
Performance: Eliminates cryptographic overhead on repeated plugin loads
(cache hit returns instantly vs. full RSA signature verification).
Security: mtime-based invalidation ensures modified plugins are re-verified,
preventing cache from masking tampering. Negative result caching prevents
retry attacks on invalid plugins.
Backward Compatibility: Fully backward compatible, cache is transparent
optimization with no API changes.
Resolved conflicts by keeping HEAD implementations: - CMakeLists.txt: Retained comprehensive KeyStore configuration with OpenSSL validation - src/H5PLint.c: Kept refactored H5PL__verify_plugin_signature() implementation The HEAD versions contain the latest KeyStore implementation with: - Support for multiple trusted keys via directory - Backward compatibility with single embedded key - Proper OpenSSL target-based linking - Windows support with Advapi32 - Comprehensive validation and error messages
Phase 1: Code Cleanup (Completed) ✅ Moved H5PL_MAX_PLUGIN_SIZE to file scope - The constant is now properly defined at file scope in src/H5PLsig.c:110-111 instead of inside a function with #define/#undef. ✅ Fixed misleading error message - Changed the error message at src/H5PLsig.c:1491 from referencing non-existent "h5sign --verify" to a more accurate message about signature compatibility. Phase 2: Algorithm Agility in h5sign (Completed) ✅ Added command-line option - New -a/--algorithm option allows users to select hash algorithms: sha256, sha384, sha512, sha256-pss, sha384-pss, sha512-pss ✅ Created algorithm parser - New parse_algorithm_name() function (tools/src/h5sign/h5sign.c:283-329) validates and maps algorithm names to OpenSSL EVP functions and algorithm IDs ✅ Updated signing function - Modified sign_plugin_file() to accept hash algorithm and algorithm ID parameters, with PSS padding support ✅ Updated help text - Enhanced usage documentation with algorithm options and examples ✅ Backward compatible - SHA-256 remains the default algorithm, so existing workflows continue to work unchanged Phase 3: Chunked I/O for Verification (Completed) ✅ Added constants - Defined H5PL_VERIFY_CHUNK_SIZE (64KB) and H5PL_MEMORY_THRESHOLD (16MB) in src/H5PLsig.c:113-117 ✅ Created chunked verification helper - New H5PL__verify_with_chunked_io() function (src/H5PLsig.c:1139-1224) performs signature verification using 64KB chunks, including PSS padding support ✅ Implemented hybrid approach - Smart verification strategy: Small files (≤16MB) + multiple keys: Reads file once into memory, verifies with all keys (optimized for speed) Large files (>16MB) OR single key: Uses chunked I/O, reducing memory from 1GB to 64KB (optimized for memory) Key Benefits Algorithm Flexibility: Users can now sign plugins with SHA-384 or SHA-512 for enhanced security, or use PSS padding variants Memory Efficiency: Large plugin verification now uses only 64KB of memory instead of up to 1GB Performance Optimized: Small files with multiple keys still use the fast memory-optimized path Backward Compatible: All changes are additive; existing signed plugins and workflows continue to work PSS Padding Support: Both signing and verification now properly handle RSA-PSS padding mode
- Fix private key path in test environment: use CMAKE_BINARY_DIR instead of HDF5_TEST_BINARY_DIR since keys are generated in the top-level build directory - Fix h5sign.c to use algorithm_id parameter instead of undefined HASH_ALGORITHM_ID macro in output messages - Remove invalid DEPENDS from post-build signing command in SignPlugin.cmake
brtnfld
requested review from
derobins,
fortnern,
jhendersonHDF,
lrknox,
qkoziol and
vchoi-hdfgroup
as code owners
brtnfld
added
the
Component - C Library
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
1 similar comment
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
H5PL__validate_directory_permissions was duplicated in each of the four platform-specific functions (Unix + Windows pairs for both the iterate and load paths). Move the check one level up into the two platform-agnostic callers: - H5PL__path_table_iterate (before dispatching to process_path) - H5PL__find_plugin_in_path_table (before dispatching to find_in_path) Remove it from H5PL__path_table_iterate_process_path and H5PL__find_plugin_in_path (all four platform variants). No behavior change: a world-writable directory is still skipped before any platform-specific directory scan begins. This also ensures the check cannot be omitted if a new platform variant is added later.
Previously H5PL__verify_with_all_keys called H5PL__verify_with_chunked_io
for each key in the keystore, allocating a 1MB buffer and re-reading +
re-hashing the entire plugin file on every iteration. With N keys and
a 100 MB plugin this causes N * 100 MB of unnecessary I/O.
Separate the hashing step from the per-key verification step:
- Replace H5PL__verify_with_chunked_io with H5PL__hash_file_binary,
which computes the raw message digest (EVP_DigestInit_ex /
EVP_DigestUpdate / EVP_DigestFinal_ex) exactly once.
- H5PL__verify_with_all_keys hashes the binary first, then loops
over the keystore using EVP_PKEY_CTX + EVP_PKEY_verify to check
the pre-computed digest against the stored signature for each key.
No file I/O occurs inside the key loop.
Also remove the now-obsolete keys_update_failed counter and
H5PL_VERIFY_REASON_UPDATE_FAILED enum value; I/O failure during
hashing aborts the whole operation before the key loop begins.
Replace sleep(2) with HDsleep(2) in test_signature_cache_invalidation to fix compilation on Windows/MSVC, where the POSIX sleep() is not available.
A NULL DACL (returned by GetNamedSecurityInfoA when a directory grants unrestricted access to everyone) would be passed to GetEffectiveRightsFromAclA, causing unpredictable behavior or API failures. Add an explicit NULL DACL check that fails with a clear security error before any ACL traversal occurs.
fgets(line, sizeof(line), fp) silently truncates lines longer than 255 characters, leaving the remainder in the stream. A subsequent read could pick up a trailing fragment that happens to be exactly 64 valid hex characters and register it as a revoked hash. After each fgets call, check whether the buffer contains a newline. If not (and feof is not set), consume and discard characters up to the next newline, then skip the current chunk entirely so no partial line is ever evaluated as a hash.
Replace bare free() calls with HDfree() when releasing canonical_dir and canonical_file buffers returned by HDrealpath, to conform with HDF5 library memory management conventions.
…checks - L1: Windows ACL checks now fail closed when GetEffectiveRightsFromAclA returns an error, preventing silent pass-through on misconfigured ACLs - L2: Add H5_STATIC_ASSERT(sizeof(size_t) >= 4) to document the safe cast assumption in footer binary size calculation - L3: Free revocation list on keystore initialization failure to prevent leak when init fails after revoked signatures are loaded - L4: Free revocation list in H5PL__cleanup_signature_cache to prevent leak on library termination - L5: Reorder H5PL__add_key_to_keystore to duplicate the source string before storing the key pointer, preventing an orphaned key on strdup failure - P2: Cache directory permission validation results per path entry so H5PL__validate_directory_permissions is called at most once per path, avoiding repeated SID/ACL queries on Windows for every plugin search
…ature constant - Add comment clarifying offset tracking in non-pread read path (for error reporting) - Differentiate keystore error message between "no keystore configured" and "attempted to load from <path>" when key loading fails - Move H5PL_MAX_SIGNATURE_SIZE to H5PLsig.h so signer and verifier share the same limit, and use it in h5sign instead of a hardcoded 8192
…ignature_revoked The assignment ret_value = true was immediately overwritten by the HGOTO_DONE(true) macro, making it dead code.
H5PL_sig_footer_t footer was declared but never used; the footer is encoded directly into footer_buf[].
…witch - Replace magic offset 8 with H5PL_SIG_FOOTER_MAGIC_OFFSET, derived from the documented footer field sizes, so layout changes cannot silently break the already-signed detection in h5sign (M4) - Add H5PL_SIG_ALGO_IS_PSS(id) macro to H5PLsig.h and replace the three-way SHA256/384/512-PSS OR-chain in both H5PLsig.c and h5sign.c (M5) - Add explicit cases for PSS variants in h5sign's verbose algorithm switch so they print a human-readable name instead of a raw hex id (M6)
The wrapper added no logic — it called H5PL__verify_signature_appended, checked the return value, and forwarded it. It also used FUNC_ENTER_PACKAGE_NOERR while returning FAIL, which silently suppressed error stack entries for signature failures. Call H5PL__verify_signature_appended directly from H5PL__open, which already wraps the call with HGOTO_ERROR to push onto the error stack.
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
- Replace hbool_t/TRUE/FALSE with bool/true/false — hbool_t is a deprecated alias for bool and TRUE/FALSE are not defined (errors) - Replace HDfree() with free() for HDrealpath-allocated buffers — HDfree is not defined in this codebase; realpath() returns malloc memory that must be freed with free() - Remove H5_STATIC_ASSERT(sizeof(size_t) >= 4) — the macro does not exist; the assertion is trivially true on all supported platforms - Remove H5PL__create_public_RSA_from_string — defined but never called - Change FUNC_ENTER_PACKAGE to FUNC_ENTER_PACKAGE_NOERR in H5PL__create_public_RSA_from_file — the function never calls HGOTO_ERROR, so the err_occurred variable injected by H5_API_SETUP_ERROR_HANDLING was unused
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
1 similar comment
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
1 similar comment
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
- Extract H5PL__check_path_perms_cached() to deduplicate permission-check logic in H5PLpath.c - Add report_openssl_error() helper and algo_table[] lookup table in h5sign to eliminate repetitive OpenSSL error-reporting and algorithm if-else chains - Fix OpenSSL 1.1.x compatibility: use EVP_PKEY_bits() instead of EVP_PKEY_get_bits(), and guard EVP_MD_get0_name() behind version check - Centralize Windows security resource cleanup in done: block in H5PLsig.c - Make signature cache update failures non-fatal (log warning, clear error) - Add explicit reserved-algorithm cases (SHA3-256, BLAKE3) with better error messages distinguishing reserved vs unknown algorithm IDs - Move SignPlugin.cmake include to parent tools/test/CMakeLists.txt to avoid duplication across tool-test subdirectories - Quote paths in test_plugin_signature.c system() call for safety - Drop unused OpenSSL::SSL from link libraries (only libcrypto is needed)
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
1 similar comment
Contributor
✅ Maven Staging Tests PassedMaven artifacts successfully generated and validated. Ready for Maven deployment to GitHub Packages. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces a robust security framework for digitally signing and verifying HDF5 dynamic plugins (filters). This feature ensures that the HDF5 library loads only trusted, unmodified plugins, significantly enhancing security in environments where plugins are distributed dynamically.
Key Features
1. Plugin Signature Verification
HDF5_PLUGIN_KEYSTORE_DIR).HDF5_PLUGIN_KEYSTORE).2. New Tool:
h5signh5sign, located intools/src/h5sign.3. Build & Configuration Changes
HDF5_REQUIRE_SIGNED_PLUGINS(Default:OFF). When enabled, HDF5 enforces signature verification and refuses to load unsigned or invalid plugins.OpenSSLas a required dependency when signed plugins are enabled.4. CI/CD & Infrastructure Updates
.github/workflows/signed-plugins.ymlto specifically test the new signature verification logic in serial and parallel configurations.Technical Details
New Files
src/H5PLsig.c/.h: Core logic for reading signatures, managing the KeyStore, and performing OpenSSL verification.tools/src/h5sign/: Source code for the signing utility.release_docs/PLUGIN_SIGNATURE_README.md: Comprehensive documentation for developers and users regarding key generation, signing, and air-gapped environment handling.test/test_plugin_signature.c: New test suite covering valid signatures, tampered binaries, and invalid keys.Modified Files
src/H5PLint.c: Hooked into the plugin loading process to trigger verification beforedlopen/LoadLibrary.CMakeLists.txt&src/CMakeLists.txt: build logic for OpenSSL linking and new source files..github/workflows/*: CI updatesTesting
h5signverifytestandtest_plugin_signatureto the test suite.Documentation
release_docs/PLUGIN_SIGNATURE_README.mddetailing the security model, usage instructions, and best practices.Fixes #5116