Skip to content

Commit 49ddb23

Browse files
tomaswolftomaswolf
committed
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.
1 parent ca8b864 commit 49ddb23

4 files changed

Lines changed: 162 additions & 42 deletions

File tree

‎sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleAccessor.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.sshd.common.util.security.bouncycastle;
20+
21+
import java.security.Provider;
22+
23+
import org.apache.sshd.common.util.ReflectionUtils;
24+
import org.bouncycastle.jcajce.interfaces.EdDSAKey;
25+
import org.bouncycastle.jce.provider.BouncyCastleProvider;
26+
27+
final class BouncyCastleAccessor {
28+
29+
static final BouncyCastleAccessor INSTANCE = new BouncyCastleAccessor();
30+
31+
private BouncyCastleAccessor() {
32+
super();
33+
}
34+
35+
public Class<?> getProviderClass(String className) {
36+
try {
37+
return Inner.getProviderClass(className);
38+
} catch (Throwable t) {
39+
return null;
40+
}
41+
}
42+
43+
public Provider createProvider(String className) throws ReflectiveOperationException {
44+
try {
45+
return Inner.createProvider(className);
46+
} catch (Throwable t) {
47+
return null;
48+
}
49+
}
50+
51+
public boolean isSupported() {
52+
try {
53+
return Inner.isSupported();
54+
} catch (Throwable t) {
55+
return false;
56+
}
57+
}
58+
59+
private static final class Inner {
60+
61+
private Inner() {
62+
super();
63+
}
64+
65+
static Class<?> getProviderClass(String className) {
66+
try {
67+
if (BouncyCastleSecurityProviderRegistrar.PROVIDER_CLASS.equals(className)) {
68+
return BouncyCastleProvider.class;
69+
} else if (BouncyCastleSecurityProviderRegistrar.FIPS_PROVIDER_CLASS.equals(className)) {
70+
return Class.forName(className);
71+
}
72+
return null;
73+
} catch (Throwable t) {
74+
return null;
75+
}
76+
}
77+
78+
static Provider createProvider(String className) throws ReflectiveOperationException {
79+
if (BouncyCastleSecurityProviderRegistrar.PROVIDER_CLASS.equals(className)) {
80+
try {
81+
return new BouncyCastleProvider();
82+
} catch (Throwable t) {
83+
return null;
84+
}
85+
} else if (BouncyCastleSecurityProviderRegistrar.FIPS_PROVIDER_CLASS.equals(className)) {
86+
try {
87+
return ReflectionUtils.newInstance(Class.forName(className), Provider.class);
88+
} catch (ClassNotFoundException e) {
89+
throw new ReflectiveOperationException("Cannot instantiate " + className, e);
90+
}
91+
}
92+
return null;
93+
}
94+
95+
static boolean isSupported() {
96+
try {
97+
// Just something that forces class loading.
98+
return EdDSAKey.class != null;
99+
} catch (Throwable t) {
100+
return false;
101+
}
102+
}
103+
}
104+
105+
}

‎sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleEdDSAAccessor.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,22 @@ private BouncyCastleEdDSAAccessor() {
3030

3131
public boolean isSupported() {
3232
try {
33-
// Just something that forces class loading.
34-
return EdDSAKey.class != null;
33+
return Inner.isSupported();
3534
} catch (Throwable t) {
3635
return false;
3736
}
3837
}
38+
39+
private static final class Inner {
40+
41+
private Inner() {
42+
super();
43+
}
44+
45+
static boolean isSupported() {
46+
// Just something that forces class loading.
47+
return EdDSAKey.class != null;
48+
}
49+
50+
}
3951
}

‎sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleSecurityProviderRegistrar.java‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919
package org.apache.sshd.common.util.security.bouncycastle;
2020

21-
import java.lang.reflect.Field;
2221
import java.security.KeyFactory;
2322
import java.security.KeyPairGenerator;
2423
import java.security.PrivateKey;
@@ -32,7 +31,6 @@
3231
import org.apache.sshd.common.util.GenericUtils;
3332
import org.apache.sshd.common.util.security.AbstractSecurityProviderRegistrar;
3433
import org.apache.sshd.common.util.security.SecurityUtils;
35-
import org.apache.sshd.common.util.threads.ThreadUtils;
3634

3735
/**
3836
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
@@ -43,7 +41,6 @@ public class BouncyCastleSecurityProviderRegistrar extends AbstractSecurityProvi
4341
public static final String FIPS_PROVIDER_CLASS = "org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider";
4442
private static final String BCFIPS_PROVIDER_NAME = "BCFIPS";
4543
private static final String BC_PROVIDER_NAME = "BC";
46-
private static final String NAME_FIELD = "PROVIDER_NAME";
4744

4845
// Do not define a static registrar instance to minimize class loading issues
4946
private final AtomicReference<Boolean> supportHolder = new AtomicReference<>(null);
@@ -124,50 +121,59 @@ public boolean isSupported() {
124121
return supported.booleanValue();
125122
}
126123
boolean requireFips = SecurityUtils.isFipsMode();
127-
Class<?> clazz = null;
128-
if (!requireFips) {
129-
clazz = ThreadUtils.resolveDefaultClass(getClass(), PROVIDER_CLASS);
130-
}
131-
if (clazz == null) {
132-
clazz = ThreadUtils.resolveDefaultClass(getClass(), FIPS_PROVIDER_CLASS);
133-
}
134-
if (clazz != null) {
124+
if (requireFips) {
135125
// Apache MINA sshd assumes that if we can get at the provider class, we can also get any other class we
136126
// need. However, and BC-based optional stuff should actually check if it does have the concrete
137127
// classes it needs accessible. The FIPS version has only a subset of the full BC.
138-
providerClass = clazz.getName();
139-
Provider provider = Security.getProvider(BCFIPS_PROVIDER_NAME);
140-
if (provider != null) {
128+
if (BouncyCastleAccessor.INSTANCE.getProviderClass(FIPS_PROVIDER_CLASS) == null) {
129+
supported = Boolean.FALSE;
130+
} else {
131+
providerClass = FIPS_PROVIDER_CLASS;
141132
providerName = BCFIPS_PROVIDER_NAME;
142-
} else if (!requireFips) {
143-
provider = Security.getProvider(BC_PROVIDER_NAME);
144-
if (provider != null) {
145-
providerName = BC_PROVIDER_NAME;
146-
}
147-
}
148-
if (providerName == null) {
149-
Field f;
150-
try {
151-
f = clazz.getField(NAME_FIELD);
152-
Object nameValue = f.get(null);
153-
if (nameValue instanceof String) {
154-
providerName = nameValue.toString();
155-
}
156-
} catch (Exception e) {
157-
log.warn("Alleged Bouncy Castle class {} has no {}; ignoring this provider.", providerClass, NAME_FIELD,
158-
e);
159-
}
133+
supported = Boolean.TRUE;
160134
}
161-
supported = Boolean.valueOf(providerName != null);
162135
} else {
163-
supported = Boolean.FALSE;
136+
// Check first what providers we have installed in the system. We also need to be able to load classes
137+
// from there, so check if we can load the class.
138+
boolean fipsInstalled = Security.getProvider(BCFIPS_PROVIDER_NAME) != null;
139+
boolean bcInstalled = Security.getProvider(BC_PROVIDER_NAME) != null;
140+
boolean haveFips = BouncyCastleAccessor.INSTANCE.getProviderClass(FIPS_PROVIDER_CLASS) != null;
141+
boolean haveBc = BouncyCastleAccessor.INSTANCE.getProviderClass(PROVIDER_CLASS) != null;
142+
if (fipsInstalled && haveFips) {
143+
providerClass = FIPS_PROVIDER_CLASS;
144+
providerName = BCFIPS_PROVIDER_NAME;
145+
supported = Boolean.TRUE;
146+
} else if (bcInstalled && haveBc) {
147+
providerClass = PROVIDER_CLASS;
148+
providerName = BC_PROVIDER_NAME;
149+
supported = Boolean.TRUE;
150+
} else if (haveFips) {
151+
providerClass = FIPS_PROVIDER_CLASS;
152+
providerName = BCFIPS_PROVIDER_NAME;
153+
supported = Boolean.TRUE;
154+
} else if (haveBc) {
155+
providerClass = PROVIDER_CLASS;
156+
providerName = BC_PROVIDER_NAME;
157+
supported = Boolean.TRUE;
158+
} else {
159+
supported = Boolean.FALSE;
160+
}
164161
}
165162
supportHolder.set(supported);
166163
}
167164

168165
return supported.booleanValue();
169166
}
170167

168+
@Override
169+
protected Provider createProviderInstance(String providerClassName) throws ReflectiveOperationException {
170+
Provider result = BouncyCastleAccessor.INSTANCE.createProvider(providerClassName);
171+
if (result == null) {
172+
throw new ReflectiveOperationException("Cannot instantiate " + providerClassName);
173+
}
174+
return result;
175+
}
176+
171177
@Override
172178
public PublicKey getPublicKey(PrivateKey key) {
173179
if (isEnabled() && isEdDSASupported() && key.getClass().getPackage().getName().startsWith("org.bouncycastle.")) {

‎sshd-osgi/pom.xml‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,15 @@
3535
<properties>
3636
<projectRoot>${project.basedir}/..</projectRoot>
3737
<!--
38-
The BC security provider class resides in a package that is referenced nowhere, except reflectively in the BouncyCastleSecurityRegistrar.
38+
The BC FIPS security provider class resides in a package that is referenced nowhere, except reflectively in the BouncyCastleSecurityRegistrar.
3939
The (optional) package import will thus be missing in the generated MANIFEST.MF. However, the BouncyCastleSecurityRegistrar expects to find
4040
class org.bouncycastle.jce.provider.BouncyCastleProvider on the classpath; otherwise its isSupported() returns false and Bouncycastle is
4141
considered not available.
42-
42+
4343
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
4444
bundle wiring too much.)
45-
46-
Arguably this is a shortcoming of the BouncyCastleSecurityRegistrar. For the EdDSASecurityProviderRegistrar, this problem does not exist
47-
since the security provider is in a package that is also referenced elsewhere.
4845
-->
49-
<bnd.extraImports>org.bouncycastle.jce.provider;version="$$&lt;range;[==,+);${bouncycastle.version}&gt;";resolution:=optional,</bnd.extraImports>
46+
<bnd.extraImports>org.bouncycastle.jcajce.provider;version="$$&lt;range;[==,+);${bouncycastle.version}&gt;";resolution:=optional,</bnd.extraImports>
5047
</properties>
5148

5249
<dependencies>

0 commit comments

Comments
 (0)