diff --git a/nifty-core/src/main/java/com/facebook/nifty/core/NiftyNoOpSecurityFactory.java b/nifty-core/src/main/java/com/facebook/nifty/core/NiftyNoOpSecurityFactory.java index abf3c33..52c77b3 100644 --- a/nifty-core/src/main/java/com/facebook/nifty/core/NiftyNoOpSecurityFactory.java +++ b/nifty-core/src/main/java/com/facebook/nifty/core/NiftyNoOpSecurityFactory.java @@ -22,7 +22,7 @@ public class NiftyNoOpSecurityFactory implements NiftySecurityFactory { - public static final ChannelHandler noOpHandler = new SimpleChannelHandler() { + static final ChannelHandler noOpHandler = new SimpleChannelHandler() { @Override public void channelOpen(ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception { diff --git a/nifty-core/src/main/java/com/facebook/nifty/ssl/JavaSslServerConfiguration.java b/nifty-core/src/main/java/com/facebook/nifty/ssl/JavaSslServerConfiguration.java index 5f47c63..3497249 100644 --- a/nifty-core/src/main/java/com/facebook/nifty/ssl/JavaSslServerConfiguration.java +++ b/nifty-core/src/main/java/com/facebook/nifty/ssl/JavaSslServerConfiguration.java @@ -20,11 +20,7 @@ import org.jboss.netty.handler.ssl.SslHandler; import org.jboss.netty.handler.ssl.SslProvider; -import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.security.cert.X509Certificate; public class JavaSslServerConfiguration extends SslServerConfiguration { @@ -69,21 +65,4 @@ public SslHandler newHandler() { throw Throwables.propagate(e); } } - - @Override - public SslSession getSession(SSLEngine engine) throws SSLException { - SSLSession session = engine.getSession(); - String cipher = session.getCipherSuite(); - long establishedTime = session.getCreationTime(); - X509Certificate peerCert = null; - try { - X509Certificate[] certs = session.getPeerCertificateChain(); - peerCert = certs[0]; - } catch (SSLPeerUnverifiedException e) { - // The peer might not have presented a certificate, in which case we consider them - // to be an unauthenticated peer. - } - String version = session.getProtocol(); - return new SslSession(null, null, version, cipher, establishedTime, peerCert); - } } diff --git a/nifty-core/src/main/java/com/facebook/nifty/ssl/SslServerConfiguration.java b/nifty-core/src/main/java/com/facebook/nifty/ssl/SslServerConfiguration.java index d830826..7678016 100644 --- a/nifty-core/src/main/java/com/facebook/nifty/ssl/SslServerConfiguration.java +++ b/nifty-core/src/main/java/com/facebook/nifty/ssl/SslServerConfiguration.java @@ -18,8 +18,6 @@ import com.google.common.base.Preconditions; import org.jboss.netty.handler.ssl.SslHandler; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLException; import java.io.File; public abstract class SslServerConfiguration { @@ -91,6 +89,4 @@ protected final void initializeServerContext() { public SslHandler createHandler() throws Exception { return serverContext.newHandler(); } - - public abstract SslSession getSession(SSLEngine engine) throws SSLException; } diff --git a/nifty-core/src/main/java/com/facebook/nifty/ssl/SslSession.java b/nifty-core/src/main/java/com/facebook/nifty/ssl/SslSession.java deleted file mode 100644 index 17215f5..0000000 --- a/nifty-core/src/main/java/com/facebook/nifty/ssl/SslSession.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright (C) 2012-2013 Facebook, Inc. - * - * Licensed 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 com.facebook.nifty.ssl; - -import javax.security.cert.X509Certificate; - -public class SslSession { - public final String alpn; - public final String npn; - public final String version; - public final String cipher; - // Time at which the connection was established since epoch in seconds. - public final long establishedTime; - public final X509Certificate peerCert; - - public SslSession( - String alpn, - String npn, - String version, - String cipher, - long establishedTime, - X509Certificate peerCert) { - this.alpn = alpn; - this.npn = npn; - this.version = version; - this.cipher = cipher; - this.establishedTime = establishedTime; - this.peerCert = peerCert; - } -} diff --git a/nifty-examples/src/test/java/com/facebook/nifty/server/TestNiftyOpenSslServer.java b/nifty-examples/src/test/java/com/facebook/nifty/server/TestNiftyOpenSslServer.java index c1695b8..b4caade 100644 --- a/nifty-examples/src/test/java/com/facebook/nifty/server/TestNiftyOpenSslServer.java +++ b/nifty-examples/src/test/java/com/facebook/nifty/server/TestNiftyOpenSslServer.java @@ -20,7 +20,10 @@ import com.facebook.nifty.client.NiftyClient; import com.facebook.nifty.client.TNiftyClientChannelTransport; import com.facebook.nifty.core.*; -import com.facebook.nifty.ssl.*; +import com.facebook.nifty.ssl.OpenSslServerConfiguration; +import com.facebook.nifty.ssl.SslClientConfiguration; +import com.facebook.nifty.ssl.TransportAttachObserver; +import com.facebook.nifty.ssl.SslServerConfiguration; import com.facebook.nifty.test.LogEntry; import com.facebook.nifty.test.ResultCode; import com.facebook.nifty.test.scribe; @@ -32,7 +35,6 @@ import org.apache.thrift.transport.TTransport; import org.apache.thrift.transport.TTransportException; import org.apache.tomcat.jni.SessionTicketKey; -import org.jboss.netty.channel.*; import org.jboss.netty.channel.group.DefaultChannelGroup; import org.jboss.netty.handler.ssl.SslHandler; import org.testng.Assert; @@ -43,11 +45,13 @@ import javax.net.ssl.*; import java.io.File; import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Field; import java.net.InetSocketAddress; import java.net.Socket; import java.security.*; +import java.security.cert.CertificateException; import java.util.Arrays; import java.util.List; @@ -221,65 +225,21 @@ private void startClientWithCerts() { } } - private SslSession[] addAuthentication(ThriftServerDefBuilder builder, SslServerConfiguration configuration) { - final SslSession[] sslSession = new SslSession[1]; - builder.withSecurityFactory(new NiftySecurityFactory() { - @Override - public NiftySecurityHandlers getSecurityHandlers(ThriftServerDef def, NettyServerConfig serverConfig) { - return new NiftySecurityHandlers() { - @Override - public ChannelHandler getAuthenticationHandler() { - return new SimpleChannelHandler() { - @Override - public void channelOpen(ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception { - super.channelOpen(ctx, e); - SslHandler handler = (SslHandler) ctx.getPipeline().get("ssl"); - handler.handshake().addListener(new ChannelFutureListener() { - @Override - public void operationComplete(ChannelFuture future) throws Exception { - synchronized (TestNiftyOpenSslServer.this) { - sslSession[0] = configuration.getSession(handler.getEngine()); - TestNiftyOpenSslServer.this.notify(); - } - } - }); - ctx.getPipeline().remove(this); - } - }; - } - - @Override - public ChannelHandler getEncryptionHandler() { - return NiftyNoOpSecurityFactory.noOpHandler; - } - }; - } - }); - return sslSession; - } - @Test - public void testDefaultServerWithClientCert() throws InterruptedException { + public void testDefaultServerWithClientCert() { SslServerConfiguration serverConfig = OpenSslServerConfiguration.newBuilder() .certFile(new File(Plain.class.getResource("/rsa.crt").getFile())) .keyFile(new File(Plain.class.getResource("/rsa.key").getFile())) .allowPlaintext(false) .clientCAFile(new File(Plain.class.getResource("/rsa.crt").getFile())) .build(); - ThriftServerDefBuilder builder = getThriftServerDefBuilder(serverConfig, null); - SslSession[] session = addAuthentication(builder, serverConfig); - startServer(builder); + + startServer(getThriftServerDefBuilder(serverConfig, null)); startClientWithCerts(); - synchronized (this) { - if (session[0] == null) { - wait(100); - } - } - Assert.assertEquals(session[0].peerCert.getSubjectDN().toString(), "CN=RSA, OU=RSA, O=RSA, L=Default City, C=XX"); } @Test - public void testClientAuthenticatingServer() throws InterruptedException { + public void testClientAuthenticatingServer() { SslServerConfiguration serverConfig = OpenSslServerConfiguration.newBuilder() .certFile(new File(Plain.class.getResource("/rsa.crt").getFile())) .keyFile(new File(Plain.class.getResource("/rsa.key").getFile())) @@ -288,17 +248,8 @@ public void testClientAuthenticatingServer() throws InterruptedException { .clientCAFile(new File(Plain.class.getResource("/rsa.crt").getFile())) .build(); - ThriftServerDefBuilder builder = getThriftServerDefBuilder(serverConfig, null); - SslSession[] session = addAuthentication(builder, serverConfig); - startServer(builder); + startServer(getThriftServerDefBuilder(serverConfig, null)); startClientWithCerts(); - // Waits for max of 100ms for the server thread to process the cert - synchronized (this) { - if (session[0] == null) { - wait(100); - } - } - Assert.assertEquals(session[0].peerCert.getSubjectDN().toString(), "CN=RSA, OU=RSA, O=RSA, L=Default City, C=XX"); } @Test(expectedExceptions = TTransportException.class) diff --git a/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslServerConfiguration.java b/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslServerConfiguration.java index 1f998b6..ba57ad8 100644 --- a/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslServerConfiguration.java +++ b/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslServerConfiguration.java @@ -20,7 +20,6 @@ import org.apache.tomcat.jni.SSL; import org.apache.tomcat.jni.SessionTicketKey; -import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; import java.io.File; @@ -169,9 +168,4 @@ protected SslHandlerFactory createSslHandlerFactory() { throw Throwables.propagate(e); } } - - @Override - public SslSession getSession(SSLEngine engine) throws SSLException { - return OpenSslSessionHelper.getSession(engine); - } } diff --git a/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslSessionHelper.java b/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslSessionHelper.java deleted file mode 100644 index 4ecee08..0000000 --- a/nifty-ssl/src/main/java/com/facebook/nifty/ssl/OpenSslSessionHelper.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright (C) 2012-2013 Facebook, Inc. - * - * Licensed 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 com.facebook.nifty.ssl; - -import org.apache.tomcat.jni.SSL; -import org.jboss.netty.handler.ssl.OpenSslEngine; - -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLException; -import javax.net.ssl.SSLSession; -import javax.security.cert.CertificateException; -import javax.security.cert.X509Certificate; -import java.lang.reflect.Field; - -/** - * This class provides a method to extract properties of the SSL session - * from an engine. - * Netty's OpenSSL engine class does not implement getSession() fully, thus - * we have to extract the properties that we need ourselves. - */ -public class OpenSslSessionHelper { - private static Field sslField; - - static { - try { - sslField = OpenSslEngine.class.getDeclaredField("ssl"); - sslField.setAccessible(true); - } - catch (Throwable t) { - // Ignore. - } - } - - public static SslSession getSession(SSLEngine sslEngine) throws SSLException { - if (!(sslEngine instanceof OpenSslEngine)) { - throw new IllegalArgumentException("ssl engine not openssl engine"); - } - OpenSslEngine engine = (OpenSslEngine) sslEngine; - if (sslField == null) { - throw new SSLException("SSL field is null"); - } - try { - long sslPtr = (long) sslField.get(engine); - if (sslPtr == 0) { - throw new SSLException("SSL not initialized"); - } - String alpn = SSL.getAlpnSelected(sslPtr); - String npn = SSL.getNextProtoNegotiated(sslPtr); - - String version = SSL.getVersion(sslPtr); - String cipher = SSL.getCipherForSSL(sslPtr); - long establishedTime = SSL.getTime(sslPtr); - - // TODO: return the entire chain. - // tc-native thinks that the chain is null, so we supply only the - // leaf cert. - byte[] cert = SSL.getPeerCertificate(sslPtr); - X509Certificate certificate = null; - if (cert != null) { - certificate = X509Certificate.getInstance(cert); - } - return new SslSession(alpn, npn, version, cipher, establishedTime, certificate); - } - catch (IllegalAccessException e) { - throw new SSLException(e); - } - catch (CertificateException e) { - throw new SSLException(e); - } - } -}