DIRMINA-1122 - added support for endpoint identification algorithm

[the-thing]

https://issues.apache.org/jira/projects/DIRMINA/issues/DIRMINA-1122

We found out that it is not possible to set endpoint identification algorithm while discussing SNI support in quickfixj https://github.com/quickfix-j/quickfixj/issues/276

Changes

• added support for endpoint identification algorithm to SslFilter
• unit tests for SNI CN/SAN certificate matching

[the-thing]

Hi. Is there any chance of including this in the next release? Would attaching a patch to the JIRA speed things up (as mentioned in https://github.com/apache/mina/pull/25)?

[jon-valliere]

Yes, patches can be accelerated. DIRMINA-1122 is currently a feature request and there is no one available to work on any features at the moment. I am, however, available to review and apply patch submissions.

[the-thing]

Attached the patch to JIRA. Hope this helps.

[elecharny]

Hmmm, the patch has been merged, but the tests are failing: the positive tests are OK, but the negative ones aren't. Also the TLS protocol is not anymore supported by Java 8, I had to switch to TLSv1.2. I'm investigating what could go wrong in MINA 2.1.X and 2.2.X branches, as the implementation is different.

[the-thing]

Interesting. I can produce a patch quickly if I can recreate the failure. If you could share some of the error logs or environment details it would be helpful.

[elecharny]

Actually, all the handshakes are passing, which make the shouldFailXXX tests failing.

Here is the modified test class for MINA 2.2.X:

package org.apache.mina.filter.ssl;

import org.apache.mina.core.filterchain.DefaultIoFilterChainBuilder;
import org.apache.mina.core.filterchain.IoFilterChain;
import org.apache.mina.core.service.IoHandlerAdapter;
import org.apache.mina.core.session.IoSession;
import org.apache.mina.filter.FilterEvent;
import org.apache.mina.filter.codec.ProtocolCodecFilter;
import org.apache.mina.filter.codec.textline.TextLineCodecFactory;
import org.apache.mina.transport.socket.nio.NioSocketAcceptor;
import org.apache.mina.transport.socket.nio.NioSocketConnector;
import org.apache.mina.util.AvailablePortFinder;
import org.junit.Before;
import org.junit.Test;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import javax.net.ssl.TrustManagerFactory;
import java.net.InetSocketAddress;
import java.security.KeyStore;
import java.security.Security;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

/**
 * Test SNI matching scenarios. (tests for DIRMINA-1122)
 *
 * <pre>
 * emptykeystore.sslTest        - empty keystore
 * server-cn.keystore           - keystore with single certificate chain  (CN=mina)
 * client-cn.truststore         - keystore with trusted certificate
 * server-san-ext.keystore      - keystore with single certificate chain (CN=mina;SAN=*.bbb.ccc,xxx.yyy)
 * client-san-ext.truststore    - keystore with trusted certificate
 * </pre>
 */
public class SslIdentificationAlgorithmTest {

    private static final String KEY_MANAGER_FACTORY_ALGORITHM;

    static {
        String algorithm = Security.getProperty("ssl.KeyManagerFactory.algorithm");
        if (algorithm == null) {
            algorithm = KeyManagerFactory.getDefaultAlgorithm();
        }

        KEY_MANAGER_FACTORY_ALGORITHM = algorithm;
    }

    private int port;
    private CountDownLatch handshakeDone;

    @Before
    public void setUp() {
        port = AvailablePortFinder.getNextAvailable(5555);
        handshakeDone = new CountDownLatch(2);
    }

    @Test
    public void shouldAuthenticateWhenServerCertificateCommonNameMatchesClientSNI() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-cn.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-cn.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "mina");

        assertTrue(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    @Test
    public void shouldFailAuthenticationWhenServerCertificateCommonNameDoesNotMatchClientSNI() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-cn.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-cn.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "example.com");

        assertFalse(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    @Test
    public void shouldFailAuthenticationWhenClientMissingSNIAndIdentificationAlgorithmProvided() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-cn.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-cn.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, null);

        assertFalse(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    /**
     * Subject Alternative Name (SAN) scenarios
     */
    @Test
    public void shouldAuthenticateWhenServerCertificateAlternativeNameMatchesClientSNIExactly() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-san-ext.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-san-ext.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "xxx.yyy");

        assertTrue(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    @Test
    public void shouldAuthenticateWhenServerCertificateAlternativeNameMatchesClientSNIViaWildcard() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-san-ext.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-san-ext.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "aaa.bbb.ccc");

        assertTrue(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    @Test
    public void shouldFailAuthenticationWhenServerCommonNameMatchesSNIAndSNINotInAlternativeName() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-san-ext.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-san-ext.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "mina");

        assertFalse(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    @Test
    public void shouldFailAuthenticationWhenMatchingAlternativeNameWildcardExactly() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-san-ext.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-san-ext.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "*.bbb.ccc");

        assertFalse(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    @Test
    public void shouldFailAuthenticationWhenMatchingAlternativeNameWithTooManyLabels() throws Exception {
        SSLContext acceptorContext = createSSLContext("server-san-ext.keystore", "emptykeystore.sslTest");
        SSLContext connectorContext = createSSLContext("emptykeystore.sslTest", "client-san-ext.truststore");

        startAcceptor(acceptorContext);
        startConnector(connectorContext, "mmm.nnn.bbb.ccc");

        assertFalse(handshakeDone.await(10, TimeUnit.SECONDS));
    }

    private void startAcceptor(SSLContext sslContext) throws Exception {
        NioSocketAcceptor acceptor = new NioSocketAcceptor();
        acceptor.setReuseAddress(true);

        SslFilter sslFilter = new SslFilter(sslContext);
        sslFilter.setEnabledProtocols(new String[] {"TLSv1.2"});

        DefaultIoFilterChainBuilder filters = acceptor.getFilterChain();
        filters.addLast("ssl", sslFilter);
        filters.addLast("text", new ProtocolCodecFilter(new TextLineCodecFactory()));

        acceptor.setHandler(new IoHandlerAdapter() {

            @Override
            public void sessionOpened(IoSession session) {
                session.write("acceptor write");
            }

            @Override
            public void event(IoSession session, FilterEvent event) {
                if (event == SslEvent.SECURED) {
                    handshakeDone.countDown();
                }
            }
        });

        acceptor.bind(new InetSocketAddress(port));
    }

    private void startConnector(SSLContext sslContext, final String sni) {
        NioSocketConnector connector = new NioSocketConnector();

        SslFilter sslFilter = new SslFilter(sslContext) {

            @Override
            public void onPreAdd(IoFilterChain parent, String name, NextFilter nextFilter) throws SSLException {
                if (sni != null) {
                    IoSession session = parent.getSession();
                    session.setAttribute(SslFilter.PEER_ADDRESS, new InetSocketAddress(sni, port));
                }

                super.onPreAdd(parent, name, nextFilter);
            }
        };

        sslFilter.setUseClientMode(true);
        sslFilter.setEndpointIdentificationAlgorithm("HTTPS");
        sslFilter.setEnabledProtocols(new String[] {"TLSv1.2"});

        DefaultIoFilterChainBuilder filters = connector.getFilterChain();
        filters.addLast("ssl", sslFilter);
        filters.addLast("text", new ProtocolCodecFilter(new TextLineCodecFactory()));

        connector.setHandler(new IoHandlerAdapter() {

            @Override
            public void sessionOpened(IoSession session) {
                session.write("connector write");
            }

            @Override
            public void event(IoSession session, FilterEvent event) {
                if (event == SslEvent.SECURED) {
                    handshakeDone.countDown();
                }
            }
        });

        connector.connect(new InetSocketAddress("localhost", port));
    }

    private SSLContext createSSLContext(String keyStorePath, String trustStorePath) throws Exception {
        char[] password = "password".toCharArray();

        KeyStore keyStore = KeyStore.getInstance("JKS");
        keyStore.load(SslTest.class.getResourceAsStream(keyStorePath), password);

        KeyManagerFactory kmf = KeyManagerFactory.getInstance(KEY_MANAGER_FACTORY_ALGORITHM);
        kmf.init(keyStore, password);

        KeyStore trustStore = KeyStore.getInstance("JKS");
        trustStore.load(SslTest.class.getResourceAsStream(trustStorePath), password);

        TrustManagerFactory tmf = TrustManagerFactory.getInstance(KEY_MANAGER_FACTORY_ALGORITHM);
        tmf.init(trustStore);

        SSLContext ctx = SSLContext.getInstance("TLSv1.2");
        ctx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);

        return ctx;
    }
}

[elecharny]

FTR, here is a part of the logs I get when running the test with Java8:

javax.net.ssl|FINE|0F|CLIENT|2023-05-08 22:21:32.416 CEST|SSLExtensions.java:173|Ignore unavailable extension: supported_versions
javax.net.ssl|FINE|0F|CLIENT|2023-05-08 22:21:32.417 CEST|ServerHello.java:955|Negotiated protocol version: TLSv1.2
javax.net.ssl|FINE|0F|CLIENT|2023-05-08 22:21:32.417 CEST|SSLExtensions.java:192|Consumed extension: renegotiation_info
javax.net.ssl|FINE|0F|CLIENT|2023-05-08 22:21:32.417 CEST|SSLExtensions.java:173|Ignore unavailable extension: server_name
...

With Java 11:

javax.net.ssl|DEBUG|0F|NioProcessor-2|2023-05-08 22:36:09.415 CEST|SSLExtensions.java:192|Consumed extension: supported_versions
javax.net.ssl|DEBUG|0F|NioProcessor-2|2023-05-08 22:36:09.415 CEST|ClientHello.java:839|Negotiated protocol version: TLSv1.2
javax.net.ssl|DEBUG|0F|NioProcessor-2|2023-05-08 22:36:09.416 CEST|ServerNameExtension.java:327|no server name matchers, ignore server name indication
...

So it seems there is a missing a SNIMatcher instance.

[the-thing]

I applied the changes manually to 2.2.X. The peerAddress session attribute has been removed, which was used to get the hostname for the SSL engine.

2.1.X - org.apache.mina.filter.ssl.SslHandler#init

InetSocketAddress peer = (InetSocketAddress) session.getAttribute(SslFilter.PEER_ADDRESS);

// Create the SSL engine here
if (peer == null) {
    sslEngine = sslFilter.sslContext.createSSLEngine();
} else {
    sslEngine = sslFilter.sslContext.createSSLEngine(peer.getHostName(), peer.getPort());
}

in 2.2.X - org.apache.mina.filter.ssl.SslFilter#createEngine

SSLEngine sslEngine = (addr != null) ? sslContext.createSSLEngine(addr.getHostString(), addr.getPort()) : sslContext.createSSLEngine();

#getHostString seems to be the problem as it is resolved IP address instead the actual host.

[elecharny]

On the test, we use a custom SSLFilter which sets the peer:

        protected SSLEngine createEngine(IoSession session, InetSocketAddress addr) {
            //Add your SNI host name and port in the IOSession
            String sniHostNames = (String)session.getAttribute( "SNIHostNames" );
            int portNumber = (int)session.getAttribute( "PortNumber");
            InetSocketAddress peer = InetSocketAddress.createUnresolved( sniHostNames, portNumber);
            
            SSLEngine sslEngine = (addr != null) ?
                sslContext.createSSLEngine(peer.getHostString(), peer.getPort()) : sslContext.createSSLEngine();
...

The default SslFilter.createEngine() never get called when initializing the Connector.

It's called only for the Acceptor.

[elecharny]

Note that MINA 2.2.X don't have anymore a PEER_ADDRESS attribute, so we have to go through the creation of a dedicated SslFilter class, which extends the default SslFilter class. Not necessary easier, but should do the trick.

[elecharny]

Also tested something: using the sniHostNames instead of doing a peer.getHostString(), changes nothing...

[the-thing]

I was able to create 2 branches based of 2.2.X and they both work (still waiting for the CI to run).

1. The old method - by providing the peer address.

https://github.com/the-thing/mina/tree/ssl_endpoint_algorithm

Probably not desired.

2. Without providing the peer address - requires additional serverNames parameter

https://github.com/the-thing/mina/tree/ssl_endpoint_2

The problem with this method is that the actual server peer address will not be automatically verified and will have to be passed as a "serverName" separately.

3. There is also another fix that doesn't require any of the above. The unit tests uses "localhost" as a host name so I would have to recreate the keystores with appropriate hostname patterns. I assume this should work, but haven't tried it yet.

[elecharny]

Hi,

I confirm the first branch (ssl_endpoint_algorithm) works. It mimics MINA_2.1.X, using a PEER attribute. I was also able to have the tests passing with the original code, by changing the way we pass the peer hostname and port. The key was to use peer.getHostName() instead of _peer.getHostString()

[elecharny]

Ok, I pushed the modified code with the working tests!

[the-thing]

Good stuff. I had a peek and I can see that the solution requires custom ssl engine creation. Drop me a message if any trouble.

[elecharny]

yes, I followed John's advice and used the new pattern (ie develop a custom SSLFilter inheriting the base SSLFilter to pass a new attribute). We may discuss this pattern though, because it's pretty heavy, IMHO...

[chrjohn]

Hi @elecharny , is there any ETA for the next 2.2.x release? Thanks, Chris.

[elecharny]

I think we can cut it ASAP. It's a matter of 3/4 days.

[chrjohn]

Could you please also check if https://github.com/apache/mina/pull/35 could be integrated into 2.2.x then. Thanks in advance.

[elecharny]

Will do this week-end!

[elecharny]

Pushed in 2.2.X