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 49ddb23f017fed1c3736c1bc4d5a265b21f37f0c Author: Thomas Wolf <[email protected]> AuthorDate: Sat Sep 20 21:22:35 2025 +0200 GH-502: Do not load/create BouncyCastleSecurityProvider reflectively It's not necessary since we have the optional dependency anyway. So standard classloading is good enough. For FIPS support, we still have to load the BouncyCastleFipsProvider reflectively. Get the class via Class.forName(), though. This loads the class through the normal mechanism that is also used for Java imports, so if we can load it that way, we have access to the BC classes we need. If we got through the thread context classloader, we might be able to load it, but later still fail to resolve imports. If a provider is registered already in the system make sure that we then can access the matching provider class. If both FIPS and non-FIPS BC providers are accessible (shouldn't happen), default to the FIPS one. If FIPS mode is explicitly set consider only the FIPS provider. --- .../bouncycastle/BouncyCastleAccessor.java | 105 +++++++++++++++++++++ .../bouncycastle/BouncyCastleEdDSAAccessor.java | 16 +++- .../BouncyCastleSecurityProviderRegistrar.java | 74 ++++++++------- sshd-osgi/pom.xml | 9 +- 4 files changed, 162 insertions(+), 42 deletions(-) diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleAccessor.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleAccessor.java new file mode 100644 index 000000000..e7d333049 --- /dev/null +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleAccessor.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.sshd.common.util.security.bouncycastle; + +import java.security.Provider; + +import org.apache.sshd.common.util.ReflectionUtils; +import org.bouncycastle.jcajce.interfaces.EdDSAKey; +import org.bouncycastle.jce.provider.BouncyCastleProvider; + +final class BouncyCastleAccessor { + + static final BouncyCastleAccessor INSTANCE = new BouncyCastleAccessor(); + + private BouncyCastleAccessor() { + super(); + } + + public Class<?> getProviderClass(String className) { + try { + return Inner.getProviderClass(className); + } catch (Throwable t) { + return null; + } + } + + public Provider createProvider(String className) throws ReflectiveOperationException { + try { + return Inner.createProvider(className); + } catch (Throwable t) { + return null; + } + } + + public boolean isSupported() { + try { + return Inner.isSupported(); + } catch (Throwable t) { + return false; + } + } + + private static final class Inner { + + private Inner() { + super(); + } + + static Class<?> getProviderClass(String className) { + try { + if (BouncyCastleSecurityProviderRegistrar.PROVIDER_CLASS.equals(className)) { + return BouncyCastleProvider.class; + } else if (BouncyCastleSecurityProviderRegistrar.FIPS_PROVIDER_CLASS.equals(className)) { + return Class.forName(className); + } + return null; + } catch (Throwable t) { + return null; + } + } + + static Provider createProvider(String className) throws ReflectiveOperationException { + if (BouncyCastleSecurityProviderRegistrar.PROVIDER_CLASS.equals(className)) { + try { + return new BouncyCastleProvider(); + } catch (Throwable t) { + return null; + } + } else if (BouncyCastleSecurityProviderRegistrar.FIPS_PROVIDER_CLASS.equals(className)) { + try { + return ReflectionUtils.newInstance(Class.forName(className), Provider.class); + } catch (ClassNotFoundException e) { + throw new ReflectiveOperationException("Cannot instantiate " + className, e); + } + } + return null; + } + + static boolean isSupported() { + try { + // Just something that forces class loading. + return EdDSAKey.class != null; + } catch (Throwable t) { + return false; + } + } + } + +} diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleEdDSAAccessor.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleEdDSAAccessor.java index 9242376d7..c3ea84ee8 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleEdDSAAccessor.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleEdDSAAccessor.java @@ -30,10 +30,22 @@ final class BouncyCastleEdDSAAccessor { public boolean isSupported() { try { - // Just something that forces class loading. - return EdDSAKey.class != null; + return Inner.isSupported(); } catch (Throwable t) { return false; } } + + private static final class Inner { + + private Inner() { + super(); + } + + static boolean isSupported() { + // Just something that forces class loading. + return EdDSAKey.class != null; + } + + } } 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 0872389e6..f5d186044 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 @@ -18,7 +18,6 @@ */ package org.apache.sshd.common.util.security.bouncycastle; -import java.lang.reflect.Field; import java.security.KeyFactory; import java.security.KeyPairGenerator; import java.security.PrivateKey; @@ -32,7 +31,6 @@ import org.apache.sshd.common.util.ExceptionUtils; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.security.AbstractSecurityProviderRegistrar; import org.apache.sshd.common.util.security.SecurityUtils; -import org.apache.sshd.common.util.threads.ThreadUtils; /** * @author <a href="mailto:[email protected]";>Apache MINA SSHD Project</a> @@ -43,7 +41,6 @@ public class BouncyCastleSecurityProviderRegistrar extends AbstractSecurityProvi public static final String FIPS_PROVIDER_CLASS = "org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider"; private static final String BCFIPS_PROVIDER_NAME = "BCFIPS"; private static final String BC_PROVIDER_NAME = "BC"; - private static final String NAME_FIELD = "PROVIDER_NAME"; // Do not define a static registrar instance to minimize class loading issues private final AtomicReference<Boolean> supportHolder = new AtomicReference<>(null); @@ -124,43 +121,43 @@ public class BouncyCastleSecurityProviderRegistrar extends AbstractSecurityProvi return supported.booleanValue(); } boolean requireFips = SecurityUtils.isFipsMode(); - Class<?> clazz = null; - if (!requireFips) { - clazz = ThreadUtils.resolveDefaultClass(getClass(), PROVIDER_CLASS); - } - if (clazz == null) { - clazz = ThreadUtils.resolveDefaultClass(getClass(), FIPS_PROVIDER_CLASS); - } - if (clazz != null) { + if (requireFips) { // Apache MINA sshd assumes that if we can get at the provider class, we can also get any other class we // need. However, and BC-based optional stuff should actually check if it does have the concrete // classes it needs accessible. The FIPS version has only a subset of the full BC. - providerClass = clazz.getName(); - Provider provider = Security.getProvider(BCFIPS_PROVIDER_NAME); - if (provider != null) { + if (BouncyCastleAccessor.INSTANCE.getProviderClass(FIPS_PROVIDER_CLASS) == null) { + supported = Boolean.FALSE; + } else { + providerClass = FIPS_PROVIDER_CLASS; providerName = BCFIPS_PROVIDER_NAME; - } else if (!requireFips) { - provider = Security.getProvider(BC_PROVIDER_NAME); - if (provider != null) { - providerName = BC_PROVIDER_NAME; - } - } - if (providerName == null) { - Field f; - try { - f = clazz.getField(NAME_FIELD); - Object nameValue = f.get(null); - if (nameValue instanceof String) { - providerName = nameValue.toString(); - } - } catch (Exception e) { - log.warn("Alleged Bouncy Castle class {} has no {}; ignoring this provider.", providerClass, NAME_FIELD, - e); - } + supported = Boolean.TRUE; } - supported = Boolean.valueOf(providerName != null); } else { - supported = Boolean.FALSE; + // Check first what providers we have installed in the system. We also need to be able to load classes + // from there, so check if we can load the class. + boolean fipsInstalled = Security.getProvider(BCFIPS_PROVIDER_NAME) != null; + boolean bcInstalled = Security.getProvider(BC_PROVIDER_NAME) != null; + boolean haveFips = BouncyCastleAccessor.INSTANCE.getProviderClass(FIPS_PROVIDER_CLASS) != null; + boolean haveBc = BouncyCastleAccessor.INSTANCE.getProviderClass(PROVIDER_CLASS) != null; + if (fipsInstalled && haveFips) { + providerClass = FIPS_PROVIDER_CLASS; + providerName = BCFIPS_PROVIDER_NAME; + supported = Boolean.TRUE; + } else if (bcInstalled && haveBc) { + providerClass = PROVIDER_CLASS; + providerName = BC_PROVIDER_NAME; + supported = Boolean.TRUE; + } else if (haveFips) { + providerClass = FIPS_PROVIDER_CLASS; + providerName = BCFIPS_PROVIDER_NAME; + supported = Boolean.TRUE; + } else if (haveBc) { + providerClass = PROVIDER_CLASS; + providerName = BC_PROVIDER_NAME; + supported = Boolean.TRUE; + } else { + supported = Boolean.FALSE; + } } supportHolder.set(supported); } @@ -168,6 +165,15 @@ public class BouncyCastleSecurityProviderRegistrar extends AbstractSecurityProvi return supported.booleanValue(); } + @Override + protected Provider createProviderInstance(String providerClassName) throws ReflectiveOperationException { + Provider result = BouncyCastleAccessor.INSTANCE.createProvider(providerClassName); + if (result == null) { + throw new ReflectiveOperationException("Cannot instantiate " + providerClassName); + } + return result; + } + @Override public PublicKey getPublicKey(PrivateKey key) { if (isEnabled() && isEdDSASupported() && key.getClass().getPackage().getName().startsWith("org.bouncycastle.")) { diff --git a/sshd-osgi/pom.xml b/sshd-osgi/pom.xml index c21a286ca..2b49d7931 100644 --- a/sshd-osgi/pom.xml +++ b/sshd-osgi/pom.xml @@ -35,18 +35,15 @@ <properties> <projectRoot>${project.basedir}/..</projectRoot> <!-- - The BC security provider class resides in a package that is referenced nowhere, except reflectively in the BouncyCastleSecurityRegistrar. + The BC FIPS security provider class resides in a package that is referenced nowhere, except reflectively in the BouncyCastleSecurityRegistrar. The (optional) package import will thus be missing in the generated MANIFEST.MF. However, the BouncyCastleSecurityRegistrar expects to find class org.bouncycastle.jce.provider.BouncyCastleProvider on the classpath; otherwise its isSupported() returns false and Bouncycastle is considered not available. - + However, in OSGi the package will not be on the bundle classpath if there is no Import-Package for it. (And using a Require-Bundle would restrict bundle wiring too much.) - - Arguably this is a shortcoming of the BouncyCastleSecurityRegistrar. For the EdDSASecurityProviderRegistrar, this problem does not exist - since the security provider is in a package that is also referenced elsewhere. --> - <bnd.extraImports>org.bouncycastle.jce.provider;version="$$<range;[==,+);${bouncycastle.version}>";resolution:=optional,</bnd.extraImports> + <bnd.extraImports>org.bouncycastle.jcajce.provider;version="$$<range;[==,+);${bouncycastle.version}>";resolution:=optional,</bnd.extraImports> </properties> <dependencies>
