This is an automated email from the ASF dual-hosted git repository. twolf pushed a commit to branch dev_3.0 in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit f5892b5e2b2c2fbeef1995fc4fc05ea694a772c0 Author: Thomas Wolf <[email protected]> AuthorDate: Sat Sep 13 13:04:35 2025 +0200 Fix default order of SecurityProviderRegistrars If net.i2p is present, we want it to take precedence over the Bouncy Castle ed25519 implementation. Therefore, its registrar must come first. Remove the extra check in the Bouncy Castle registrar about the net.i2p registrar being registered. Which one is chosen shall depend solely on the order of registrars. Fix the OpenSSHKeyPairResourceWriterTest to work with any ed25519 implementation. Change key comparison methods to compare ed25519 from different providers. --- .../apache/sshd/common/config/keys/KeyUtils.java | 28 +++++------ .../sshd/common/util/security/SecurityUtils.java | 9 +--- .../BouncyCastleSecurityProviderRegistrar.java | 4 +- .../openssh/OpenSSHKeyPairResourceWriterTest.java | 55 ++++++---------------- 4 files changed, 30 insertions(+), 66 deletions(-) diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java index ab6f70273..3796e9f4f 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java @@ -737,8 +737,7 @@ public final class KeyUtils { } else { return curve.getKeyType(); } - } else if (SecurityUtils.EDDSA.equalsIgnoreCase(key.getAlgorithm()) - || SecurityUtils.ED25519.equalsIgnoreCase(key.getAlgorithm())) { + } else if (isEd25519(key)) { return KeyPairProvider.SSH_ED25519; } @@ -808,8 +807,7 @@ public final class KeyUtils { if (curve != null) { return curve.getKeySize(); } - } else if (SecurityUtils.EDDSA.equalsIgnoreCase(key.getAlgorithm()) - || SecurityUtils.ED25519.equalsIgnoreCase(key.getAlgorithm())) { + } else if (isEd25519(key)) { return 256; } @@ -857,6 +855,13 @@ public final class KeyUtils { return compareKeys(k1.getPublic(), k2.getPublic()) && compareKeys(k1.getPrivate(), k2.getPrivate()); } + private static boolean isEd25519(Key k) { + return k != null + && (SecurityUtils.ED25519.equals(k.getAlgorithm()) // + || (SecurityUtils.EDDSA.equalsIgnoreCase(k.getAlgorithm()) + && k.getClass().getCanonicalName().startsWith("net.i2p."))); + } + public static boolean compareKeys(PublicKey k1, PublicKey k2) { if (Objects.equals(k1, k2)) { return true; @@ -869,11 +874,7 @@ public final class KeyUtils { return compareECKeys(ECPublicKey.class.cast(k1), ECPublicKey.class.cast(k2)); } else if ((k1 instanceof SkEcdsaPublicKey) && (k2 instanceof SkEcdsaPublicKey)) { return compareSkEcdsaKeys(SkEcdsaPublicKey.class.cast(k1), SkEcdsaPublicKey.class.cast(k2)); - } else if ((k1 != null) && SecurityUtils.EDDSA.equalsIgnoreCase(k1.getAlgorithm()) - && (k2 != null) && SecurityUtils.EDDSA.equalsIgnoreCase(k2.getAlgorithm())) { - return SecurityUtils.compareEDDSAPPublicKeys(k1, k2); - } else if ((k1 != null) && SecurityUtils.ED25519.equalsIgnoreCase(k1.getAlgorithm()) - && (k2 != null) && SecurityUtils.ED25519.equalsIgnoreCase(k2.getAlgorithm())) { + } else if (isEd25519(k1) && isEd25519(k2)) { return SecurityUtils.compareEDDSAPPublicKeys(k1, k2); } else if ((k1 instanceof SkED25519PublicKey) && (k2 instanceof SkED25519PublicKey)) { return compareSkEd25519Keys(SkED25519PublicKey.class.cast(k1), SkED25519PublicKey.class.cast(k2)); @@ -888,8 +889,7 @@ public final class KeyUtils { return recoverRSAPublicKey((RSAPrivateKey) key); } else if (key instanceof DSAPrivateKey) { return recoverDSAPublicKey((DSAPrivateKey) key); - } else if ((key != null) && (SecurityUtils.EDDSA.equalsIgnoreCase(key.getAlgorithm()) - || SecurityUtils.ED25519.equalsIgnoreCase(key.getAlgorithm()))) { + } else if (isEd25519(key)) { return SecurityUtils.recoverEDDSAPublicKey(key); } return null; @@ -902,11 +902,7 @@ public final class KeyUtils { return compareDSAKeys(DSAPrivateKey.class.cast(k1), DSAPrivateKey.class.cast(k2)); } else if ((k1 instanceof ECPrivateKey) && (k2 instanceof ECPrivateKey)) { return compareECKeys(ECPrivateKey.class.cast(k1), ECPrivateKey.class.cast(k2)); - } else if ((k1 != null) && SecurityUtils.EDDSA.equalsIgnoreCase(k1.getAlgorithm()) - && (k2 != null) && SecurityUtils.EDDSA.equalsIgnoreCase(k2.getAlgorithm())) { - return SecurityUtils.compareEDDSAPrivateKeys(k1, k2); - } else if ((k1 != null) && SecurityUtils.ED25519.equalsIgnoreCase(k1.getAlgorithm()) && (k2 != null) - && SecurityUtils.ED25519.equalsIgnoreCase(k2.getAlgorithm())) { + } else if (isEd25519(k1) && isEd25519(k2)) { return SecurityUtils.compareEDDSAPrivateKeys(k1, k2); } return false; // either key is null or not of same class diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/SecurityUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/SecurityUtils.java index 609cd0b0d..891a0e012 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/SecurityUtils.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/SecurityUtils.java @@ -134,8 +134,8 @@ public final class SecurityUtils { public static final List<String> DEFAULT_SECURITY_PROVIDER_REGISTRARS = Collections.unmodifiableList( Arrays.asList( "org.apache.sshd.common.util.security.SunJCESecurityProviderRegistrar", - "org.apache.sshd.common.util.security.bouncycastle.BouncyCastleSecurityProviderRegistrar", - "org.apache.sshd.common.util.security.eddsa.EdDSASecurityProviderRegistrar")); + "org.apache.sshd.common.util.security.eddsa.EdDSASecurityProviderRegistrar", + "org.apache.sshd.common.util.security.bouncycastle.BouncyCastleSecurityProviderRegistrar")); public static final String PROP_DEFAULT_SECURITY_PROVIDER = "org.apache.sshd.security.defaultProvider"; @@ -578,11 +578,6 @@ public final class SecurityUtils { return isSupported != null && isSupported.booleanValue(); } - public static boolean isNetI2pCryptoEdDSARegistered() { - register(); - return isProviderRegistered(EDDSA); - } - /* -------------------------------------------------------------------- */ public static PrivateKeyEntryDecoder getOpenSSHEDDSAPrivateKeyEntryDecoder() { diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleSecurityProviderRegistrar.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleSecurityProviderRegistrar.java index 8c886e83f..25e5474b6 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleSecurityProviderRegistrar.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleSecurityProviderRegistrar.java @@ -105,11 +105,11 @@ public class BouncyCastleSecurityProviderRegistrar extends AbstractSecurityProvi if (KeyPairGenerator.class.isAssignableFrom(entityType) || KeyFactory.class.isAssignableFrom(entityType)) { if (SecurityUtils.ED25519.equalsIgnoreCase(name)) { - return !SecurityUtils.isNetI2pCryptoEdDSARegistered() && isEdDSASupported(); + return isEdDSASupported(); } } else if (Signature.class.isAssignableFrom(entityType)) { if (SecurityUtils.ED25519.equalsIgnoreCase(name)) { - return !SecurityUtils.isNetI2pCryptoEdDSARegistered() && isEdDSASupported(); + return isEdDSASupported(); } } diff --git a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java index a9f28c8a8..514f09f51 100644 --- a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java +++ b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java @@ -33,14 +33,11 @@ import java.security.GeneralSecurityException; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.PublicKey; -import java.security.spec.AlgorithmParameterSpec; -import java.security.spec.ECGenParameterSpec; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; -import net.i2p.crypto.eddsa.spec.EdDSAGenParameterSpec; import org.apache.sshd.common.config.keys.AuthorizedKeyEntry; import org.apache.sshd.common.config.keys.FilePasswordProvider; import org.apache.sshd.common.config.keys.KeyUtils; @@ -61,36 +58,22 @@ class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport { static Collection<TestData> parameters() { List<TestData> result = new ArrayList<>(); - result.add(new TestData("RSA", 1024, null)); - result.add(new TestData("RSA", 2048, null)); - result.add(new TestData("DSA", 1024, null)); - result.add(new TestData("ECDSA", 256, new ECGenParameterSpec("secp256r1"))); - result.add(new TestData("ECDSA", 384, new ECGenParameterSpec("secp384r1"))); - result.add(new TestData("ECDSA", 521, new ECGenParameterSpec("secp521r1"))); + result.add(new TestData("RSA", 1024)); + result.add(new TestData("RSA", 2048)); + result.add(new TestData("DSA", 1024)); + result.add(new TestData("ECDSA", 256)); + result.add(new TestData("ECDSA", 384)); + result.add(new TestData("ECDSA", 521)); if (SecurityUtils.isEDDSACurveSupported()) { - // Note: BC also has an EDDSA provider, but that one returns - // "Ed25519" as algorithm from its keys, while the one in - // net.i2p.crypto.eddsa gives keys with "EDDSA" as algorithm. - // sshd handles only the latter. - result.add(new TestData( - "EDDSA", "EdDSA", 25519, - new EdDSAGenParameterSpec("Ed25519"))); + result.add(new TestData(SecurityUtils.ED25519, -1)); } return result; } private KeyPair generateKey(TestData data) throws Exception { - KeyPairGenerator generator; - if (data.provider == null) { - generator = KeyPairGenerator.getInstance(data.algorithm); - } else { - generator = KeyPairGenerator.getInstance(data.algorithm, data.provider); - } - - if (data.spec != null) { - generator.initialize(data.spec); - } else { + KeyPairGenerator generator = SecurityUtils.getKeyPairGenerator(data.algorithm); + if (data.keySize > 0) { generator.initialize(data.keySize); } return generator.generateKeyPair(); @@ -450,29 +433,19 @@ class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport { final String algorithm; - final String provider; - final int keySize; - final AlgorithmParameterSpec spec; - - TestData(String algorithm, int keySize, - AlgorithmParameterSpec spec) { - this(algorithm, null, keySize, spec); - } - - TestData(String algorithm, String provider, int keySize, - AlgorithmParameterSpec spec) { + TestData(String algorithm, int keySize) { this.algorithm = algorithm; - this.provider = provider; this.keySize = keySize; - this.spec = spec; } @Override public String toString() { - return algorithm + '-' + keySize - + (provider == null ? "" : '(' + provider + ')'); + if (keySize > 0) { + return algorithm + '-' + keySize; + } + return algorithm; } } }
