Add ML-KEM PQC support to hsmCompatVerify tools#5356
Conversation
Adds comprehensive ML-KEM (FIPS 203) support to hsmCompatVerify verification tools: **PQC Mode:** - --pqc flag enables ML-DSA for CA and ML-KEM for transport/storage - --pqc-ca-algorithm: ml-dsa-44, ml-dsa-65, ml-dsa-87 (default: ml-dsa-65) - --pqc-kem-algorithm: ml-kem-512, ml-kem-768, ml-kem-1024 (default: ml-kem-768) - Uses NIST FIPS 203/204 standard hyphenated algorithm names **User Key Type Support:** - --user-key-type: RSA, EC, or ML-KEM (default: ML-KEM if --pqc, RSA otherwise) - Allows verifying ML-KEM transport with different user key types - Isolates ML-KEM transport functionality from user key operations **PKCS#12 Modes:** - --pkcs12-mode: kwp (AES-KWP), cbc (AES-256-CBC PBKDF2), legacy (3DES-CBC) - Default: kwp - Supports ML-KEM key export to PKCS#12 **CryptoUtil Integration:** - Uses CryptoUtil.getMLDSAStrength() for algorithm name mapping - Uses CryptoUtil.getMLDSASignatureAlgorithm() for signature algorithms - Uses CryptoUtil.getMLKEMStrength() for KEM strength mapping - Uses CryptoUtil.encapsulateMLKEM() for key wrapping - Uses CryptoUtil.decapsulateMLKEM() for key unwrapping (3 occurrences) - Replaces ~100 lines of switch statements and KEM boilerplate with helper calls **Bug Fixes:** - Fix --p12-output path handling to respect relative paths from CWD - Add ML-KEM recognition to CryptoUtil.unwrap() **Verification Improvements:** - --test-decrypt-user flag for debugging wrapped key data - Enhanced configuration output for PQC-specific options - Hides non-applicable options in PQC mode This enables end-to-end verification of ML-KEM key encapsulation for KRA transport and storage certificates, validating the PQC migration path. IDM-6295, IDM-5815, IDM-5475 Assisted-by: Claude
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements Post-Quantum Cryptography (PQC) support in the HSM compatibility verification tools, adding ML-DSA for CA signing and ML-KEM for KRA transport and storage. The changes include new command-line options, refactored initialization logic, and updated archival/verification workflows. Review feedback identifies a potential deadlock in process execution, security concerns regarding temporary file permissions and default charsets, and recommends using temporary HSM objects to avoid persistent artifacts.
| int exitCode = p.waitFor(); | ||
| if (exitCode != 0) { | ||
| java.io.BufferedReader reader = new java.io.BufferedReader( | ||
| new java.io.InputStreamReader(p.getInputStream())); | ||
| String line; | ||
| while ((line = reader.readLine()) != null) { | ||
| log(" certutil: " + line); | ||
| } | ||
| throw new Exception("Failed to create NSS database (exit code: " + exitCode + ")"); | ||
| } |
There was a problem hiding this comment.
Calling p.waitFor() before consuming the process output can lead to a deadlock if the process produces enough output to fill the OS pipe buffer. The output should be read while the process is running or before waiting for it to complete to ensure the process can finish execution.
StringBuilder output = new StringBuilder();
try (java.io.BufferedReader reader = new java.io.BufferedReader(
new java.io.InputStreamReader(p.getInputStream()))) {
String line;
while ((line = reader.readLine()) != null) {
output.append(line).append("\n");
}
}
int exitCode = p.waitFor();
if (exitCode != 0) {
log(" certutil output:\n" + output.toString());
throw new Exception("Failed to create NSS database (exit code: " + exitCode + ")");
}| java.io.File tmpPwdFile = java.io.File.createTempFile("nsspwd", ".txt"); | ||
| tmpPwdFile.deleteOnExit(); | ||
| java.nio.file.Files.write(tmpPwdFile.toPath(), pkiservPasswd.getBytes()); |
There was a problem hiding this comment.
Using File.createTempFile and String.getBytes() without an explicit charset can lead to security and portability issues. File.createTempFile may create files with default permissions that are too permissive for sensitive data like passwords. It is recommended to use java.nio.file.Files.createTempFile (which typically uses more restrictive permissions on POSIX systems) and to specify StandardCharsets.UTF_8 when writing the password.
| java.io.File tmpPwdFile = java.io.File.createTempFile("nsspwd", ".txt"); | |
| tmpPwdFile.deleteOnExit(); | |
| java.nio.file.Files.write(tmpPwdFile.toPath(), pkiservPasswd.getBytes()); | |
| java.nio.file.Path tmpPwdPath = java.nio.file.Files.createTempFile("nsspwd", ".txt"); | |
| java.io.File tmpPwdFile = tmpPwdPath.toFile(); | |
| tmpPwdFile.deleteOnExit(); | |
| java.nio.file.Files.write(tmpPwdPath, pkiservPasswd.getBytes(java.nio.charset.StandardCharsets.UTF_8)); |
| String transportCertPEM = org.mozilla.jss.netscape.security.util.Cert.HEADER + "\n" + | ||
| java.util.Base64.getMimeEncoder(64, "\n".getBytes()).encodeToString(transportCertBytes) + "\n" + | ||
| org.mozilla.jss.netscape.security.util.Cert.FOOTER; | ||
| java.nio.file.Files.write(java.nio.file.Paths.get(transportCertFile), transportCertPEM.getBytes()); |
There was a problem hiding this comment.
Specify an explicit charset (e.g., StandardCharsets.UTF_8) when converting the PEM string to bytes to ensure consistent behavior across different platforms.
| java.nio.file.Files.write(java.nio.file.Paths.get(transportCertFile), transportCertPEM.getBytes()); | |
| java.nio.file.Files.write(java.nio.file.Paths.get(transportCertFile), transportCertPEM.getBytes(java.nio.charset.StandardCharsets.UTF_8)); |
| String storageCertPEM = org.mozilla.jss.netscape.security.util.Cert.HEADER + "\n" + | ||
| java.util.Base64.getMimeEncoder(64, "\n".getBytes()).encodeToString(storageCertBytes) + "\n" + | ||
| org.mozilla.jss.netscape.security.util.Cert.FOOTER; | ||
| java.nio.file.Files.write(java.nio.file.Paths.get(storageCertFile), storageCertPEM.getBytes()); |
There was a problem hiding this comment.
Specify an explicit charset (e.g., StandardCharsets.UTF_8) when converting the PEM string to bytes to ensure consistent behavior across different platforms.
| java.nio.file.Files.write(java.nio.file.Paths.get(storageCertFile), storageCertPEM.getBytes()); | |
| java.nio.file.Files.write(java.nio.file.Paths.get(storageCertFile), storageCertPEM.getBytes(java.nio.charset.StandardCharsets.UTF_8)); |
| PrivateKey unwrappedUserPrivate = CryptoUtil.unwrap( | ||
| hsmTokenObj, | ||
| userPublicKey, | ||
| false, // permanent - try permanent keys for ML-KEM PKCS#12 export |
There was a problem hiding this comment.
Unwrapping the user private key as a permanent object (false) on the HSM may fail if the token is read-only or lacks appropriate permissions, and it leaves persistent objects on the token. Since this is a verification tool, consider using true (temporary) instead, as is done elsewhere in the code (e.g., line 2757). If a permanent key is required for specific operations, it should be explicitly deleted after use.
| PrivateKey unwrappedUserPrivate = CryptoUtil.unwrap( | |
| hsmTokenObj, | |
| userPublicKey, | |
| false, // permanent - try permanent keys for ML-KEM PKCS#12 export | |
| PrivateKey unwrappedUserPrivate = CryptoUtil.unwrap( | |
| hsmTokenObj, | |
| userPublicKey, | |
| true, // temporary |
b6f5ff2 to
bf1a071
Compare
Change encapsulateMLKEM() and decapsulateMLKEM() to accept key size in bits instead of bytes, consistent with PKI convention and other CryptoUtil methods like generateKey(). The methods now convert bits to bytes internally when calling the Java KEM API, making them compatible with params.getSkLength() which returns key size in bits. IDM-6295
Change all CryptoUtil.encapsulateMLKEM() and decapsulateMLKEM() calls to use 256 (bits) instead of 32 (bytes), consistent with the updated CryptoUtil API that accepts key size in bits. IDM-6295
bf1a071 to
7f32f9b
Compare
|




Adds comprehensive ML-KEM (FIPS 203) support to hsmCompatVerify verification tools:
Depends on #5363 (CryptoUtil ML-KEM bits fix)
PQC Mode:
User Key Type Support:
PKCS#12 Modes:
CryptoUtil Integration:
Bug Fixes:
Verification Improvements:
This enables end-to-end verification of ML-KEM key encapsulation for KRA transport and storage certificates, validating the PQC migration path.
IDM-6295, IDM-5815, IDM-5475
Assisted-by: Claude