Skip to content

Commit

Permalink
common-security, doors: switch to openssl where possible
Browse files Browse the repository at this point in the history
Motivation:

Using the javax.net.SSLContext interface currently
does not allow (easily at least) for a pass-through
to native (open)SSL.  More recent versions of
Netty, however, use a new SslContext object which
looks for the presence of openssl in the environment
and uses it by default, falling back to the Java
implementation if it is not present on the host.

For xroot TLS, we have seen at least a 20% performance
gain over JSSE when using openssl.

Modification:

The major change comes in the CanlContextFactory.
We have refitted the Builder to take the class
of the context object to use.

The rest of the code has been refactored to fit this
API.  In most cases, switching to Netty was simple,
but in the few places where the javax.net.SSLContext
was necessary (in order explicitly to open a socket)
we have left that in place.

Finally, we have refactored the Http factory beans
and changed the injection to use the Netty version.

Result:

Where possible, the Netty context is used, which
prefers openssl by default and falls back to Java
where it doesn't exist.

No visible changes to the user.

Target: master
Patch: https://rb.dcache.org/r/13044
Acked-by: Tigran
  • Loading branch information
alrossi committed Jun 2, 2021
1 parent f64a880 commit ddd6c88
Show file tree
Hide file tree
Showing 17 changed files with 231 additions and 164 deletions.
8 changes: 8 additions & 0 deletions modules/common-security/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,13 @@
<groupId>org.italiangrid</groupId>
<artifactId>voms-api-java</artifactId>
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-handler</artifactId>
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-tcnative-boringssl-static</artifactId>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableSet;
import eu.emi.security.authn.x509.X509Credential;

import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;

Expand Down Expand Up @@ -68,9 +69,9 @@ public DssContext create(InetSocketAddress remoteSocketAddress, InetSocketAddres
{
try {
SSLEngine delegate =
contextFactory.getContext(credential).createSSLEngine(
remoteSocketAddress.getHostString(),
remoteSocketAddress.getPort());
contextFactory.getContext(SSLContext.class, credential)
.createSSLEngine(remoteSocketAddress.getHostString(),
remoteSocketAddress.getPort());
SSLParameters sslParameters = delegate.getSSLParameters();
String[] cipherSuites = toArray(filter(asList(sslParameters.getCipherSuites()), not(in(bannedCiphers))), String.class);
String[] protocols = toArray(filter(asList(sslParameters.getProtocols()), not(in(bannedProtocols))), String.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import eu.emi.security.authn.x509.NamespaceCheckingMode;
import eu.emi.security.authn.x509.OCSPCheckingMode;

import javax.net.ssl.SSLContext;
import io.netty.buffer.ByteBufAllocator;
import io.netty.handler.ssl.SslContext;

import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;

Expand Down Expand Up @@ -63,7 +65,7 @@ public class ServerGsiEngineDssContextFactory implements DssContextFactory
private final CertificateFactory cf;
private final Set<String> bannedCiphers;
private final Set<String> bannedProtocols;
private final Callable<SSLContext> factory;
private final Callable<SslContext> factory;
private final KeyPairCache keyPairCache;

public ServerGsiEngineDssContextFactory(String args) throws Exception
Expand Down Expand Up @@ -97,14 +99,14 @@ public ServerGsiEngineDssContextFactory(File serverKeyPath, File serverCertifica
keyPairCache = new KeyPairCache(keyCacheLifetime, keyCacheLifetimeUnit);

factory = CanlContextFactory.custom()
.withCertificateAuthorityPath(certificateAuthorityPath.toPath())
.withCrlCheckingMode(crlMode)
.withOcspCheckingMode(ocspMode)
.withNamespaceMode(namespaceMode)
.withLazy(false)
.withKeyPath(serverKeyPath.toPath())
.withCertificatePath(serverCertificatePath.toPath())
.buildWithCaching();
.withCertificateAuthorityPath(certificateAuthorityPath.toPath())
.withCrlCheckingMode(crlMode)
.withOcspCheckingMode(ocspMode)
.withNamespaceMode(namespaceMode)
.withLazy(false)
.withKeyPath(serverKeyPath.toPath())
.withCertificatePath(serverCertificatePath.toPath())
.buildWithCaching(SslContext.class);
factory.call(); // Fail fast in case of config errors
}

Expand All @@ -113,8 +115,9 @@ public DssContext create(InetSocketAddress remoteSocketAddress, InetSocketAddres
throws IOException
{
try {
SSLEngine delegate = factory.call().createSSLEngine(remoteSocketAddress.getHostString(),
remoteSocketAddress.getPort());
SSLEngine delegate = factory.call().newEngine(ByteBufAllocator.DEFAULT,
remoteSocketAddress.getHostString(),
remoteSocketAddress.getPort());
SSLParameters sslParameters = delegate.getSSLParameters();
String[] cipherSuites = toArray(filter(asList(sslParameters.getCipherSuites()), not(in(bannedCiphers))), String.class);
String[] protocols = toArray(filter(asList(sslParameters.getProtocols()), not(in(bannedProtocols))), String.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,23 @@
import eu.emi.security.authn.x509.StoreUpdateListener;
import eu.emi.security.authn.x509.ValidationError;
import eu.emi.security.authn.x509.ValidationErrorCategory;
import eu.emi.security.authn.x509.ValidationErrorListener;
import eu.emi.security.authn.x509.X509CertChainValidator;
import eu.emi.security.authn.x509.X509Credential;
import eu.emi.security.authn.x509.helpers.ssl.SSLTrustManager;
import eu.emi.security.authn.x509.impl.OpensslCertChainValidator;
import eu.emi.security.authn.x509.impl.PEMCredential;
import eu.emi.security.authn.x509.impl.ValidatorParams;
import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SslContextBuilder;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.SSLException;

import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.security.GeneralSecurityException;
Expand All @@ -62,6 +63,10 @@
/**
* SslContextFactory based on the CANL library. Uses the builder pattern to
* create immutable instances.
* <p/>
*
* Implements the SslContextFactory which allows specifying either Java or Native (OpenSSL)
* as implementation.
*/
public class CanlContextFactory implements SslContextFactory
{
Expand All @@ -72,6 +77,7 @@ public class CanlContextFactory implements SslContextFactory

private final SecureRandom secureRandom = new SecureRandom();
private final TrustManager[] trustManagers;
private final boolean startTls;

private static final AutoCloseable NOOP = new AutoCloseable()
{
Expand All @@ -81,8 +87,9 @@ public void close() throws Exception
}
};

protected CanlContextFactory(TrustManager... trustManagers)
protected CanlContextFactory(boolean startTls, TrustManager... trustManagers)
{
this.startTls = startTls;
this.trustManagers = trustManagers;
}

Expand All @@ -102,7 +109,19 @@ public TrustManager[] getTrustManagers()
}

@Override
public SSLContext getContext(X509Credential credential)
public <T> T getContext(Class<T> type, X509Credential credential)
throws GeneralSecurityException
{
if (type.isAssignableFrom(SSLContext.class)) {
return (T) getJavaSSLContext(credential);
} else if (type.isAssignableFrom(SslContext.class)) {
return (T) getNettySslContext(credential);
}

throw new GeneralSecurityException("cannot get SSL context of type " + type);
}

private SSLContext getJavaSSLContext(X509Credential credential)
throws GeneralSecurityException
{
KeyManager[] keyManagers;
Expand All @@ -117,6 +136,19 @@ public SSLContext getContext(X509Credential credential)
return context;
}

private SslContext getNettySslContext(X509Credential credential)
throws GeneralSecurityException
{
KeyManager keyManager = credential == null ? null : credential.getKeyManager();
SslContextBuilder builder = startTls ? SslContextBuilder.forServer(keyManager)
: SslContextBuilder.forClient();
try {
return builder.trustManager(trustManagers[0]).startTls(startTls).build();
} catch (SSLException e) {
throw new GeneralSecurityException("Could not get Netty SSL context: " + e.getMessage());
}
}

public static class Builder
{
private Path certificateAuthorityPath = FileSystems.getDefault().getPath("/etc/grid-security/certificates");
Expand All @@ -131,11 +163,18 @@ public static class Builder
private TimeUnit credentialUpdateIntervalUnit = TimeUnit.MINUTES;
private Supplier<AutoCloseable> loggingContextSupplier = () -> NOOP;
private long validationCacheLifetime = 300000;
private boolean startTls = true; // default/server mode

private Builder()
{
}

public Builder startTls(boolean startTls)
{
this.startTls = startTls;
return this;
}

public Builder withCertificateAuthorityPath(Path certificateAuthorityPath)
{
this.certificateAuthorityPath = certificateAuthorityPath;
Expand Down Expand Up @@ -271,17 +310,19 @@ public void loadingNotification(String location, String type, Severity level, Ex
}
return false;
});
return new CanlContextFactory(new SSLTrustManager(v));
return new CanlContextFactory(startTls, new SSLTrustManager(v));
}

public Callable<SSLContext> buildWithCaching()
{
public <T> Callable<T> buildWithCaching(Class<T> contextType) throws Exception {
final CanlContextFactory factory = build();
Callable<SSLContext> newContext =
() -> factory.getContext(
new PEMCredential(keyPath.toString(), certificatePath.toString(), null));
return memoizeWithExpiration(memoizeFromFiles(newContext, keyPath, certificatePath),
credentialUpdateInterval, credentialUpdateIntervalUnit);
PEMCredential credential
= new PEMCredential(keyPath.toString(), certificatePath.toString(), null);
Callable newContext = () -> factory.getContext(contextType, credential);
return (Callable<T>) memoizeWithExpiration(memoizeFromFiles(newContext,
keyPath,
certificatePath),
credentialUpdateInterval,
credentialUpdateIntervalUnit);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public CanlSslServerSocketCreator(File keyPath,
.withKeyPath(keyPath.toPath())
.withCertificatePath(certPath.toPath())
.withLazy(false)
.buildWithCaching();
.buildWithCaching(SSLContext.class);
factory.call();
} catch (Exception e) {
Throwables.propagateIfPossible(e, IOException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public CanlSslSocketCreator(File keyPath,
.withKeyPath(keyPath.toPath())
.withCertificatePath(certPath.toPath())
.withLazy(false)
.buildWithCaching();
.buildWithCaching(SSLContext.class);
factory.call();
} catch (Exception e) {
Throwables.propagateIfPossible(e, IOException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,23 @@
import eu.emi.security.authn.x509.X509Credential;

import javax.annotation.Nullable;
import javax.net.ssl.SSLContext;

import java.security.GeneralSecurityException;

/**
* Factory for creating SSLContext instances.
* Factory for creating SSL context instances.
* (e.g., javax.net.SSLContext or io.netty.handler.ssl.SslContext).
*/
public interface SslContextFactory
{
/**
* Provides an SSLContext that will use the supplied optional client
* Provides an SslContext that will use the supplied optional client
* credential for authentication.
* @param type of the context object to create.
* @param credential the credential to use, or null if no X.509 credential.
* @return an SSLContext to use with an SSLSocket.
* @return an SSL context to use.
* @throws GeneralSecurityException if there is a problem establishing the context.
*/
SSLContext getContext(@Nullable X509Credential credential) throws GeneralSecurityException;
<T> T getContext(Class<T> type, @Nullable X509Credential credential)
throws GeneralSecurityException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import eu.emi.security.authn.x509.NamespaceCheckingMode;
import eu.emi.security.authn.x509.OCSPCheckingMode;

import javax.net.ssl.SSLContext;
import io.netty.buffer.ByteBufAllocator;
import io.netty.handler.ssl.SslContext;

import javax.net.ssl.SSLEngine;

import java.io.File;
Expand Down Expand Up @@ -84,7 +86,7 @@ public class TlsFtpInterpreterFactory extends FtpInterpreterFactory

private Optional<String> anonUser;

private SSLContext sslContext;
private SslContext sslContext;

@Override
public void configure(Args args) throws ConfigurationException
Expand All @@ -104,7 +106,7 @@ public void configure(Args args) throws ConfigurationException
@Override
protected AbstractFtpDoorV1 createInterpreter()
{
SSLEngine engine = sslContext.createSSLEngine();
SSLEngine engine = sslContext.newEngine(ByteBufAllocator.DEFAULT);
engine.setNeedClientAuth(false);

/* REVISIT: with FTPS, it is possible for a client to send an X.509
Expand All @@ -119,7 +121,7 @@ protected AbstractFtpDoorV1 createInterpreter()
anonymousRoot, requireAnonEmailPassword);
}

protected SSLContext buildContext() throws Exception
protected SslContext buildContext() throws Exception
{
return CanlContextFactory.custom()
.withCertificatePath(service_cert.toPath())
Expand All @@ -130,7 +132,7 @@ protected SSLContext buildContext() throws Exception
.withNamespaceMode(namespaceMode)
.withLazy(false)
.withLoggingContext(new CDC()::restore)
.buildWithCaching()
.buildWithCaching(SslContext.class)
.call();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
*/
package org.dcache.xrootd.plugins.tls;

import dmg.cells.nucleus.CDC;
import eu.emi.security.authn.x509.CrlCheckingMode;
import eu.emi.security.authn.x509.NamespaceCheckingMode;
import eu.emi.security.authn.x509.OCSPCheckingMode;
import io.netty.handler.ssl.SslContext;
import java.io.File;
import java.util.Properties;
import javax.net.ssl.SSLContext;

import org.dcache.ssl.CanlContextFactory;

/**
Expand All @@ -42,7 +42,7 @@ public class CDCCanlTLSHandlerFactory extends SSLHandlerFactory {
private static final String OCSP_MODE = "xrootd.security.tls.ca.ocsp-mode";

@Override
protected SSLContext buildContext(Properties properties) throws Exception {
protected SslContext buildContext(Properties properties) throws Exception {
File serviceKey = new File(properties.getProperty(SERVICE_KEY));
File serviceCert = new File(properties.getProperty(SERVICE_CERT));
File serviceCaCerts = new File(properties.getProperty(SERVICE_CACERTS));
Expand All @@ -61,8 +61,8 @@ protected SSLContext buildContext(Properties properties) throws Exception {
.withOcspCheckingMode(ocspMode)
.withNamespaceMode(namespaceMode)
.withLazy(false)
.withLoggingContext(new CDC()::restore)
.buildWithCaching()
.startTls(startTls)
.buildWithCaching(SslContext.class)
.call();
}
}
4 changes: 4 additions & 0 deletions modules/dcache/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@
<groupId>io.netty</groupId>
<artifactId>netty-codec-haproxy</artifactId>
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-tcnative-boringssl-static</artifactId>
</dependency>
<dependency>
<groupId>com.sleepycat</groupId>
<artifactId>je</artifactId>
Expand Down
Loading

0 comments on commit ddd6c88

Please sign in to comment.