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
Revert two incidental blank line removals in H5PLint.c and H5PLpath.c that were not part of the digital signature feature.
Collaborator
Author
|
@jhendersonHDF @glennsong09, this is ready for review when you get the chance. |
H5Z_find: when a required filter is not in the registered table, attempt H5PL_load so that any real load failure (e.g. signature verification) appears in the HDF5 error stack. Without this, H5Dcreate reported only "filter not registered" with no indication that the plugin was found but could not be loaded. When built with H5_REQUIRE_DIGITAL_SIGNATURE, the outermost error message also directs the user to sign the plugin with h5sign. h5sign: add S_ISREG check before the permissions check so that passing a directory path (e.g. the key directory instead of the key file) reports "not a regular file" rather than the misleading "insecure permissions (755)".
Footer format: - Reorder footer layout to magic-first for early validation - Remove unused H5PL_SIG_FOOTER_MAGIC_OFFSET macro - Remove dead #ifdef H5_REQUIRE_DIGITAL_SIGNATURE section in header - Update tests to use H5PL_sig_encode_footer instead of manual encoding - Fix footer layout in shell scripts and README Remove over-engineering: - Remove signature verification cache (plugins already cached by loader) - Remove directory permission checks (Unix S_IWOTH, Windows ACL) - Remove private key permission check from h5sign tool - Remove H5Z_find plugin load/register fallback - Remove pre-append ftruncate in h5sign - Remove redundant SIZE_MAX overflow check - Remove special-case "upgrade HDF5" error for reserved algorithms - Delete empty h5signgentest.h Error handling: - Replace verbose diagnostic dumps with concise error messages - Simplify revoked signature error to single line Build system: - Move H5_REQUIRE_DIGITAL_SIGNATURE to H5pubconf.h - Make HDF5_PLUGIN_KEYSTORE_DIR optional (env var fallback) - Simplify CMake generator expressions in test files - Move H5PL_MAX_PLUGIN_SIZE to shared header H5PLsig.h Documentation: - Remove ~370 lines of air-gapped environment content - Add Keystore Security Requirements section - Fix version info (2.2.0+) - Remove feature/dig_sig_ver from workflow triggers Cleanup: - Move EVP_PKEY_free to done section in H5PL__process_key_file - Remove memcpy in revocation hash lookup (compare directly) - Define H5PL_REVOKED_SIGS_FILENAME macro - Rename cleanup function to H5PL__cleanup_signature_resources - Add format version compatibility comment
glennsong09
reviewed
Mar 20, 2026
- Change H5_ATTR_UNUSED to static inline on H5PL_sig_encode_footer and H5PL_sig_decode_footer - Add buf_size parameter with assert to both encode/decode functions; update all 6 call sites to pass sizeof(buf) - Convert algorithm ID #defines to H5PL_sig_algo_t enum for type safety; update footer struct, H5PL__get_hash_algorithm, h5sign function signatures, and add explicit casts for byte conversion
Collaborator
Author
Category 1: Footer Format — Magic First (5 comments)
Category 2: Footer Encode/Decode Improvements (4 comments)
Category 3: Dead Code Removal (4 comments)
Category 4: Security Check Concerns (3 comments)
Category 5: Error Handling & Diagnostics (3 comments)
Category 6: Revocation Logic (3 comments + 1 related)
Category 7: Caching (2 comments)
Category 8: Build/CMake (4 comments)
Category 9: Miscellaneous (5 comments)
Category 10: Test Code (2 comments)
Category 11: Workflow/Doc Cleanup (2 comments)
Category 12: Design Questions (2 comments)
|
Per jhendersonHDF and glennsong09 review feedback: the README should state what to do, not how to do it. Removed all specific chmod, mkdir, openssl genrsa commands, shell scripts, incident response timelines, keystore audit scripts, CI/CD examples, and trade-off tables. Reduced from 1165 lines to 281 lines. Kept h5sign usage, technical format details, FAQ, and troubleshooting.
- Use cmake_dependent_option for HDF5_LOCK_PLUGIN_KEYSTORE - Move HDF5_PLUGIN_KEYSTORE_DIR inside if(HDF5_REQUIRE_SIGNED_PLUGINS) - Remove build-time keystore directory existence checks - Change no-keystore message from STATUS to NOTICE - Remove Advapi32 link and unused Windows security headers - Replace H5E_clear_stack with H5E_pause/resume_stack
- Validate magic and format version inside H5PL_sig_decode_footer (returns bool); simplify read_and_validate_footer accordingly - Add H5PL_SIGNATURE_HASH_HEX_LEN constant to clarify hex-chars vs bytes in revocation hash comparison - Check signed file size against H5PL_MAX_PLUGIN_SIZE in h5sign - Simplify verification error message (remove verbose key stats) - Remove unimplemented runtime lock_keystore feature from README - Trim prescriptive security guidance per reviewer feedback - Add forward-compatibility comment on format version constant - Document that compile-time keystore path is visible in binary
- Add test_verify_revoked_plugin to h5signverifytest: creates a revoked_signatures.txt with the signed plugin's signature hash, re-initializes HDF5, and verifies the plugin is rejected - Link test with OpenSSL::Crypto for SHA-256 hash computation - Document revoked_signatures.txt format in PLUGIN_SIGNATURE_README - Add "plugin signature has been revoked" to troubleshooting table
The revocation test called package-private functions directly without going through the public API, so H5PL_init_g was never set to true. This caused H5PL_term_package() to skip keystore cleanup during H5close(), leaving a stale keystore (without the revocation list) across the H5close/H5open cycle. Add H5PLget_loading_state() call to properly initialize the H5PL package so that H5close() cleans up the keystore and forces re-initialization with the revocation file on re-open.
Document the new optional plugin signature verification in the Library section and the h5sign signing tool in the Tools section.
tools/src/h5sign/h5sign.c
Outdated
| goto done; | ||
| } | ||
|
|
||
| if (file_size > H5PL_MAX_PLUGIN_SIZE) { |
Collaborator
There was a problem hiding this comment.
Moving part of the old comment as half is outdated now: related, though not that it's likely to happen, this seems like it should check if file_size + bytes_added > H5PL_MAX_PLUGIN_SIZE so that a load error doesn't occur later. If bytes_added isn't known at this point, the file size should be checked after being signed.
- Replace assert with real error check for buf_size in decode_footer - Add backward-compatibility comment and range check for format version - Post-sign size check (file_size + sig_len + footer) already present
- Per CodeQL best practice, permissions: contents: read moved from workflow level to job level (test-signed-plugins) - Remove leftover icacls step from Windows ACL permission checks that were removed in earlier cleanup
The binary size guard was firing on the raw on-disk file size, which for an already-signed file includes the old signature and footer bytes. This caused --force re-signing to fail for any plugin whose total size exceeded H5PL_MAX_PLUGIN_SIZE even when the binary itself was within the limit. Move the check to after the detect/strip block so file_size always reflects the binary-only size, consistent with how the verifier enforces the same limit (binary_size_off) in H5PLsig.c.
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
10 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