You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by "holgerfriedrich (via GitHub)" <gi...@apache.org> on 2026/09/20 12:35:14 UTC
[PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
holgerfriedrich opened a new pull request, #2927:
URL: https://github.com/apache/karaf/pull/2927
Enable ssh-ed25519 in the default signature algorithms, match ed25519 keys in the publickey login module, and add the bouncycastle jars from the distribution to the bin/client classpath.
Fixes #2925.
Note: One thing is now working yet: keys with password. This is out of scope here.
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #2927:
URL: https://github.com/apache/karaf/pull/2927
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "holgerfriedrich (via GitHub)" <gi...@apache.org>.
holgerfriedrich commented on PR #2927:
URL: https://github.com/apache/karaf/pull/2927#issuecomment-5750522402
@utafrali Thanks for the quick review and for the new insights. I will have a look.
I think we can disregard the older Java versions - at least for the main branch. We aligned to lift the minimum Java version for Karaf 4.5.0 to 21. The PR is already in work, see #2214. Still relevant if we would think about backporting to 4.4.x, though.
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "holgerfriedrich (via GitHub)" <gi...@apache.org>.
holgerfriedrich commented on code in PR #2927:
URL: https://github.com/apache/karaf/pull/2927#discussion_r4057279245
##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/publickey/PublickeyLoginModule.java:
##########
@@ -239,6 +247,23 @@ public static boolean equals(PublicKey key, String storedKey) throws FailedLogin
PublicKey generatedPublicKey = keyFactory.generatePublic(keySpec);
return key.equals(generatedPublicKey);
+ } else if (ED25519_IDENTIFIER.equals(identifier)) {
+ // OpenSSH stores an ed25519 key as the raw 32 bytes of the compressed point.
+ // The key implementation depends on the registered provider (for instance
+ // BouncyCastle), so compare the X.509 encodings instead of the key objects.
+ int size = dis.readInt();
+ if (size != ED25519_KEY_LENGTH) {
+ return false;
+ }
+ byte[] bytes = new byte[size];
+ dis.readFully(bytes);
+
+ KeyFactory keyFactory = KeyFactory.getInstance("Ed25519");
Review Comment:
Thanks, interesting background. See my comment on lift to JDK 21 below.
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "holgerfriedrich (via GitHub)" <gi...@apache.org>.
holgerfriedrich commented on code in PR #2927:
URL: https://github.com/apache/karaf/pull/2927#discussion_r4057230318
##########
jaas/modules/src/test/java/org/apache/karaf/jaas/modules/publickey/PublicKeyEncodingTest.java:
##########
@@ -209,4 +211,22 @@ public void testEC256_2() throws FailedLoginException, NoSuchAlgorithmException,
assertFalse(PublickeyLoginModule.equals(publicKey, differentKey));
}
Review Comment:
done
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2927:
URL: https://github.com/apache/karaf/pull/2927#issuecomment-5750099999
## Test Results
726 files ±0 726 suites ±0 1h 16m 26s ⏱️ + 1m 6s
986 tests +1 938 ✅ +1 48 💤 ±0 0 ❌ ±0
2 958 runs +3 2 814 ✅ +3 144 💤 ±0 0 ❌ ±0
Results for commit 2d25b3bc. ± Comparison against base commit a110f751.
[test-results]:data:application/gzip;base64,H4sIAIHfr2oC/03MTQ6DIBCG4asY1l3gOCj2Mg0OkJD60yCsTO9eaK2wfJ/JfAezbjY7uzcD9LeG7dGFKnX0KrhtTYBCZknXkO9jqcceiTJ1stDTvfJTEavcnIRfYLzf/Ck+rnkURiHP+o+CbPGi32iLl1Sj3643aVsWF1Iw0CCmbiIiBTCalgY+gO6Qo+2BIxgrNPTI2fsDYOTgww8BAAA=
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "holgerfriedrich (via GitHub)" <gi...@apache.org>.
holgerfriedrich commented on code in PR #2927:
URL: https://github.com/apache/karaf/pull/2927#discussion_r4057200066
##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/publickey/PublickeyLoginModule.java:
##########
@@ -239,6 +247,23 @@ public static boolean equals(PublicKey key, String storedKey) throws FailedLogin
PublicKey generatedPublicKey = keyFactory.generatePublic(keySpec);
return key.equals(generatedPublicKey);
+ } else if (ED25519_IDENTIFIER.equals(identifier)) {
+ // OpenSSH stores an ed25519 key as the raw 32 bytes of the compressed point.
+ // The key implementation depends on the registered provider (for instance
+ // BouncyCastle), so compare the X.509 encodings instead of the key objects.
+ int size = dis.readInt();
+ if (size != ED25519_KEY_LENGTH) {
+ return false;
+ }
+ byte[] bytes = new byte[size];
+ dis.readFully(bytes);
+
+ KeyFactory keyFactory = KeyFactory.getInstance("Ed25519");
+ KeySpec publicKeySpec = new X509EncodedKeySpec(x509Ed25519(bytes));
+ PublicKey generatedPublicKey = keyFactory.generatePublic(publicKeySpec);
+
+ byte[] encoded = key.getEncoded();
+ return encoded != null && Arrays.equals(encoded, generatedPublicKey.getEncoded());
Review Comment:
ok, done as requested
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "holgerfriedrich (via GitHub)" <gi...@apache.org>.
holgerfriedrich commented on code in PR #2927:
URL: https://github.com/apache/karaf/pull/2927#discussion_r4057274005
##########
shell/ssh/src/main/java/org/apache/karaf/shell/ssh/Activator.java:
##########
@@ -169,7 +169,7 @@ protected SshServer createSshServer(SessionFactory sessionFactory) {
String[] macs = getStringArray("macs", "hmac-sha2-512,hmac-sha2-256");
String[] ciphers = getStringArray("ciphers", "aes256-ctr,aes192-ctr,aes128-ctr");
String[] kexAlgorithms = getStringArray("kexAlgorithms", "ecdh-sha2-nistp521,ecdh-sha2-nistp384,ecdh-sha2-nistp256,diffie-hellman-group-exchange-sha256");
- String[] sigAlgorithms = getStringArray("sigAlgorithms", "ssh-rsa,rsa-sha2-256,rsa-sha2-512,sk-ecdsa-sha2-nistp256@openssh.com,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521");
+ String[] sigAlgorithms = getStringArray("sigAlgorithms", "ssh-rsa,rsa-sha2-256,rsa-sha2-512,sk-ecdsa-sha2-nistp256@openssh.com,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-ed25519,sk-ssh-ed25519@openssh.com");
Review Comment:
This basically mirrors the behavior before: `sk-ecdsa-sha2-nistp256@openssh.com` is there as well. It it also accepted by the server but not yet supported in PublicKeyLoginModule. This is pretty much a different PR in my view.
So lets go with the comment for now.
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "utafrali (via GitHub)" <gi...@apache.org>.
utafrali commented on code in PR #2927:
URL: https://github.com/apache/karaf/pull/2927#discussion_r4056954733
##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/publickey/PublickeyLoginModule.java:
##########
@@ -239,6 +247,23 @@ public static boolean equals(PublicKey key, String storedKey) throws FailedLogin
PublicKey generatedPublicKey = keyFactory.generatePublic(keySpec);
return key.equals(generatedPublicKey);
+ } else if (ED25519_IDENTIFIER.equals(identifier)) {
+ // OpenSSH stores an ed25519 key as the raw 32 bytes of the compressed point.
+ // The key implementation depends on the registered provider (for instance
+ // BouncyCastle), so compare the X.509 encodings instead of the key objects.
+ int size = dis.readInt();
+ if (size != ED25519_KEY_LENGTH) {
+ return false;
+ }
+ byte[] bytes = new byte[size];
+ dis.readFully(bytes);
+
+ KeyFactory keyFactory = KeyFactory.getInstance("Ed25519");
+ KeySpec publicKeySpec = new X509EncodedKeySpec(x509Ed25519(bytes));
+ PublicKey generatedPublicKey = keyFactory.generatePublic(publicKeySpec);
+
+ byte[] encoded = key.getEncoded();
+ return encoded != null && Arrays.equals(encoded, generatedPublicKey.getEncoded());
Review Comment:
`key.getEncoded()` returning `null` silently causes authentication to fail with no diagnostic information. The SSH client will just see a rejected key. Add a debug-level log before returning false:
```java
byte[] encoded = key.getEncoded();
if (encoded == null) {
LOG.debug("Ed25519 key returned null encoding — provider may not support getEncoded()");
return false;
}
return Arrays.equals(encoded, generatedPublicKey.getEncoded());
```
##########
shell/ssh/src/main/java/org/apache/karaf/shell/ssh/Activator.java:
##########
@@ -169,7 +169,7 @@ protected SshServer createSshServer(SessionFactory sessionFactory) {
String[] macs = getStringArray("macs", "hmac-sha2-512,hmac-sha2-256");
String[] ciphers = getStringArray("ciphers", "aes256-ctr,aes192-ctr,aes128-ctr");
String[] kexAlgorithms = getStringArray("kexAlgorithms", "ecdh-sha2-nistp521,ecdh-sha2-nistp384,ecdh-sha2-nistp256,diffie-hellman-group-exchange-sha256");
- String[] sigAlgorithms = getStringArray("sigAlgorithms", "ssh-rsa,rsa-sha2-256,rsa-sha2-512,sk-ecdsa-sha2-nistp256@openssh.com,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521");
+ String[] sigAlgorithms = getStringArray("sigAlgorithms", "ssh-rsa,rsa-sha2-256,rsa-sha2-512,sk-ecdsa-sha2-nistp256@openssh.com,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-ed25519,sk-ssh-ed25519@openssh.com");
Review Comment:
`sk-ssh-ed25519@openssh.com` is added to `sigAlgorithms` here (and in the config file comment), but `PublickeyLoginModule.equals()` only handles `"ssh-ed25519"`. A client authenticating with a FIDO2 hardware ed25519 key will hit the `throw new FailedLoginException("Unsupported key type...")` branch. This is consistent with how the existing `sk-ecdsa-sha2-nistp256@openssh.com` entry is handled, but it is worth a comment next to that value noting that hardware-backed sk keys are not yet supported in the login module.
##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/publickey/PublickeyLoginModule.java:
##########
@@ -239,6 +247,23 @@ public static boolean equals(PublicKey key, String storedKey) throws FailedLogin
PublicKey generatedPublicKey = keyFactory.generatePublic(keySpec);
return key.equals(generatedPublicKey);
+ } else if (ED25519_IDENTIFIER.equals(identifier)) {
+ // OpenSSH stores an ed25519 key as the raw 32 bytes of the compressed point.
+ // The key implementation depends on the registered provider (for instance
+ // BouncyCastle), so compare the X.509 encodings instead of the key objects.
+ int size = dis.readInt();
+ if (size != ED25519_KEY_LENGTH) {
+ return false;
+ }
+ byte[] bytes = new byte[size];
+ dis.readFully(bytes);
+
+ KeyFactory keyFactory = KeyFactory.getInstance("Ed25519");
Review Comment:
`KeyFactory.getInstance("Ed25519")` relies on a JCA provider that supports Ed25519. Native JDK support only arrived in JDK 15; Karaf supports JDK 11+. On JDK 11-14, this call succeeds only if BouncyCastle is already registered in the JVM security provider list, which MINA SSHD does on startup via `BouncyCastleSecurityProviderRegistrar`. That sequencing dependency is invisible here. The caught `GeneralSecurityException` will surface it as a `FailedLoginException`, so it fails safely — but the message will be cryptic. A comment noting the BC dependency (and the JDK 15 floor for BC-free operation) would help future maintainers.
##########
jaas/modules/src/test/java/org/apache/karaf/jaas/modules/publickey/PublicKeyEncodingTest.java:
##########
@@ -209,4 +211,22 @@ public void testEC256_2() throws FailedLoginException, NoSuchAlgorithmException,
assertFalse(PublickeyLoginModule.equals(publicKey, differentKey));
}
Review Comment:
The test covers the happy path and a different-key rejection, which is good. It is missing a test for the malformed-size path: `size != ED25519_KEY_LENGTH`. Something like:
```java
// A base64 blob whose wire-format size field claims 31 bytes instead of 32
// should be rejected cleanly rather than throwing an exception
String badSizeKey = "...";
assertFalse(PublickeyLoginModule.equals(publicKey, badSizeKey));
```
This is low risk given the guard is simple, but it would pin the behaviour.
##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/publickey/PublickeyLoginModule.java:
##########
@@ -247,6 +272,17 @@ public static boolean equals(PublicKey key, String storedKey) throws FailedLogin
}
}
+ /**
+ * Wraps the raw bytes of an ed25519 public key in a X.509 SubjectPublicKeyInfo structure,
+ * so that it can be read by a {@link KeyFactory}.
+ */
Review Comment:
`x509Ed25519` has an implicit contract that `rawKey` must be exactly 32 bytes — the `0x2a` length byte in `ED25519_X509_PREFIX` encodes exactly 42 (10 prefix bytes + 32 key bytes). If this method is ever called with a different size, it will silently produce malformed DER. The caller already guards this, but the method itself should either validate or document the contract:
```java
/**
* Wraps the raw bytes of an ed25519 public key (must be exactly {@value ED25519_KEY_LENGTH} bytes)
* in a X.509 SubjectPublicKeyInfo structure so that it can be read by a {@link KeyFactory}.
*/
private static byte[] x509Ed25519(byte[] rawKey) {
if (rawKey.length != ED25519_KEY_LENGTH) {
throw new IllegalArgumentException("Ed25519 raw key must be " + ED25519_KEY_LENGTH + " bytes, got " + rawKey.length);
}
...
```
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ssh] Support ed25519 keys using bouncycastle [karaf]
Posted by "holgerfriedrich (via GitHub)" <gi...@apache.org>.
holgerfriedrich commented on code in PR #2927:
URL: https://github.com/apache/karaf/pull/2927#discussion_r4057205313
##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/publickey/PublickeyLoginModule.java:
##########
@@ -247,6 +272,17 @@ public static boolean equals(PublicKey key, String storedKey) throws FailedLogin
}
}
+ /**
+ * Wraps the raw bytes of an ed25519 public key in a X.509 SubjectPublicKeyInfo structure,
+ * so that it can be read by a {@link KeyFactory}.
+ */
Review Comment:
ok. check added.
--
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: commits-unsubscribe@karaf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org