Skip to content

Commit

Permalink
Ensure X509KeyManager methods are called on the correct time when usi…
Browse files Browse the repository at this point in the history 
…ng 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 netty/netty-tcnative#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.
  • Loading branch information
@normanmaurer
normanmaurer committed Sep 14, 2018
1 parent 34d52fc commit 239dc86
Show file tree
Hide file tree
Showing 16 changed files with 276 additions and 132 deletions.
56 changes: 24 additions & 32 deletions handler/src/main/java/io/netty/handler/ssl/ExtendedOpenSslSession.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -60,128 +58,122 @@ public List<byte[]> 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();
}
}
7 changes: 7 additions & 0 deletions handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ static List getSniHostNames(List<String> 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();
}
Expand Down
5 changes: 0 additions & 5 deletions handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,4 @@ public OpenSslClientContext(File trustCertCollectionFile, TrustManagerFactory tr
public OpenSslSessionContext sessionContext() {
return sessionContext;
}

@Override
OpenSslKeyMaterialManager keyMaterialManager() {
return null;
}
}
28 changes: 8 additions & 20 deletions handler/src/main/java/io/netty/handler/ssl/OpenSslKeyMaterialManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -115,7 +104,6 @@ void setKeyMaterialClientSide(ReferenceCountedOpenSslEngine engine, long certOut
}
}
}

private String chooseClientAlias(ReferenceCountedOpenSslEngine engine,
String[] keyTypes, X500Principal[] issuer) {
X509KeyManager manager = provider.keyManager();
Expand Down
11 changes: 1 addition & 10 deletions handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,7 +36,6 @@
*/
public final class OpenSslServerContext extends OpenSslContext {
private final OpenSslServerSessionContext sessionContext;
private final OpenSslKeyMaterialManager keyMaterialManager;

/**
* Creates a new instance.
Expand Down Expand Up @@ -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) {
Expand All @@ -365,9 +361,4 @@ private OpenSslServerContext(
public OpenSslServerSessionContext sessionContext() {
return sessionContext;
}

@Override
OpenSslKeyMaterialManager keyMaterialManager() {
return keyMaterialManager;
}
}
32 changes: 15 additions & 17 deletions handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -68,11 +69,6 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted
}
}

@Override
OpenSslKeyMaterialManager keyMaterialManager() {
return null;
}

@Override
public OpenSslSessionContext sessionContext() {
return sessionContext;
Expand Down Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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<String> keyTypesSet = supportedClientKeyTypes(keyTypeBytes);
Expand All @@ -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");
Expand All @@ -281,6 +276,9 @@ public void requested(
* {@code X509ExtendedKeyManager.chooseEngineClientAlias}.
*/
private static Set<String> supportedClientKeyTypes(byte[] clientCertificateTypes) {
if (clientCertificateTypes == null) {
return Collections.emptySet();
}
Set<String> result = new HashSet<String>(clientCertificateTypes.length);
for (byte keyTypeCode : clientCertificateTypes) {
String keyType = clientKeyType(keyTypeCode);
Expand All @@ -296,15 +294,15 @@ private static Set<String> 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;
Expand Down
2 changes: 0 additions & 2 deletions handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down

0 comments on commit 239dc86

Please sign in to comment.