Add DefaultSecurityProviderConfig with Bouncy Castle disabled (#861)

* Added DefaultSecurityProviderConfig with Bouncy Castle disabled

* Upgrade test to junit jupiter

---------

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>

src/main/java/net/schmizz/sshj/AndroidConfig.java

@@ -19,8 +19,6 @@ import com.hierynomus.sshj.key.KeyAlgorithm;
 import com.hierynomus.sshj.key.KeyAlgorithms;
 import net.schmizz.sshj.common.Factory;
 import net.schmizz.sshj.common.SecurityUtils;
-import net.schmizz.sshj.transport.random.JCERandom;
-import net.schmizz.sshj.transport.random.SingletonRandomFactory;
 
 import java.util.Arrays;
 
@@ -34,7 +32,6 @@ public class AndroidConfig
         SecurityUtils.registerSecurityProvider("org.spongycastle.jce.provider.BouncyCastleProvider");
     }
 
-
     @Override
     protected void initKeyAlgorithms() {
         setKeyAlgorithms(Arrays.<Factory.Named<KeyAlgorithm>>asList(
@@ -43,10 +40,4 @@ public class AndroidConfig
                 KeyAlgorithms.SSHDSA()
         ));
     }
-
-    @Override
-    protected void initRandomFactory(boolean ignored) {
-        setRandomFactory(new SingletonRandomFactory(new JCERandom.Factory()));
-    }
-
 }

src/main/java/net/schmizz/sshj/DefaultConfig.java

@@ -29,7 +29,6 @@ import com.hierynomus.sshj.userauth.keyprovider.OpenSSHKeyV1KeyFile;
 import net.schmizz.keepalive.KeepAliveProvider;
 import net.schmizz.sshj.common.Factory;
 import net.schmizz.sshj.common.LoggerFactory;
-import net.schmizz.sshj.common.SecurityUtils;
 import net.schmizz.sshj.transport.cipher.Cipher;
 import net.schmizz.sshj.transport.compression.NoneCompression;
 import net.schmizz.sshj.transport.kex.Curve25519SHA256;
@@ -43,7 +42,11 @@ import net.schmizz.sshj.userauth.keyprovider.PKCS8KeyFile;
 import net.schmizz.sshj.userauth.keyprovider.PuTTYKeyFile;
 import org.slf4j.Logger;
 
-import java.util.*;
+import java.util.Arrays;
+import java.util.List;
+import java.util.LinkedList;
+import java.util.ListIterator;
+import java.util.Properties;
 
 /**
  * A {@link net.schmizz.sshj.Config} that is initialized as follows. Items marked with an asterisk are added to the config only if
@@ -62,8 +65,6 @@ import java.util.*;
  * <li>{@link net.schmizz.sshj.ConfigImpl#setVersion Client version}: {@code "NET_3_0"}</li>
  * </ul>
  * <p/>
- * [1] It is worth noting that Sun's JRE does not have the unlimited cryptography extension enabled by default. This
- * prevents using ciphers with strength greater than 128.
  */
 public class DefaultConfig
         extends ConfigImpl {
@@ -73,11 +74,10 @@ public class DefaultConfig
     public DefaultConfig() {
         setLoggerFactory(LoggerFactory.DEFAULT);
         setVersion(readVersionFromProperties());
-        final boolean bouncyCastleRegistered = SecurityUtils.isBouncyCastleRegistered();
+        initKeyExchangeFactories();
-        initKeyExchangeFactories(bouncyCastleRegistered);
         initKeyAlgorithms();
-        initRandomFactory(bouncyCastleRegistered);
+        initRandomFactory();
-        initFileKeyProviderFactories(bouncyCastleRegistered);
+        initFileKeyProviderFactories();
         initCipherFactories();
         initCompressionFactories();
         initMACFactories();
@@ -102,8 +102,7 @@ public class DefaultConfig
         log = loggerFactory.getLogger(getClass());
     }
 
-    protected void initKeyExchangeFactories(boolean bouncyCastleRegistered) {
+    protected void initKeyExchangeFactories() {
-        if (bouncyCastleRegistered) {
         setKeyExchangeFactories(
                 new Curve25519SHA256.Factory(),
                 new Curve25519SHA256.FactoryLibSsh(),
@@ -127,10 +126,8 @@ public class DefaultConfig
                 ExtendedDHGroups.Group16SHA384AtSSH(),
                 ExtendedDHGroups.Group16SHA512AtSSH(),
                 ExtendedDHGroups.Group18SHA512AtSSH(),
-                    new ExtInfoClientFactory());
+                new ExtInfoClientFactory()
-        } else {
+        );
-            setKeyExchangeFactories(DHGroups.Group1SHA1(), new DHGexSHA1.Factory());
-        }
     }
 
     protected void initKeyAlgorithms() {
@@ -151,20 +148,18 @@ public class DefaultConfig
                 KeyAlgorithms.SSHDSA()));
     }
 
-    protected void initRandomFactory(boolean bouncyCastleRegistered) {
+    protected void initRandomFactory() {
         setRandomFactory(new SingletonRandomFactory(new JCERandom.Factory()));
     }
 
-    protected void initFileKeyProviderFactories(boolean bouncyCastleRegistered) {
+    protected void initFileKeyProviderFactories() {
-        if (bouncyCastleRegistered) {
         setFileKeyProviderFactories(
                 new OpenSSHKeyV1KeyFile.Factory(),
                 new PKCS8KeyFile.Factory(),
                 new OpenSSHKeyFile.Factory(),
-                    new PuTTYKeyFile.Factory());
+                new PuTTYKeyFile.Factory()
+        );
     }
-    }
-
 
     protected void initCipherFactories() {
         List<Factory.Named<Cipher>> avail = new LinkedList<Factory.Named<Cipher>>(Arrays.<Factory.Named<Cipher>>asList(
@@ -203,27 +198,22 @@ public class DefaultConfig
                 StreamCiphers.Arcfour256())
         );
 
-        boolean warn = false;
+        final ListIterator<Factory.Named<Cipher>> factories = avail.listIterator();
-        // Ref. https://issues.apache.org/jira/browse/SSHD-24
+        while (factories.hasNext()) {
-        // "AES256 and AES192 requires unlimited cryptography extension"
+            final Factory.Named<Cipher> factory = factories.next();
-        for (Iterator<Factory.Named<Cipher>> i = avail.iterator(); i.hasNext(); ) {
-            final Factory.Named<Cipher> f = i.next();
             try {
-                final Cipher c = f.create();
+                final Cipher cipher = factory.create();
-                final byte[] key = new byte[c.getBlockSize()];
+                final byte[] key = new byte[cipher.getBlockSize()];
-                final byte[] iv = new byte[c.getIVSize()];
+                final byte[] iv = new byte[cipher.getIVSize()];
-                c.init(Cipher.Mode.Encrypt, key, iv);
+                cipher.init(Cipher.Mode.Encrypt, key, iv);
             } catch (Exception e) {
-                warn = true;
+                log.info("Cipher [{}] disabled: {}", factory.getName(), e.getCause().getMessage());
-                log.warn(e.getCause().getMessage());
+                factories.remove();
-                i.remove();
             }
         }
-        if (warn)
-            log.warn("Disabling high-strength ciphers: cipher strengths apparently limited by JCE policy");
 
         setCipherFactories(avail);
-        log.debug("Available cipher factories: {}", avail);
+        log.debug("Available Ciphers {}", avail);
     }
 
     protected void initMACFactories() {

src/main/java/net/schmizz/sshj/DefaultSecurityProviderConfig.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package net.schmizz.sshj;
+
+import net.schmizz.sshj.common.SecurityUtils;
+
+/**
+ * SSHJ Configuration that uses the default Security Provider configuration from java.security and disables Bouncy Castle registration
+ */
+public class DefaultSecurityProviderConfig extends DefaultConfig {
+    static {
+        // Disable Bouncy Castle Provider registration prior to invoking constructors
+        SecurityUtils.setRegisterBouncyCastle(false);
+    }
+}

src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java

@@ -57,9 +57,6 @@ class ECDSAVariationsAdapter {
 
     static PublicKey readPubKeyFromBuffer(Buffer<?> buf, String variation) throws GeneralSecurityException {
         String algorithm = BASE_ALGORITHM_NAME + variation;
-        if (!SecurityUtils.isBouncyCastleRegistered()) {
-            throw new GeneralSecurityException("BouncyCastle is required to read a key of type " + algorithm);
-        }
         try {
             // final String algo = buf.readString(); it has been already read
             final String curveName = buf.readString();

src/main/java/net/schmizz/sshj/common/SecurityUtils.java

@@ -259,6 +259,11 @@ public class SecurityUtils {
         return false;
     }
 
+    /**
+     * Configure whether to register the Bouncy Castle Security Provider. Must be called prior to other methods
+     *
+     * @param registerBouncyCastle Enable or disable Bouncy Castle Provider registration on subsequent method invocation
+     */
     public static synchronized void setRegisterBouncyCastle(boolean registerBouncyCastle) {
         SecurityUtils.registerBouncyCastle = registerBouncyCastle;
         registrationDone = false;

src/test/java/net/schmizz/sshj/DefaultSecurityProviderConfigTest.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package net.schmizz.sshj;
+
+import net.schmizz.sshj.common.SecurityUtils;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.security.Provider;
+import java.security.Security;
+import java.util.Arrays;
+import java.util.Optional;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+public class DefaultSecurityProviderConfigTest {
+
+    private static Provider bouncyCastleProvider;
+
+    @BeforeAll
+    public static void removeProviders() {
+        bouncyCastleProvider = Security.getProvider(SecurityUtils.BOUNCY_CASTLE);
+        if (bouncyCastleProvider != null) {
+            Security.removeProvider(SecurityUtils.BOUNCY_CASTLE);
+        }
+    }
+
+    @AfterAll
+    public static void addProviders() {
+        if (bouncyCastleProvider != null) {
+            Security.addProvider(bouncyCastleProvider);
+        }
+    }
+
+    @Test
+    public void testBouncyCastleNotRegistered() {
+        new DefaultSecurityProviderConfig();
+
+        assertBouncyCastleNotRegistered();
+    }
+
+    private void assertBouncyCastleNotRegistered() {
+        final Optional<String> bouncyCastleFound = Arrays.stream(Security.getProviders())
+                .map(Provider::getName)
+                .filter(SecurityUtils.BOUNCY_CASTLE::contentEquals)
+                .findFirst();
+
+        assertFalse(bouncyCastleFound.isPresent());
+    }
+}

src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java

@@ -19,7 +19,6 @@ import com.hierynomus.sshj.common.KeyDecryptionFailedException;
 import com.hierynomus.sshj.userauth.certificate.Certificate;
 import com.hierynomus.sshj.userauth.keyprovider.OpenSSHKeyV1KeyFile;
 import net.schmizz.sshj.common.KeyType;
-import net.schmizz.sshj.common.SecurityUtils;
 import net.schmizz.sshj.userauth.keyprovider.FileKeyProvider;
 import net.schmizz.sshj.userauth.keyprovider.OpenSSHKeyFile;
 import net.schmizz.sshj.userauth.password.PasswordFinder;
@@ -448,13 +447,6 @@ public class OpenSSHKeyFileTest {
         assertThrows(IOException.class, keyProvider::getPrivate, "This key is not in 'openssh-key-v1' format");
     }
 
-    @BeforeEach
-    public void checkBCRegistration() {
-        if (!SecurityUtils.isBouncyCastleRegistered()) {
-            throw new AssertionError("bouncy castle needed");
-        }
-    }
-
     private String readFile(String pathname)
             throws IOException {
         StringBuilder fileContents = new StringBuilder();