Skip to content

Commit

Permalink
GH-524: Use built-in native AES from SunJCE if available
Browse files Browse the repository at this point in the history
Bouncy Castle apparently only has native implementations in its "LTS"
releases. BC 1.78.1 has none. SunJCE uses native code.

The "security registrar" architecture in Apache MINA sshd prefers
registered providers over the platform providers, and it automatically
registers Bouncy Castle if it's present. So with Bouncy Castle on the
classpath and an SSH connection for which an AES cipher was specified,
Apache MINA sshd would use the much slower Bouncy Castle AES.

Add a new "SunJCEWrapper" registrar that brings the SunJCE AES and
HmacSHA* to the front of the list, so that they are used even if Bouncy
Castle is also registered.

The new registrar can be disabled via a system property as usual, and
it is only enabled if the JVM indeed has a "SunJCE" security provider
registered.

See also https://issues.apache.org/jira/browse/SSHD-719 (issue closed
as "not needed").

Bug: #524
  • Loading branch information
tomaswolf committed Jul 25, 2024
1 parent a7ccaf7 commit f60f759
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 5 deletions.
12 changes: 12 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,23 @@

## Bug Fixes

* [GH-524](https://github.com/apache/mina-sshd/issues/524) Performance improvements

## New Features

* New utility methods `SftpClient.put(Path localFile, String remoteFileName)` and `SftpClient.put(InputStream in, String remoteFileName)` facilitate SFTP file uploading.

## Potential compatibility issues

There is a new `SecurityProviderRegistrar` that is registered by default
if there is a `SunJCE` security provider and that uses the AES and
HmacSHA* implementations from `SunJCE` even if Bouncy Castle is also
registered. `SunJCE` has native implementation, whereas Bouncy Castle
may not.

The new registrar has the name "SunJCEWrapper" and can be configured
like any other registrar. It can be disabled via the system property
`org.apache.sshd.security.provider.SunJCEWrapper.enabled=false`.

## Major Code Re-factoring

Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public final class SecurityUtils {
public static final String SECURITY_PROVIDER_REGISTRARS = "org.apache.sshd.security.registrars";
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"));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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;

import java.security.Provider;
import java.security.Security;
import java.util.HashMap;
import java.util.Map;

import org.apache.sshd.common.util.GenericUtils;

/**
* This is registrar ensures that even if other registrars are active, we still use the Java built-in security provider
* at least for some security entities.
* <p>
* The problem is that if the Bouncy Castle registrar is present and enabled, we'll end up using the Bouncy Castle
* implementations for just about anything. But not all Bouncy Castle versions have native implementations of the
* algorithms. If BC AES is used and is implemented in Java, performance will be very poor. SunJCE's AES uses native
* code and is much faster.
* </p>
* <p>
* If no Bouncy Castle is registered, this extra registrar will not have an effect. Like all registrars, this one can be
* disabled via a system property {@code org.apache.sshd.security.provider.SunJCEWrapper.enabled=false}. Note that this
* does <em>not</em> disable the fallback to the platform provider; it only disables this wrapper which can be used to
* force the use of the "SunJCE" standard Java provider even if some other registrar also supports an algorithm (and
* would thus normally be preferred).
* </p>
* <p>
* The registrar can be configured as usual. By default it has only the AES cipher and the SHA macs enabled, everything
* else is disabled.
* </p>
*
* @author <a href="mailto:[email protected]">Apache MINA SSHD Project</a>
*/
public class SunJCESecurityProviderRegistrar extends AbstractSecurityProviderRegistrar {

private final Map<String, String> defaultProperties = new HashMap<>();

public SunJCESecurityProviderRegistrar() {
super("SunJCEWrapper");
String baseName = getBasePropertyName();
defaultProperties.put(baseName + ".Cipher", "AES");
defaultProperties.put(baseName + ".Mac", "HmacSha1,HmacSha224,HmacSha256,HmacSha384,HmacSha512");
}

@Override
public boolean isEnabled() {
if (!super.isEnabled()) {
return false;
}
return isSupported();
}

@Override
public String getDefaultSecurityEntitySupportValue(Class<?> entityType) {
return "";
}

@Override
public String getString(String name) {
String configured = super.getString(name);
if (GenericUtils.isEmpty(configured)) {
String byDefault = defaultProperties.get(name);
if (byDefault != null) {
return byDefault;
}
}
return configured;
}

@Override
public boolean isNamedProviderUsed() {
return false;
}

@Override
public Provider getSecurityProvider() {
return Security.getProvider("SunJCE");
}

@Override
public boolean isSupported() {
return getSecurityProvider() != null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
import java.security.spec.AlgorithmParameterSpec;
import java.util.Arrays;

import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;

Expand Down Expand Up @@ -78,9 +80,15 @@ protected void ensureKeySizeSupported(
javax.crypto.Cipher cipher = SecurityUtils.getCipher(transformation);
byte[] key = new byte[bsize];
byte[] iv = new byte[ivsize];
AlgorithmParameterSpec params;
if (transformation.contains("/GCM/")) {
params = new GCMParameterSpec(128, iv);
} else {
params = new IvParameterSpec(iv);
}
cipher.init(javax.crypto.Cipher.ENCRYPT_MODE,
new SecretKeySpec(key, algorithm),
new IvParameterSpec(iv));
params);
} catch (GeneralSecurityException e) {
if (e instanceof InvalidKeyException) {
Assume.assumeTrue(algorithm + "/" + transformation + "[" + bsize + "/" + ivsize + "]", false /*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.security.SecureRandom;
import java.util.Arrays;

import org.apache.sshd.common.util.security.SecurityUtils;
import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
import org.bouncycastle.crypto.SecretWithEncapsulation;
import org.bouncycastle.pqc.crypto.ntruprime.SNTRUPrimeKEMExtractor;
Expand All @@ -42,9 +41,6 @@ private SNTRUP761() {
}

static boolean isSupported() {
if (!SecurityUtils.isBouncyCastleRegistered()) {
return false;
}
try {
return SNTRUPrimeParameters.sntrup761.getSessionKeySize() == 256; // BC < 1.78 had only 128
} catch (Throwable e) {
Expand Down

0 comments on commit f60f759

Please sign in to comment.