From 239dc862e7761f8f846034d3f11f44ef06bed6af Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 11 Sep 2018 19:06:51 +0200 Subject: [PATCH] Ensure X509KeyManager methods are called on the correct time when using server-side and support more methods of ExtendedSSLSession. Motivation: Before when on server-side we just called the X509KeyManager methods when handshake() was called the first time which is not quite correct as we may not have received the full SSL hello / handshake and so could not extra for example the SNI hostname that was requested. OpenSSL exposes the SSL_CTX_set_cert_cb function which allows to set a callback which is executed at the correct moment, so we should use it. This also allows us to support more methods of ExtendedSSLSession easily. Modifications: - Make use of new methods exposed by netty-tcnative since https://github.com/netty/netty-tcnative/pull/388 to ensure we select the key material at the correct time. - Implement more methods of ExtendedOpenSslSession - Add unit tests to ensure we are able to retrieve various things on server-side in the X509KeyManager and so verify it is called at the correct time. - Simplify code by using new netty-tcnative methods. Result: More correct implementation for server-side usage and more complete implemented of ExtendedSSLSession. --- .../handler/ssl/ExtendedOpenSslSession.java | 56 +++++++--------- .../io/netty/handler/ssl/Java8SslUtils.java | 7 ++ .../handler/ssl/OpenSslClientContext.java | 5 -- .../ssl/OpenSslKeyMaterialManager.java | 28 +++----- .../handler/ssl/OpenSslServerContext.java | 11 +-- .../ReferenceCountedOpenSslClientContext.java | 32 +++++---- .../ssl/ReferenceCountedOpenSslContext.java | 2 - .../ssl/ReferenceCountedOpenSslEngine.java | 66 +++++++++++++++--- .../ReferenceCountedOpenSslServerContext.java | 67 ++++++++++++------- .../ssl/SignatureAlgorithmConverter.java | 55 +++++++++++++++ .../netty/handler/ssl/OpenSslTestUtils.java | 4 ++ .../ssl/SignatureAlgorithmConverterTest.java | 44 ++++++++++++ .../handler/ssl/SniClientJava8TestUtil.java | 22 ++++-- .../io/netty/handler/ssl/SniClientTest.java | 3 +- .../io/netty/handler/ssl/SslErrorTest.java | 4 +- pom.xml | 2 +- 16 files changed, 276 insertions(+), 132 deletions(-) create mode 100644 handler/src/main/java/io/netty/handler/ssl/SignatureAlgorithmConverter.java create mode 100644 handler/src/test/java/io/netty/handler/ssl/SignatureAlgorithmConverterTest.java diff --git a/handler/src/main/java/io/netty/handler/ssl/ExtendedOpenSslSession.java b/handler/src/main/java/io/netty/handler/ssl/ExtendedOpenSslSession.java index 87185db529d..69a6125d172 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ExtendedOpenSslSession.java +++ b/handler/src/main/java/io/netty/handler/ssl/ExtendedOpenSslSession.java @@ -15,8 +15,6 @@ */ package io.netty.handler.ssl; -import io.netty.util.internal.EmptyArrays; - import javax.net.ssl.ExtendedSSLSession; import javax.net.ssl.SSLException; import javax.net.ssl.SSLPeerUnverifiedException; @@ -60,128 +58,122 @@ public List getStatusResponses() { } @Override - public void handshakeFinished() throws SSLException { + public final void handshakeFinished() throws SSLException { wrapped.handshakeFinished(); } @Override - public void tryExpandApplicationBufferSize(int packetLengthDataOnly) { + public final void tryExpandApplicationBufferSize(int packetLengthDataOnly) { wrapped.tryExpandApplicationBufferSize(packetLengthDataOnly); } @Override - public String[] getLocalSupportedSignatureAlgorithms() { + public final String[] getLocalSupportedSignatureAlgorithms() { return LOCAL_SUPPORTED_SIGNATURE_ALGORITHMS.clone(); } @Override - public String[] getPeerSupportedSignatureAlgorithms() { - // Always return empty for now. - return EmptyArrays.EMPTY_STRINGS; - } - - @Override - public byte[] getId() { + public final byte[] getId() { return wrapped.getId(); } @Override - public SSLSessionContext getSessionContext() { + public final SSLSessionContext getSessionContext() { return wrapped.getSessionContext(); } @Override - public long getCreationTime() { + public final long getCreationTime() { return wrapped.getCreationTime(); } @Override - public long getLastAccessedTime() { + public final long getLastAccessedTime() { return wrapped.getLastAccessedTime(); } @Override - public void invalidate() { + public final void invalidate() { wrapped.invalidate(); } @Override - public boolean isValid() { + public final boolean isValid() { return wrapped.isValid(); } @Override - public void putValue(String s, Object o) { + public final void putValue(String s, Object o) { wrapped.putValue(s, o); } @Override - public Object getValue(String s) { + public final Object getValue(String s) { return wrapped.getValue(s); } @Override - public void removeValue(String s) { + public final void removeValue(String s) { wrapped.removeValue(s); } @Override - public String[] getValueNames() { + public final String[] getValueNames() { return wrapped.getValueNames(); } @Override - public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { + public final Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { return wrapped.getPeerCertificates(); } @Override - public Certificate[] getLocalCertificates() { + public final Certificate[] getLocalCertificates() { return wrapped.getLocalCertificates(); } @Override - public X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException { + public final X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException { return wrapped.getPeerCertificateChain(); } @Override - public Principal getPeerPrincipal() throws SSLPeerUnverifiedException { + public final Principal getPeerPrincipal() throws SSLPeerUnverifiedException { return wrapped.getPeerPrincipal(); } @Override - public Principal getLocalPrincipal() { + public final Principal getLocalPrincipal() { return wrapped.getLocalPrincipal(); } @Override - public String getCipherSuite() { + public final String getCipherSuite() { return wrapped.getCipherSuite(); } @Override - public String getProtocol() { + public final String getProtocol() { return wrapped.getProtocol(); } @Override - public String getPeerHost() { + public final String getPeerHost() { return wrapped.getPeerHost(); } @Override - public int getPeerPort() { + public final int getPeerPort() { return wrapped.getPeerPort(); } @Override - public int getPacketBufferSize() { + public final int getPacketBufferSize() { return wrapped.getPacketBufferSize(); } @Override - public int getApplicationBufferSize() { + public final int getApplicationBufferSize() { return wrapped.getApplicationBufferSize(); } } diff --git a/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java b/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java index 648c0f2e65c..c47d9657121 100644 --- a/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java +++ b/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java @@ -62,6 +62,13 @@ static List getSniHostNames(List names) { return sniServerNames; } + static List getSniHostName(byte[] hostname) { + if (hostname == null || hostname.length == 0) { + return Collections.emptyList(); + } + return Collections.singletonList(new SNIHostName(hostname)); + } + static boolean getUseCipherSuitesOrder(SSLParameters sslParameters) { return sslParameters.getUseCipherSuitesOrder(); } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java index 46412e9f526..6856c7f2746 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java @@ -203,9 +203,4 @@ public OpenSslClientContext(File trustCertCollectionFile, TrustManagerFactory tr public OpenSslSessionContext sessionContext() { return sessionContext; } - - @Override - OpenSslKeyMaterialManager keyMaterialManager() { - return null; - } } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslKeyMaterialManager.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslKeyMaterialManager.java index f365dee279c..8b8a41d1146 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslKeyMaterialManager.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslKeyMaterialManager.java @@ -74,36 +74,25 @@ void setKeyMaterialServerSide(ReferenceCountedOpenSslEngine engine) throws SSLEx if (type != null) { String alias = chooseServerAlias(engine, type); if (alias != null && aliases.add(alias)) { - OpenSslKeyMaterial keyMaterial = null; - try { - keyMaterial = provider.chooseKeyMaterial(engine.alloc, alias); - if (keyMaterial != null) { - SSL.setKeyMaterialServerSide( - ssl, keyMaterial.certificateChainAddress(), keyMaterial.privateKeyAddress()); - } - } catch (SSLException e) { - throw e; - } catch (Exception e) { - throw new SSLException(e); - } finally { - if (keyMaterial != null) { - keyMaterial.release(); - } - } + setKeyMaterial(engine, alias); } } } } - void setKeyMaterialClientSide(ReferenceCountedOpenSslEngine engine, long certOut, long keyOut, String[] keyTypes, + void setKeyMaterialClientSide(ReferenceCountedOpenSslEngine engine, String[] keyTypes, X500Principal[] issuer) throws SSLException { String alias = chooseClientAlias(engine, keyTypes, issuer); + setKeyMaterial(engine, alias); + } + + private void setKeyMaterial(ReferenceCountedOpenSslEngine engine, String alias) throws SSLException { OpenSslKeyMaterial keyMaterial = null; try { keyMaterial = provider.chooseKeyMaterial(engine.alloc, alias); if (keyMaterial != null) { - SSL.setKeyMaterialClientSide(engine.sslPointer(), certOut, keyOut, - keyMaterial.certificateChainAddress(), keyMaterial.privateKeyAddress()); + SSL.setKeyMaterial(engine.sslPointer(), + keyMaterial.certificateChainAddress(), keyMaterial.privateKeyAddress()); } } catch (SSLException e) { throw e; @@ -115,7 +104,6 @@ void setKeyMaterialClientSide(ReferenceCountedOpenSslEngine engine, long certOut } } } - private String chooseClientAlias(ReferenceCountedOpenSslEngine engine, String[] keyTypes, X500Principal[] issuer) { X509KeyManager manager = provider.keyManager(); diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java index f57434b133c..d9cf94e971d 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java @@ -15,7 +15,6 @@ */ package io.netty.handler.ssl; -import io.netty.handler.ssl.ReferenceCountedOpenSslServerContext.ServerContext; import io.netty.internal.tcnative.SSL; import java.io.File; @@ -37,7 +36,6 @@ */ public final class OpenSslServerContext extends OpenSslContext { private final OpenSslServerSessionContext sessionContext; - private final OpenSslKeyMaterialManager keyMaterialManager; /** * Creates a new instance. @@ -349,10 +347,8 @@ private OpenSslServerContext( // Create a new SSL_CTX and configure it. boolean success = false; try { - ServerContext context = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, + sessionContext = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory); - sessionContext = context.sessionContext; - keyMaterialManager = context.keyMaterialManager; success = true; } finally { if (!success) { @@ -365,9 +361,4 @@ private OpenSslServerContext( public OpenSslServerSessionContext sessionContext() { return sessionContext; } - - @Override - OpenSslKeyMaterialManager keyMaterialManager() { - return keyMaterialManager; - } } diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java index 29815c429e9..9e524a7a008 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java @@ -15,15 +15,16 @@ */ package io.netty.handler.ssl; +import io.netty.internal.tcnative.CertificateCallback; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; -import io.netty.internal.tcnative.CertificateRequestedCallback; import io.netty.internal.tcnative.SSL; import io.netty.internal.tcnative.SSLContext; import java.security.KeyStore; import java.security.PrivateKey; import java.security.cert.X509Certificate; +import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -68,11 +69,6 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted } } - @Override - OpenSslKeyMaterialManager keyMaterialManager() { - return null; - } - @Override public OpenSslSessionContext sessionContext() { return sessionContext; @@ -118,7 +114,7 @@ static OpenSslSessionContext newSessionContext(ReferenceCountedOpenSslContext th if (keyMaterialProvider != null) { OpenSslKeyMaterialManager materialManager = new OpenSslKeyMaterialManager(keyMaterialProvider); - SSLContext.setCertRequestedCallback(ctx, new OpenSslCertificateRequestedCallback( + SSLContext.setCertificateCallback(ctx, new OpenSslClientCertificateCallback( engineMap, materialManager)); } } @@ -238,18 +234,17 @@ void verify(ReferenceCountedOpenSslEngine engine, X509Certificate[] peerCerts, S } } - private static final class OpenSslCertificateRequestedCallback implements CertificateRequestedCallback { + private static final class OpenSslClientCertificateCallback implements CertificateCallback { private final OpenSslEngineMap engineMap; private final OpenSslKeyMaterialManager keyManagerHolder; - OpenSslCertificateRequestedCallback(OpenSslEngineMap engineMap, OpenSslKeyMaterialManager keyManagerHolder) { + OpenSslClientCertificateCallback(OpenSslEngineMap engineMap, OpenSslKeyMaterialManager keyManagerHolder) { this.engineMap = engineMap; this.keyManagerHolder = keyManagerHolder; } @Override - public void requested( - long ssl, long certOut, long keyOut, byte[] keyTypeBytes, byte[][] asn1DerEncodedPrincipals) { + public void handle(long ssl, byte[] keyTypeBytes, byte[][] asn1DerEncodedPrincipals) throws Exception { final ReferenceCountedOpenSslEngine engine = engineMap.get(ssl); try { final Set keyTypesSet = supportedClientKeyTypes(keyTypeBytes); @@ -263,7 +258,7 @@ public void requested( issuers[i] = new X500Principal(asn1DerEncodedPrincipals[i]); } } - keyManagerHolder.setKeyMaterialClientSide(engine, certOut, keyOut, keyTypes, issuers); + keyManagerHolder.setKeyMaterialClientSide(engine, keyTypes, issuers); } catch (Throwable cause) { logger.debug("request of key failed", cause); SSLHandshakeException e = new SSLHandshakeException("General OpenSslEngine problem"); @@ -281,6 +276,9 @@ public void requested( * {@code X509ExtendedKeyManager.chooseEngineClientAlias}. */ private static Set supportedClientKeyTypes(byte[] clientCertificateTypes) { + if (clientCertificateTypes == null) { + return Collections.emptySet(); + } Set result = new HashSet(clientCertificateTypes.length); for (byte keyTypeCode : clientCertificateTypes) { String keyType = clientKeyType(keyTypeCode); @@ -296,15 +294,15 @@ private static Set supportedClientKeyTypes(byte[] clientCertificateTypes private static String clientKeyType(byte clientCertificateType) { // See also http://www.ietf.org/assignments/tls-parameters/tls-parameters.xml switch (clientCertificateType) { - case CertificateRequestedCallback.TLS_CT_RSA_SIGN: + case CertificateCallback.TLS_CT_RSA_SIGN: return OpenSslKeyMaterialManager.KEY_TYPE_RSA; // RFC rsa_sign - case CertificateRequestedCallback.TLS_CT_RSA_FIXED_DH: + case CertificateCallback.TLS_CT_RSA_FIXED_DH: return OpenSslKeyMaterialManager.KEY_TYPE_DH_RSA; // RFC rsa_fixed_dh - case CertificateRequestedCallback.TLS_CT_ECDSA_SIGN: + case CertificateCallback.TLS_CT_ECDSA_SIGN: return OpenSslKeyMaterialManager.KEY_TYPE_EC; // RFC ecdsa_sign - case CertificateRequestedCallback.TLS_CT_RSA_FIXED_ECDH: + case CertificateCallback.TLS_CT_RSA_FIXED_ECDH: return OpenSslKeyMaterialManager.KEY_TYPE_EC_RSA; // RFC rsa_fixed_ecdh - case CertificateRequestedCallback.TLS_CT_ECDSA_FIXED_ECDH: + case CertificateCallback.TLS_CT_ECDSA_FIXED_ECDH: return OpenSslKeyMaterialManager.KEY_TYPE_EC_EC; // RFC ecdsa_fixed_ecdh default: return null; diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java index 18c86795744..cbdbbc86f5f 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java @@ -366,8 +366,6 @@ SSLEngine newEngine0(ByteBufAllocator alloc, String peerHost, int peerPort, bool return new ReferenceCountedOpenSslEngine(this, alloc, peerHost, peerPort, jdkCompatibilityMode, true); } - abstract OpenSslKeyMaterialManager keyMaterialManager(); - /** * Returns a new server-side {@link SSLEngine} with the current configuration. */ diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java index 1d608eeeb7f..b570cb84ee0 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -20,6 +20,7 @@ import io.netty.internal.tcnative.Buffer; import io.netty.internal.tcnative.SSL; import io.netty.util.AbstractReferenceCounted; +import io.netty.util.CharsetUtil; import io.netty.util.ReferenceCounted; import io.netty.util.ResourceLeakDetector; import io.netty.util.ResourceLeakDetectorFactory; @@ -142,7 +143,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc // OpenSSL state private long ssl; private long networkBIO; - private boolean certificateSet; private enum HandshakeState { /** @@ -217,7 +217,6 @@ protected void deallocate() { private final Certificate[] localCerts; private final ByteBuffer[] singleSrcBuffer = new ByteBuffer[1]; private final ByteBuffer[] singleDstBuffer = new ByteBuffer[1]; - private final OpenSslKeyMaterialManager keyMaterialManager; private final boolean enableOcsp; private int maxWrapOverhead; private int maxWrapBufferSize; @@ -238,18 +237,69 @@ protected void deallocate() { * wrap or unwrap call. * @param leakDetection {@code true} to enable leak detection of this object. */ - ReferenceCountedOpenSslEngine(ReferenceCountedOpenSslContext context, ByteBufAllocator alloc, String peerHost, + ReferenceCountedOpenSslEngine(ReferenceCountedOpenSslContext context, final ByteBufAllocator alloc, String peerHost, int peerPort, boolean jdkCompatibilityMode, boolean leakDetection) { super(peerHost, peerPort); OpenSsl.ensureAvailability(); this.alloc = checkNotNull(alloc, "alloc"); apn = (OpenSslApplicationProtocolNegotiator) context.applicationProtocolNegotiator(); clientMode = context.isClient(); - if (PlatformDependent.javaVersion() >= 7 && context.isClient()) { + if (PlatformDependent.javaVersion() >= 7) { session = new ExtendedOpenSslSession(new DefaultOpenSslSession(context.sessionContext())) { + private String[] peerSupportedSignatureAlgorithms; + private List requestedServerNames; + @Override public List getRequestedServerNames() { - return Java8SslUtils.getSniHostNames(sniHostNames); + if (clientMode) { + return Java8SslUtils.getSniHostNames(sniHostNames); + } else { + synchronized (ReferenceCountedOpenSslEngine.this) { + if (requestedServerNames == null) { + if (isDestroyed()) { + requestedServerNames = Collections.emptyList(); + } else { + String name = SSL.getSniHostname(ssl); + if (name == null) { + requestedServerNames = Collections.emptyList(); + } else { + // Convert to bytes as we do not want to do any strict validation of the + // SNIHostName while creating it. + requestedServerNames = + Java8SslUtils.getSniHostName( + SSL.getSniHostname(ssl).getBytes(CharsetUtil.UTF_8)); + } + } + } + return requestedServerNames; + } + } + } + + @Override + public String[] getPeerSupportedSignatureAlgorithms() { + synchronized (ReferenceCountedOpenSslEngine.this) { + if (peerSupportedSignatureAlgorithms == null) { + if (isDestroyed()) { + peerSupportedSignatureAlgorithms = EmptyArrays.EMPTY_STRINGS; + } else { + String[] algs = SSL.getSigAlgs(ssl); + if (algs == null) { + peerSupportedSignatureAlgorithms = EmptyArrays.EMPTY_STRINGS; + } else { + List algorithmList = new ArrayList(algs.length); + for (String alg: algs) { + String converted = SignatureAlgorithmConverter.toJavaName(alg); + if (converted != null) { + algorithmList.add(converted); + } + } + peerSupportedSignatureAlgorithms = algorithmList.toArray(new String[0]); + } + } + } + return peerSupportedSignatureAlgorithms.clone(); + } } }; } else { @@ -257,7 +307,6 @@ public List getRequestedServerNames() { } engineMap = context.engineMap; localCerts = context.keyCertChain; - keyMaterialManager = context.keyMaterialManager(); enableOcsp = context.enableOcsp; this.jdkCompatibilityMode = jdkCompatibilityMode; Lock readerLock = context.ctxLock.readLock(); @@ -1579,11 +1628,6 @@ private SSLEngineResult.HandshakeStatus handshake() throws SSLException { lastAccessed = System.currentTimeMillis(); } - if (!certificateSet && keyMaterialManager != null) { - certificateSet = true; - keyMaterialManager.setKeyMaterialServerSide(this); - } - int code = SSL.doHandshake(ssl); if (code <= 0) { // Check if we have a pending exception that was created during the handshake and if so throw it after diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java index b9c8cf60b8c..d0608462319 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -16,6 +16,7 @@ package io.netty.handler.ssl; import io.netty.buffer.ByteBufAllocator; +import io.netty.internal.tcnative.CertificateCallback; import io.netty.internal.tcnative.SSL; import io.netty.internal.tcnative.SSLContext; import io.netty.internal.tcnative.SniHostNameMatcher; @@ -29,6 +30,7 @@ import java.security.cert.X509Certificate; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLException; +import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedTrustManager; import javax.net.ssl.X509TrustManager; @@ -48,7 +50,6 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted InternalLoggerFactory.getInstance(ReferenceCountedOpenSslServerContext.class); private static final byte[] ID = {'n', 'e', 't', 't', 'y'}; private final OpenSslServerSessionContext sessionContext; - private final OpenSslKeyMaterialManager keyMaterialManager; ReferenceCountedOpenSslServerContext( X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, @@ -72,10 +73,8 @@ private ReferenceCountedOpenSslServerContext( // Create a new SSL_CTX and configure it. boolean success = false; try { - ServerContext context = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, + sessionContext = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory); - sessionContext = context.sessionContext; - keyMaterialManager = context.keyMaterialManager; success = true; } finally { if (!success) { @@ -89,23 +88,13 @@ public OpenSslServerSessionContext sessionContext() { return sessionContext; } - @Override - OpenSslKeyMaterialManager keyMaterialManager() { - return keyMaterialManager; - } - - static final class ServerContext { - OpenSslServerSessionContext sessionContext; - OpenSslKeyMaterialManager keyMaterialManager; - } - - static ServerContext newSessionContext(ReferenceCountedOpenSslContext thiz, long ctx, OpenSslEngineMap engineMap, - X509Certificate[] trustCertCollection, - TrustManagerFactory trustManagerFactory, - X509Certificate[] keyCertChain, PrivateKey key, - String keyPassword, KeyManagerFactory keyManagerFactory) + static OpenSslServerSessionContext newSessionContext(ReferenceCountedOpenSslContext thiz, long ctx, + OpenSslEngineMap engineMap, + X509Certificate[] trustCertCollection, + TrustManagerFactory trustManagerFactory, + X509Certificate[] keyCertChain, PrivateKey key, + String keyPassword, KeyManagerFactory keyManagerFactory) throws SSLException { - ServerContext result = new ServerContext(); OpenSslKeyMaterialProvider keyMaterialProvider = null; try { try { @@ -134,7 +123,8 @@ static ServerContext newSessionContext(ReferenceCountedOpenSslContext thiz, long } keyMaterialProvider = providerFor(keyManagerFactory, keyPassword); - result.keyMaterialManager = new OpenSslKeyMaterialManager(keyMaterialProvider); + SSLContext.setCertificateCallback(ctx, new OpenSslServerCertificateCallback( + engineMap, new OpenSslKeyMaterialManager(keyMaterialProvider))); } } catch (Exception e) { throw new SSLException("failed to set certificate and key", e); @@ -159,8 +149,8 @@ static ServerContext newSessionContext(ReferenceCountedOpenSslContext thiz, long // Use this to prevent an error when running on java < 7 if (useExtendedTrustManager(manager)) { - SSLContext.setCertVerifyCallback(ctx, - new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager)); + SSLContext.setCertVerifyCallback(ctx, new ExtendedTrustManagerVerifyCallback( + engineMap, (X509ExtendedTrustManager) manager)); } else { SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager)); } @@ -191,12 +181,12 @@ static ServerContext newSessionContext(ReferenceCountedOpenSslContext thiz, long throw new SSLException("unable to setup trustmanager", e); } - result.sessionContext = new OpenSslServerSessionContext(thiz, keyMaterialProvider); - result.sessionContext.setSessionIdContext(ID); + OpenSslServerSessionContext sessionContext = new OpenSslServerSessionContext(thiz, keyMaterialProvider); + sessionContext.setSessionIdContext(ID); keyMaterialProvider = null; - return result; + return sessionContext; } finally { if (keyMaterialProvider != null) { keyMaterialProvider.destroy(); @@ -204,6 +194,31 @@ static ServerContext newSessionContext(ReferenceCountedOpenSslContext thiz, long } } + private static final class OpenSslServerCertificateCallback implements CertificateCallback { + private final OpenSslEngineMap engineMap; + private final OpenSslKeyMaterialManager keyManagerHolder; + + OpenSslServerCertificateCallback(OpenSslEngineMap engineMap, OpenSslKeyMaterialManager keyManagerHolder) { + this.engineMap = engineMap; + this.keyManagerHolder = keyManagerHolder; + } + + @Override + public void handle(long ssl, byte[] keyTypeBytes, byte[][] asn1DerEncodedPrincipals) throws Exception { + final ReferenceCountedOpenSslEngine engine = engineMap.get(ssl); + try { + // For now we just ignore the asn1DerEncodedPrincipals as this is kind of inline with what the + // OpenJDK SSLEngineImpl does. + keyManagerHolder.setKeyMaterialServerSide(engine); + } catch (Throwable cause) { + logger.debug("request of key failed", cause); + SSLHandshakeException e = new SSLHandshakeException("General OpenSslEngine problem"); + e.initCause(cause); + engine.handshakeException = e; + } + } + } + private static final class TrustManagerVerifyCallback extends AbstractCertificateVerifier { private final X509TrustManager manager; diff --git a/handler/src/main/java/io/netty/handler/ssl/SignatureAlgorithmConverter.java b/handler/src/main/java/io/netty/handler/ssl/SignatureAlgorithmConverter.java new file mode 100644 index 00000000000..c029cda2336 --- /dev/null +++ b/handler/src/main/java/io/netty/handler/ssl/SignatureAlgorithmConverter.java @@ -0,0 +1,55 @@ +/* + * Copyright 2018 The Netty Project + * + * The Netty Project 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 io.netty.handler.ssl; + +import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Converts OpenSSL signature Algorithm names to + * + * Java signature Algorithm names. + */ +final class SignatureAlgorithmConverter { + + private SignatureAlgorithmConverter() { } + + // OpenSSL has 3 different formats it uses at the moment we will match against all of these. + private static final Pattern PATTERN = Pattern.compile( + "((^[a-zA-Z].+)With(.+)Encryption$)|((^[a-zA-Z].+)(_with_|-with-)(.+$))"); + + /** + * Converts an OpenSSL algorithm name to a Java algorithm name and return it, + * or return {@code null} if the conversation failed because the format is not known. + */ + static String toJavaName(String opensslName) { + if (opensslName == null) { + return null; + } + Matcher matcher = PATTERN.matcher(opensslName); + if (matcher.matches()) { + String group2 = matcher.group(2); + if (group2 != null) { + return group2.toUpperCase(Locale.ROOT) + "with" + matcher.group(3).toUpperCase(Locale.ROOT); + } + if (matcher.group(4) != null) { + return matcher.group(7).toUpperCase(Locale.ROOT) + "with" + matcher.group(5).toUpperCase(Locale.ROOT); + } + } + return null; + } +} diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java index 7882a61b4f4..e8c46ed7a05 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java @@ -24,4 +24,8 @@ private OpenSslTestUtils() { static void checkShouldUseKeyManagerFactory() { assumeTrue(OpenSsl.supportsKeyManagerFactory() && OpenSsl.useKeyManagerFactory()); } + + static boolean isBoringSSL() { + return "BoringSSL".equals(OpenSsl.versionString()); + } } diff --git a/handler/src/test/java/io/netty/handler/ssl/SignatureAlgorithmConverterTest.java b/handler/src/test/java/io/netty/handler/ssl/SignatureAlgorithmConverterTest.java new file mode 100644 index 00000000000..41c5e0e4a0b --- /dev/null +++ b/handler/src/test/java/io/netty/handler/ssl/SignatureAlgorithmConverterTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 2018 The Netty Project + * + * The Netty Project 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 io.netty.handler.ssl; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class SignatureAlgorithmConverterTest { + + @Test + public void testWithEncryption() { + assertEquals("SHA512withRSA", SignatureAlgorithmConverter.toJavaName("sha512WithRSAEncryption")); + } + + @Test + public void testWithDash() { + assertEquals("SHA256withECDSA", SignatureAlgorithmConverter.toJavaName("ecdsa-with-SHA256")); + } + + @Test + public void testWithUnderscore() { + assertEquals("SHA256withDSA", SignatureAlgorithmConverter.toJavaName("dsa_with_SHA256")); + } + + @Test + public void testInvalid() { + assertNull(SignatureAlgorithmConverter.toJavaName("ThisIsSomeThingInvalid")); + } +} diff --git a/handler/src/test/java/io/netty/handler/ssl/SniClientJava8TestUtil.java b/handler/src/test/java/io/netty/handler/ssl/SniClientJava8TestUtil.java index c6d05d05c71..2260da85d0a 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SniClientJava8TestUtil.java +++ b/handler/src/test/java/io/netty/handler/ssl/SniClientJava8TestUtil.java @@ -63,6 +63,7 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -154,11 +155,11 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc } } - static void assertSSLSession(SSLSession session, String name) { - assertSSLSession(session, new SNIHostName(name)); + static void assertSSLSession(boolean clientSide, SSLSession session, String name) { + assertSSLSession(clientSide, session, new SNIHostName(name)); } - private static void assertSSLSession(SSLSession session, SNIServerName name) { + private static void assertSSLSession(boolean clientSide, SSLSession session, SNIServerName name) { Assert.assertNotNull(session); if (session instanceof ExtendedSSLSession) { ExtendedSSLSession extendedSSLSession = (ExtendedSSLSession) session; @@ -166,6 +167,17 @@ private static void assertSSLSession(SSLSession session, SNIServerName name) { Assert.assertEquals(1, names.size()); Assert.assertEquals(name, names.get(0)); Assert.assertTrue(extendedSSLSession.getLocalSupportedSignatureAlgorithms().length > 0); + if (clientSide) { + Assert.assertEquals(0, extendedSSLSession.getPeerSupportedSignatureAlgorithms().length); + } else { + if (session instanceof OpenSslSession && OpenSslTestUtils.isBoringSSL()) { + // BoringSSL does not support SSL_get_sigalgs(...) + // https://boringssl.googlesource.com/boringssl/+/ba16a1e405c617f4179bd780ad15522fb25b0a65%5E%21/ + Assert.assertEquals(0, extendedSSLSession.getPeerSupportedSignatureAlgorithms().length); + } else { + Assert.assertTrue(extendedSSLSession.getPeerSupportedSignatureAlgorithms().length > 0); + } + } } } @@ -215,7 +227,7 @@ public void checkClientTrusted(X509Certificate[] x509Certificates, String s, SSL @Override public void checkServerTrusted(X509Certificate[] x509Certificates, String s, SSLEngine sslEngine) throws CertificateException { - assertSSLSession(sslEngine.getHandshakeSession(), name); + assertSSLSession(sslEngine.getUseClientMode(), sslEngine.getHandshakeSession(), name); } @Override @@ -311,7 +323,7 @@ public String chooseEngineServerAlias(String s, Principal[] principals, SSLEngine sslEngine) { SSLSession session = sslEngine.getHandshakeSession(); - assertSSLSession(session, name); + assertSSLSession(sslEngine.getUseClientMode(), session, name); return ((X509ExtendedKeyManager) km) .chooseEngineServerAlias(s, principals, sslEngine); } diff --git a/handler/src/test/java/io/netty/handler/ssl/SniClientTest.java b/handler/src/test/java/io/netty/handler/ssl/SniClientTest.java index 238594bbf44..997261d57dc 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SniClientTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SniClientTest.java @@ -151,7 +151,8 @@ public SslContext map(String input) { Assert.assertNull(handler.engine().getHandshakeSession()); if (PlatformDependent.javaVersion() >= 8) { - SniClientJava8TestUtil.assertSSLSession(handler.engine().getSession(), sniHostName); + SniClientJava8TestUtil.assertSSLSession( + handler.engine().getUseClientMode(), handler.engine().getSession(), sniHostName); } } finally { if (cc != null) { diff --git a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java index 6285ae0c4ef..d935cdf683d 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java @@ -204,7 +204,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { verifyException(unwrappedCause, "expired", promise); } else if (reason == CertPathValidatorException.BasicReason.NOT_YET_VALID) { // BoringSSL uses "expired" in this case while others use "bad" - if ("BoringSSL".equals(OpenSsl.versionString())) { + if (OpenSslTestUtils.isBoringSSL()) { verifyException(unwrappedCause, "expired", promise); } else { verifyException(unwrappedCause, "bad", promise); @@ -216,7 +216,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { verifyException(unwrappedCause, "expired", promise); } else if (exception instanceof CertificateNotYetValidException) { // BoringSSL uses "expired" in this case while others use "bad" - if ("BoringSSL".equals(OpenSsl.versionString())) { + if (OpenSslTestUtils.isBoringSSL()) { verifyException(unwrappedCause, "expired", promise); } else { verifyException(unwrappedCause, "bad", promise); diff --git a/pom.xml b/pom.xml index 702828a86da..a9d0d333e1c 100644 --- a/pom.xml +++ b/pom.xml @@ -221,7 +221,7 @@ fedora netty-tcnative - 2.0.15.Final + 2.0.16.Final-SNAPSHOT ${os.detected.classifier} org.conscrypt conscrypt-openjdk-uber