update documentation and tests

Author: brtnfld

CMakeLists.txt

@@ -793,8 +793,11 @@ set(HDF5_PLUGIN_KEYSTORE_DIR "" CACHE PATH
     "Directory containing trusted public keys (.pem files) for plugin verification")
 # Security Note: For production deployments where environment variables may be
 # controllable by untrusted users (e.g., HPC clusters, multi-tenant systems),
-# consider enabling HDF5_LOCK_PLUGIN_KEYSTORE to prevent runtime override of
-# the keystore directory via the HDF5_PLUGIN_KEYSTORE environment variable.
+# you can prevent runtime override of the keystore directory via:
+#   1. Compile-time: Enable HDF5_LOCK_PLUGIN_KEYSTORE (requires rebuild)
+#   2. Runtime: Create lock file (works with pre-built binaries):
+#      - Unix/Linux: /etc/hdf5/lock_keystore
+#      - Windows: C:\ProgramData\HDF_Group\HDF5\lock_keystore
 
 # Path to public key file for plugin signature verification (backward compatibility)
 set(HDF5_PLUGIN_PUBLIC_KEY_FILE "" CACHE FILEPATH
@@ -804,6 +807,9 @@ set(HDF5_PLUGIN_PUBLIC_KEY_FILE "" CACHE FILEPATH
 option(HDF5_PLUGIN_KEYSTORE_DEBUG "Enable debug output for plugin KeyStore initialization" OFF)
 
 # Security: Disable environment variable override (prevents runtime keystore redirection)
+# Note: Sysadmins can also lock the keystore at runtime without recompiling by
+# creating a lock file (/etc/hdf5/lock_keystore on Unix, or
+# C:\ProgramData\HDF_Group\HDF5\lock_keystore on Windows).
 option(HDF5_LOCK_PLUGIN_KEYSTORE "Disable HDF5_PLUGIN_KEYSTORE environment variable override (security hardening)" OFF)
 mark_as_advanced(HDF5_LOCK_PLUGIN_KEYSTORE)
 

release_docs/PLUGIN_SIGNATURE_README.md

@@ -1067,6 +1067,52 @@ The current system makes conscious trade-offs:
    - Ensure keystore directory is not world-writable
    - Use NTFS permissions to restrict write access
 
+6. **Environment Variable Security (Production Deployments)**
+
+   In multi-tenant or HPC environments where untrusted users can control environment variables, you can lock the keystore location to prevent them from overriding `HDF5_PLUGIN_KEYSTORE` with a malicious keystore.
+
+   **Runtime Lock (No Recompilation Required):**
+   ```bash
+   # Unix/Linux: Create lock file to disable environment variable override
+   sudo mkdir -p /etc/hdf5
+   sudo touch /etc/hdf5/lock_keystore
+
+   # Windows: Create lock file
+   mkdir "C:\ProgramData\HDF_Group\HDF5"
+   type nul > "C:\ProgramData\HDF_Group\HDF5\lock_keystore"
+   ```
+
+   **Compile-Time Lock (Requires Rebuild):**
+   ```bash
+   # Configure HDF5 with locked keystore (completely disables env var)
+   cmake -DHDF5_LOCK_PLUGIN_KEYSTORE=ON \
+         -DHDF5_PLUGIN_KEYSTORE_DIR=/etc/hdf5/keystore \
+         /path/to/hdf5/source
+   ```
+
+   **When to Use:**
+   - ✅ HPC clusters with untrusted users
+   - ✅ Multi-tenant systems
+   - ✅ Production servers with strict security requirements
+   - ✅ Pre-built binaries distributed to security-critical environments
+
+   **How It Works:**
+   1. If lock file exists, `HDF5_PLUGIN_KEYSTORE` environment variable is ignored
+   2. HDF5 will only use the compile-time configured keystore (`HDF5_PLUGIN_KEYSTORE_DIR`)
+   3. Prevents privilege escalation via keystore override attacks
+   4. System administrators can apply this to pre-built HDF5 libraries without recompilation
+
+   **Verification:**
+   ```bash
+   # Test that environment variable is ignored after locking
+   export HDF5_PLUGIN_KEYSTORE=/tmp/fake_keystore
+
+   # Enable debug output to see which keystore is used
+   HDF5_PLUGIN_KEYSTORE_DEBUG=1 h5dump test_file.h5
+
+   # Expected output: "Skipping HDF5_PLUGIN_KEYSTORE environment variable (locked by sysadmin)"
+   ```
+
 ---
 
 ## Troubleshooting

src/H5PLsig.c

@@ -61,6 +61,15 @@
  *
  * If plugin loading is ever made to bypass the global lock, these data structures will
  * require explicit mutex protection or atomic operations.
+ *
+ * TODO: If H5PL__load is ever refactored to support fine-grained locking or lock-free
+ *       concurrent plugin loading, wrap these static globals in a struct protected by
+ *       a dedicated mutex (e.g., H5PL_sig_lock_g). This affects:
+ *       - H5PL_keystore_g and related counters (lines 73-76)
+ *       - H5PL_revoked_sigs_g and related counters (lines 84-87)
+ *       - H5PL_sig_cache_g and related counters (lines 97-99)
+ *       All read/write operations on these variables must be synchronized if the
+ *       global library lock is removed or bypassed for plugin operations.
  */
 
 /* KeyStore entry for storing multiple trusted public keys */
@@ -69,7 +78,9 @@ typedef struct H5PL_keystore_entry_t {
     char     *source; /* Key source (filename or "embedded") for debugging */
 } H5PL_keystore_entry_t;
 
-/* KeyStore for signature verification */
+/* KeyStore for signature verification
+ * TODO (Thread Safety): Requires mutex protection if global lock is removed
+ */
 static H5PL_keystore_entry_t *H5PL_keystore_g             = NULL;
 static size_t                 H5PL_keystore_count_g       = 0;
 static size_t                 H5PL_keystore_capacity_g    = 0;
@@ -81,6 +92,7 @@ typedef struct H5PL_revoked_signature_t {
     unsigned char hash[H5PL_SIGNATURE_HASH_SIZE]; /* SHA-256 hash of signature */
 } H5PL_revoked_signature_t;
 
+/* TODO (Thread Safety): Requires mutex protection if global lock is removed */
 static H5PL_revoked_signature_t *H5PL_revoked_sigs_g             = NULL;
 static size_t                    H5PL_revoked_sigs_count_g       = 0;
 static size_t                    H5PL_revoked_sigs_capacity_g    = 0;
@@ -93,7 +105,9 @@ typedef struct H5PL_signature_cache_entry_t {
     bool   verified; /* Verification status (true=success, false=failure) */
 } H5PL_signature_cache_entry_t;
 
-/* Signature verification cache */
+/* Signature verification cache
+ * TODO (Thread Safety): Requires mutex protection if global lock is removed
+ */
 static H5PL_signature_cache_entry_t *H5PL_sig_cache_g          = NULL;
 static size_t                        H5PL_sig_cache_count_g    = 0;
 static size_t                        H5PL_sig_cache_capacity_g = 0;
@@ -110,8 +124,8 @@ static size_t                        H5PL_sig_cache_capacity_g = 0;
 /* Maximum plugin file size (1GB - prevents unreasonable allocations) */
 #define H5PL_MAX_PLUGIN_SIZE ((HDoff_t)(1024 * 1024 * 1024))
 
-/* I/O chunk size for verification (64KB - matches h5sign) */
-#define H5PL_VERIFY_CHUNK_SIZE ((size_t)(64 * 1024))
+/* I/O chunk size for verification (1MB - optimized for modern I/O subsystems) */
+#define H5PL_VERIFY_CHUNK_SIZE ((size_t)(1024 * 1024))
 
 /* Memory threshold for multi-key optimization (16MB) */
 #define H5PL_MEMORY_THRESHOLD ((size_t)(16 * 1024 * 1024))
@@ -736,6 +750,54 @@ H5PL__load_keys_from_directory(const char *dir_path)
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5PL__load_keys_from_directory() */
 
+/*-------------------------------------------------------------------------
+ * Function:    H5PL__is_keystore_locked
+ *
+ * Purpose:     Check if keystore environment variable override is locked
+ *              by presence of system lock file
+ *
+ *              Lock file locations:
+ *              - Unix/Linux: /etc/hdf5/lock_keystore
+ *              - Windows: C:\ProgramData\HDF_Group\HDF5\lock_keystore
+ *
+ *              This allows system administrators to disable the
+ *              HDF5_PLUGIN_KEYSTORE environment variable on pre-built
+ *              binaries without recompiling HDF5.
+ *
+ * Return:      true if locked, false otherwise
+ *-------------------------------------------------------------------------
+ */
+static bool
+H5PL__is_keystore_locked(void)
+{
+    h5_stat_t st;
+    bool      ret_value = false;
+
+    FUNC_ENTER_PACKAGE_NOERR
+
+#ifndef H5_HAVE_WIN32_API
+    /* Unix/Linux: Check for /etc/hdf5/lock_keystore */
+    if (HDstat("/etc/hdf5/lock_keystore", &st) == 0) {
+        ret_value = true;
+#ifdef H5PL_DEBUG_KEYSTORE
+        fprintf(stderr, "HDF5 KeyStore: Environment variable override disabled by /etc/hdf5/lock_keystore\n");
+#endif
+    }
+#else
+    /* Windows: Check for C:\ProgramData\HDF_Group\HDF5\lock_keystore */
+    if (HDstat("C:\\ProgramData\\HDF_Group\\HDF5\\lock_keystore", &st) == 0) {
+        ret_value = true;
+#ifdef H5PL_DEBUG_KEYSTORE
+        fprintf(stderr,
+                "HDF5 KeyStore: Environment variable override disabled by "
+                "C:\\ProgramData\\HDF_Group\\HDF5\\lock_keystore\n");
+#endif
+    }
+#endif
+
+    FUNC_LEAVE_NOAPI(ret_value)
+} /* end H5PL__is_keystore_locked() */
+
 /*-------------------------------------------------------------------------
  * Function:    H5PL__init_keystore
  *
@@ -771,17 +833,25 @@ H5PL__init_keystore(void)
 
     /* 1. Check environment variable (highest priority) */
 #ifndef H5PL_DISABLE_ENV_KEYSTORE
-    if (NULL != (env_keystore = getenv("HDF5_PLUGIN_KEYSTORE"))) {
-        if (H5PL__load_keys_from_directory(env_keystore) < 0)
-            HGOTO_ERROR(H5E_PLUGIN, H5E_CANTLOAD, FAIL, "failed to load keys from HDF5_PLUGIN_KEYSTORE: %s",
-                        env_keystore);
-        keys_loaded = true;
-
-        /* Load revoked signatures from same directory */
-        if (H5PL__load_revoked_signatures(env_keystore) < 0) {
-            /* Non-fatal - continue even if revoked signatures fail to load */
+    /* Check if environment variable override is locked by runtime lock file */
+    if (!H5PL__is_keystore_locked()) {
+        if (NULL != (env_keystore = getenv("HDF5_PLUGIN_KEYSTORE"))) {
+            if (H5PL__load_keys_from_directory(env_keystore) < 0)
+                HGOTO_ERROR(H5E_PLUGIN, H5E_CANTLOAD, FAIL, "failed to load keys from HDF5_PLUGIN_KEYSTORE: %s",
+                            env_keystore);
+            keys_loaded = true;
+
+            /* Load revoked signatures from same directory */
+            if (H5PL__load_revoked_signatures(env_keystore) < 0) {
+                /* Non-fatal - continue even if revoked signatures fail to load */
+            }
         }
     }
+#ifdef H5PL_DEBUG_KEYSTORE
+    else {
+        fprintf(stderr, "HDF5 KeyStore: Skipping HDF5_PLUGIN_KEYSTORE environment variable (locked by sysadmin)\n");
+    }
+#endif
 #else
     /* Environment variable override disabled at compile time (security hardening) */
     env_keystore = NULL; /* Suppress unused variable warning */