Implement pkcs8 cli #2342
Implement pkcs8 cli #2342
Conversation
Add the pkcs8 command to the AWS-LC OpenSSL-compatible CLI tools. This implementation includes support for the following options: - [-in filename] [-out filename] - [-inform format] [-outform format] - [-topk8] [-nocrypt] - [-v2 alg] [-v2prf alg] - [-passin arg] [-passout arg] The implementation ensures identical output to OpenSSL 1.1.1 and includes appropriate test files. This functionality allows users to convert between traditional key formats and PKCS#8 with options for encryption, format control, and password handling.
This fix addresses three main issues in the PKCS8 CLI tool: 1) Properly handle PRF parameters with AWS-LC's implementation 2) Support all key types by using d2i_AutoPrivateKey instead of RSA-specific function 3) Add more descriptive error messages for better troubleshooting
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2342 +/- ##
==========================================
- Coverage 78.81% 78.74% -0.07%
==========================================
Files 621 623 +2
Lines 108431 109106 +675
Branches 15399 15473 +74
==========================================
+ Hits 85460 85919 +459
- Misses 22300 22518 +218
+ Partials 671 669 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kingstjo
force-pushed
the
implement-pkcs8-cli
branch
from
1d59317 to
fe370f1
Compare
kingstjo
force-pushed
the
implement-pkcs8-cli
branch
from
ab578cf to
fe370f1
Compare
- Updated the implementation to only accept hmacWithSHA1 as a valid PRF algorithm - Added explicit error messages when users specify unsupported PRF algorithms - Updated comments to clearly indicate that only hmacWithSHA1 is supported - Updated tests to use hmacWithSHA1 and added test for unsupported PRF rejection
Added OpenSSLPointer template in internal.h to handle memory allocated with OPENSSL functions. Updated pkcs8.cc to use smart pointers for better resource management and to eliminate manual cleanup. This improves code safety, simplifies error paths, and aligns with patterns used in the rest of the codebase.
When -v2 option is specified without a value in OpenSSL 1.1.1, it defaults to using aes-256-cbc. This commit ensures AWS-LC maintains the same behavior for compatibility. Added tests to verify this behavior.
- Added cross-compatibility tests to verify AWS-LC and OpenSSL can decrypt each other's outputs - Added tests for custom PRF algorithm and default cipher interoperability - Moved helper functions to test_util.h for better code organization - Enhanced test validation to verify decrypted keys match original keys - Maintained backward compatibility with existing tests
…ded prompts for pass similar to OpenSSL
This commit refactors the PKCS8 CLI tool to use the AWS-LC library functions for key operations: - Uses d2i_PKCS8PrivateKey_bio for DER key parsing - Uses PEM_write_bio_PKCS8PrivateKey and i2d_PKCS8PrivateKey_bio for encryption - Maintains all security features: - File size validation to prevent DoS - Password length validation - Cipher and PRF allowlists - Secure memory clearing This approach reduces code complexity, improves security by using well-tested library functions, and ensures better consistency with other AWS-LC tools.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…of creating new variables. Reduce memory usage by accessing cipher and PRF options directly from parsed_args.
The segmentation fault occurred in the PKCS8Test.PKCS8ToolEnvVarPasswordTest due to multiple issues: 1. ReadFileToString was using try/catch when exceptions were disabled in the build 2. Variable shadowing in ReadFileToString causing compiler warnings 3. Environment variable handling differences between local build and CI Changes: - Simplified ReadFileToString to avoid exceptions and variable shadowing - Rewrote DecryptPrivateKey to use BIO directly with better robustness - Modified problematic tests to be more resilient across environments All tests in the PKCS8Test suite now pass successfully.
This change removes the overengineered shared files and consolidates their functionality: - Moved function declarations to test_util.h - Moved function implementations to pkcs8_test.cc - Added required constants directly to pkcs8_test.cc - Removed rsa_pkcs8_shared.* files from the codebase - Updated CMakeLists.txt to remove them from the build
This commit improves code organization by moving functions and templates that are only used by pkcs8_test.cc out of the shared test_util.h file: - Move CreateTestKey(), CheckKeyBoundaries(), DecryptPrivateKey(), and CompareKeys() functions from test_util.h into pkcs8_test.cc - Make these functions static to restrict their scope to the file - Remove their forward declarations from pkcs8_test.cc - Remove TestKeyToolOptionErrors template function from test_util.h and replace its usage with inline implementation in pkcs8_test.cc - Remove now-unnecessary OpenSSL headers from test_util.h - Explicitly specify key size parameters rather than using defaults These changes improve code organization by keeping utilities with their sole consumer and reduce header dependencies in test_util.h.
There was a problem hiding this comment.
Awesome tests and the code is much more concise with only what's needed! There are still some examples of "over-documentation" and a few nits here and there. Preferably would like the comments and additional test debugging error messages cut down a bit before merging, but I don't see any major blockers.
| err: | ||
| secure_clear(passin_arg); | ||
| secure_clear(passout_arg); |
There was a problem hiding this comment.
If we wanted to, we could wrap a class or UniquePtr around passin_arg & passout_arg and assign secure_clear as the destructor. That way we could just return false instead using the goto err statements.
Not a blocker though.
There was a problem hiding this comment.
Hmmm didn't realize you can assign custom destructors to bssl::UniqPtrs that would make this even cleaner!
There was a problem hiding this comment.
In general, ASSERTs are for logic that is required for the test to properly proceed. This would include pointer or malloc assertions for example. We can use EXPECT_TRUE/FALSE to do testing logic that won't block later operations. But this isn't a blocker, just something nice to keep in mind.
| { | ||
| args_list_t args = {"-in", in_path, "-out", out_path, "-topk8", "-nocrypt"}; | ||
| bool result = pkcs8Tool(args); | ||
| ASSERT_TRUE(result) << "Failed to create unencrypted PKCS8 file"; |
There was a problem hiding this comment.
The debugging messages here and throughout the test are nice, but I feel like much of them are redundant. It makes the test code a bit harder to read through, but that might just be my own preference. The code should be self explanatory enough to indicate what's failed.
There was a problem hiding this comment.
I think +1 here since the code would be printing out reason for failure anyway right (before erroring out)?
| std::ifstream file_stream(file_path, std::ios::binary); | ||
| if (!file_stream) { | ||
| if (!file_stream.is_open()) { |
There was a problem hiding this comment.
do we need to still check if file_stream is null before calling is_open() on it?
Issues:
Addresses CryptoAlg-3073
Addresses CryptoAlg-3074
Addresses CryptoAlg-3075
Description of changes:
This PR implements the PKCS8 CLI command wrapper for AWS-LC, providing OpenSSL-compatible command-line interface to the existing PKCS8 functionality. The CLI tool adds support for:
Call-outs:
PRF Parameter Compatibility: The implementation accepts the
-v2prfparameter for OpenSSL command-line compatibility and validates supported values (hmacWithSHA1, hmacWithSHA256, hmacWithSHA512). However, AWS-LC's underlying PKCS#8 implementation always uses its default PRF implementation (-1) in the PKCS8_encrypt function, regardless of the specified-v2prfvalue.Comprehensive Key Type Support: The CLI implementation uses d2i_AutoPrivateKey to provide support for all private key types that AWS-LC can handle. This allows users to process various key formats through the pkcs8 command without type restrictions.
User-Friendly Error Handling: The implementation provides context-rich error messages that guide users through troubleshooting common issues including:
Testing:
The CLI wrapper is validated through two testing approaches:
Unit tests (
PKCS8Testclass) verify:PKCS8ToolBasicTest)-informand-outformoptions (PKCS8ToolFormatTest)-v2 aes-256-cbcoptions (PKCS8ToolEncryptionTest)-v2prf(PKCS8ToolPRFTest)PKCS8OptionUsageErrorsTest)Comparison tests (
PKCS8ComparisonTestclass) directly compare AWS-LC's output with OpenSSL's:PKCS8ToolCompareUnencryptedOpenSSL)PKCS8ToolCompareEncryptedOpenSSL)PKCS8ToolCompareDERFormatOpenSSL)PKCS8ToolCompareV2prfOpenSSL)These tests ensure that the AWS-LC CLI tool produces output files with the correct PKCS#8 boundaries and format compatibility with OpenSSL's pkcs8 command.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.