Copilot commented on code in PR #12708:
URL: https://github.com/apache/cloudstack/pull/12708#discussion_r3217973239
##########
utils/src/main/java/com/cloud/utils/ssh/SSHKeysHelper.java:
##########
@@ -105,17 +110,53 @@ public static String getPublicKeyFromKeyMaterial(String
keyMaterial) {
}
public String getPublicKey() {
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- keyPair.writePublicKey(baos, "");
+ if (keyPair == null || keyPair.getPublic() == null) {
+ return null;
+ }
+ try {
+ RSAPublicKey rsaPublicKey = (RSAPublicKey) keyPair.getPublic();
+
+ ByteArrayOutputStream buffer = new ByteArrayOutputStream();
- return baos.toString();
+ writeString(buffer, "ssh-rsa");
+ writeBigInt(buffer, rsaPublicKey.getPublicExponent());
+ writeBigInt(buffer, rsaPublicKey.getModulus());
+
+ String base64 = Base64.encodeBase64String(buffer.toByteArray());
+
+ return "ssh-rsa " + base64;
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ return null;
}
- public String getPrivateKey() {
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- keyPair.writePrivateKey(baos);
+ private static void writeString(ByteArrayOutputStream out, String str)
throws Exception {
+ byte[] data = str.getBytes(StandardCharsets.UTF_8);
+ out.write(ByteBuffer.allocate(4).putInt(data.length).array());
+ out.write(data);
+ }
+
+ private static void writeBigInt(ByteArrayOutputStream out, BigInteger
value) throws Exception {
+ byte[] data = value.toByteArray();
+ out.write(ByteBuffer.allocate(4).putInt(data.length).array());
+ out.write(data);
+ }
- return baos.toString();
+ public String getPrivateKey() {
+ if (keyPair == null || keyPair.getPrivate() == null) {
+ return null;
+ }
+ try {
+ StringWriter sw = new StringWriter();
+ try (JcaPEMWriter pemWriter = new JcaPEMWriter(sw)) {
+ pemWriter.writeObject(keyPair.getPrivate());
+ }
+ return sw.toString();
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ return null;
}
Review Comment:
Writing a `PrivateKey` via `JcaPEMWriter.writeObject(keyPair.getPrivate())`
commonly emits PKCS#8 (`-----BEGIN PRIVATE KEY-----`) rather than the PKCS#1
`-----BEGIN RSA PRIVATE KEY-----` format that JSch historically produced (and
that this PR’s unit test expects). If consumers rely on the legacy PKCS#1
header/footer, this becomes a breaking behavior change. Consider explicitly
emitting PKCS#1 for RSA keys (or, if the new intended contract is PKCS#8,
update the test + any callers/serialization expectations accordingly).
##########
.pre-commit-config.yaml:
##########
@@ -82,7 +82,8 @@ repos:
^services/console-proxy/rdpconsole/src/test/doc/rdp-key\.pem$|
^systemvm/agent/certs/localhost\.key$|
^systemvm/agent/certs/realhostip\.key$|
- ^test/integration/smoke/test_ssl_offloading\.py$
+ ^test/integration/smoke/test_ssl_offloading\.py$|
+ ^utils/src/test/java/com/cloud/utils/ssh/SSHKeysHelperTest\.java$
Review Comment:
Excluding `SSHKeysHelperTest.java` from this hook reduces the effectiveness
of secret/private-key detection across the repo and can mask accidental commits
in that file. Instead of excluding the entire test file, prefer adjusting the
test data so it doesn’t resemble real key material (or narrow the exclusion to
only the minimal false-positive pattern, if the hook supports it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
