utils: use CertUtils.generateRandomKeyPair to create SSH keypair#12708
utils: use CertUtils.generateRandomKeyPair to create SSH keypair#12708weizhouapache wants to merge 7 commits intoapache:4.22from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the JSch library with the CertUtils utility for SSH key pair generation in the SSHKeysHelper class. The change modernizes the codebase by consolidating cryptographic operations under the already-used CertUtils class (which uses BouncyCastle) and removes a dependency on the JSch library.
Changes:
- Replaced JSch-based key generation with CertUtils.generateRandomKeyPair in the constructor
- Implemented manual SSH public key format encoding (ssh-rsa format with base64-encoded components)
- Replaced JSch's key serialization methods with CertUtils.privateKeyToPem for private key export
- Removed JSch dependency from both utils/pom.xml and the root pom.xml
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| utils/src/main/java/com/cloud/utils/ssh/SSHKeysHelper.java | Replaces JSch key generation and serialization with CertUtils methods; implements SSH public key encoding manually |
| utils/pom.xml | Removes JSch dependency from the utils module |
| pom.xml | Removes JSch version property and dependency management entry from root POM |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public String getPublicKey() { | ||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
| keyPair.writePublicKey(baos, ""); | ||
| try { | ||
| RSAPublicKey rsaPublicKey = (RSAPublicKey) keyPair.getPublic(); | ||
|
|
||
| ByteArrayOutputStream buffer = new ByteArrayOutputStream(); | ||
|
|
||
| writeString(buffer,"ssh-rsa"); | ||
| writeBigInt(buffer, rsaPublicKey.getPublicExponent()); | ||
| writeBigInt(buffer, rsaPublicKey.getModulus()); | ||
|
|
||
| return baos.toString(); | ||
| String base64 = Base64.encodeBase64String(buffer.toByteArray()); | ||
|
|
||
| return "ssh-rsa " + base64; | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The new implementation of getPublicKey that uses CertUtils.generateRandomKeyPair and manually constructs the SSH public key format is not covered by tests. The existing tests only verify static methods (getPublicKeyFromKeyMaterial and getPublicKeyFingerprint). Consider adding a test that creates an SSHKeysHelper instance, generates a key pair, retrieves the public key, and validates that it can be correctly parsed back and used for encryption (similar to what RSAHelper.encryptWithSSHPublicKey does).
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16945 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15529)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12708 +/- ##
============================================
+ Coverage 17.60% 17.62% +0.01%
- Complexity 15659 15676 +17
============================================
Files 5917 5917
Lines 531394 531429 +35
Branches 64970 64971 +1
============================================
+ Hits 93575 93641 +66
+ Misses 427269 427235 -34
- Partials 10550 10553 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16975 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-15544) |
|
[SF] Trillian Build Failed (tid-15545) |
|
[SF] Trillian test result (tid-15546)
|
|
@weizhouapache is this ready for review? also, check copilot comments if relevant. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This reverts commit 960d2aa445a9adf5c0c71c800c40625359cb4c6e.
|
@blueorangutan package |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
d2809b6 to
a876cfb
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17072 |
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17075 |
Description
This PR replaces the ssk key generation by jsch with CertUtils.generateRandomKeyPair
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?