[ssh] Support ed25519 keys using bouncycastle

[holgerfriedrich]

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: Things that are not working yet:

• keys with password
• hardware keys (sk-...), e.g. on a FIDO stick

This is considered out of scope here.

[utafrali]

The core logic is correct — the DER prefix matches RFC 8410, the wire-format parsing is sound, and comparing encoded bytes across providers is the right strategy. The main gaps are missing diagnostics when getEncoded() returns null, an undocumented size contract on the x509Ed25519 helper, and a silent behavioural mismatch between sk-ssh-ed25519@openssh.com appearing in sigAlgorithms but being unsupported in the login module.

[utafrali]

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:

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());

[holgerfriedrich]

ok, done as requested

[utafrali]

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:

/**
 * 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);
    }
    ...

[holgerfriedrich]

ok. check added.

[utafrali]

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.

[holgerfriedrich]

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.

[utafrali]

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.

[holgerfriedrich]

Thanks, interesting background. See my comment on lift to JDK 21 below.

[utafrali]

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:

// 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.

[holgerfriedrich]

done

[github-actions]

Test Results

  726 files  ±0    726 suites  ±0   1h 17m 12s ⏱️ + 1m 52s
  986 tests +1    938 ✅ +1   48 💤 ±0  0 ❌ ±0 
2 958 runs  +3  2 814 ✅ +3  144 💤 ±0  0 ❌ ±0 

Results for commit b30878a. ± Comparison against base commit a110f75.

♻️ This comment has been updated with latest results.

[holgerfriedrich]

@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.