Secure DICOM Listener blocking during handshake

[Mike Ross]

Hi all,

I have a scenario where my dcm4che TCPListener blocks during the SSL handshake and becomes unresponsive.

I am able to reproduce the issue if I simply telnet to my listening DICOM endpoint.  As long as I keep that telnet session open, I am no longer able to establish a new association from other DICOM endpoints.  Once the telnet session closes, the TCPListener is no longer blocked, and I can successfully establish associations again.

Here is the stack trace that demonstrates the blocking handshake call:

"pool-24-thread-1@12954" prio=5 tid=0xde nid=NA runnable
  java.lang.Thread.State: RUNNABLE
 at java.net.SocketInputStream.socketRead0(SocketInputStream.java:-1)
 at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
 at java.net.SocketInputStream.read(SocketInputStream.java:171)
 at java.net.SocketInputStream.read(SocketInputStream.java:141)
 at sun.security.ssl.InputRecord.readFully(InputRecord.java:465)
 at sun.security.ssl.InputRecord.read(InputRecord.java:503)
 at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:983)
 - locked <0x3312> (a java.lang.Object)
 at sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
 - locked <0x3313> (a java.lang.Object)
 at sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
 at sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
 at org.dcm4che3.net.TCPListener.listen(TCPListener.java:112)
 at org.dcm4che3.net.TCPListener.access$000(TCPListener.java:56)
 at org.dcm4che3.net.TCPListener$1.run(TCPListener.java:74)
 at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
 at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
 at java.lang.Thread.run(Thread.java:748)

I am using dcm4che version 5.19.1.  Has anyone else experienced this?

Thanks,

Mike

[Mike Ross]

Also, I am using Java 8_162.

[Mike Ross]

If I change org.dcm4che3.net.TCPListener, I can prevent the behavior of blocking indefinitely by doing the following:

1. Perform the handshake in its own thread as to not block the main thread from listening for new connections.

2. Set the SO Timeout prior to performing the handshake so that we don't hang indefinitely if no bytes are received.

Is this something that we could submit a patch for?  Here is my modified class:

/* ***** BEGIN LICENSE BLOCK *****
 * Version: MPL 1.1/GPL 2.0/LGPL 2.1
 *
 * The contents of this file are subject to the Mozilla Public License Version
 * 1.1 (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.mozilla.org/MPL/
 *
 * Software distributed under the License is distributed on an "AS IS" basis,
 * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
 * for the specific language governing rights and limitations under the
 * License.
 *
 * The Original Code is part of dcm4che, an implementation of DICOM(TM) in
 * Java(TM), hosted at https://github.com/dcm4che.
 *
 * The Initial Developer of the Original Code is
 * Agfa Healthcare.
 * Portions created by the Initial Developer are Copyright (C) 2013
 * the Initial Developer. All Rights Reserved.
 *
 * Contributor(s):
 * See @authors listed below
 *
 * Alternatively, the contents of this file may be used under the terms of
 * either the GNU General Public License Version 2 or later (the "GPL"), or
 * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
 * in which case the provisions of the GPL or the LGPL are applicable instead
 * of those above. If you wish to allow use of your version of this file only
 * under the terms of either the GPL or the LGPL, and not to allow others to
 * use your version of this file under the terms of the MPL, indicate your
 * decision by deleting the provisions above and replace them with the notice
 * and other provisions required by the GPL or the LGPL. If you do not delete
 * the provisions above, a recipient may use your version of this file under
 * the terms of any one of the MPL, the GPL or the LGPL.
 *
 * ***** END LICENSE BLOCK ***** */

package org.dcm4che3.net;

import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLServerSocketFactory;
import javax.net.ssl.SSLSocket;
import java.io.IOException;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketAddress;
import java.security.GeneralSecurityException;

/**
 * @author Gunter Zeilinger <gunterze@gmail.com>
 *
 */
class TCPListener implements Listener {

    private final Connection conn;
    private final TCPProtocolHandler handler;
    private final ServerSocket ss;

    public TCPListener(Connection conn, TCPProtocolHandler handler)
            throws IOException, GeneralSecurityException {
        try {

            this.conn = conn;
            this.handler = handler;
            ss = conn.isTls() ? createTLSServerSocket(conn) : new ServerSocket();
            conn.setReceiveBufferSize(ss);
            ss.bind(conn.getBindPoint(), conn.getBacklog());
            conn.getDevice().execute(new Runnable() {
                @Override
                public void run() {
                    listen();
                }
            });

        } catch (IOException e) {
            throw new IOException("Unable to start TCPListener on "+conn.getHostname()+":"+conn.getPort(), e);
        }
    }

    private ServerSocket createTLSServerSocket(Connection conn)
            throws IOException, GeneralSecurityException {
        SSLContext sslContext = conn.getDevice().sslContext();
        SSLServerSocketFactory ssf = sslContext.getServerSocketFactory();
        SSLServerSocket ss = (SSLServerSocket) ssf.createServerSocket();
        ss.setEnabledProtocols(conn.getTlsProtocols());
        ss.setEnabledCipherSuites(conn.getTlsCipherSuites());
        ss.setNeedClientAuth(conn.isTlsNeedClientAuth());
        return ss;
    }

    private void listen() {
        SocketAddress sockAddr = ss.getLocalSocketAddress();
        Connection.LOG.info("Start TCP Listener on {}", sockAddr);
        try {
            while (!ss.isClosed()) {
                Connection.LOG.debug("Wait for connection on {}", sockAddr);
                Socket s = ss.accept();
                final ConnectionMonitor monitor = conn.getDevice() != null
                        ? conn.getDevice().getConnectionMonitor()
                        : null;
                if (conn.isBlackListed(s.getInetAddress())) {
                    if (monitor != null) {
                        monitor.onConnectionRejectedBlacklisted(conn, s);
                    }
                    Connection.LOG.info("Reject blacklisted connection {}", s);
                    conn.close(s);
                } else {
                    // create new runnable to handle the handshake so that the main thread doesn't get stuck waiting for a TLS handshake
                    final Runnable handshakeRunnable = new Runnable() {
                        @Override
                        public void run() {
                            boolean successful = true;
                            try {
                                conn.setSocketSendOptions(s);
                                if (s instanceof SSLSocket) {
                                    int originalSoTimeout = s.getSoTimeout();
                                    s.setSoTimeout(30_000);
                                    ((SSLSocket) s).startHandshake();
                                    s.setSoTimeout(originalSoTimeout);
                                }
                            } catch (IOException e) {
                                successful = false;
                                if (monitor != null) {
                                    monitor.onConnectionRejected(conn, s, e);
                                }
                                Connection.LOG.warn("Reject connection {}:",s, e);
                                conn.close(s);
                            }

                            if (successful) {
                                if (monitor != null) {
                                    monitor.onConnectionAccepted(conn, s);
                                }
                                Connection.LOG.info("Accept connection {}", s);
                                try {
                                    handler.onAccept(conn, s);
                                } catch (IOException e) {
                                    Connection.LOG.warn("Exception on accepted connection {}:", s, e);
                                    conn.close(s);
                                }
                            }
                        }
                    };
                    // execute TLS handshake using new runnable
                    final Thread handshakeThread = new Thread(handshakeRunnable, "Secure Socket Handshake Thread");
                    handshakeThread.start();
                }
            }
        } catch (IOException e) {
            if (!ss.isClosed()) { // ignore exception caused by close()
                Connection.LOG.error("Exception on listing on {}:", sockAddr, e);
            }
        }
        Connection.LOG.info("Stop TCP Listener on {}", sockAddr);
    }


    @Override
    public SocketAddress getEndPoint() {
        return ss.getLocalSocketAddress();
    }

    @Override
    public void close() throws IOException {
        try {
            ss.close();
        } catch (IOException e) {
            Connection.LOG.info("Exception when closing socket:", e);
            // Ignore errors when closing the server socket.
        }
    }
}