From 350a806f1e91bf9f177cd76c02f319198d9138a3 Mon Sep 17 00:00:00 2001 From: Vladimir Lagunov Date: Mon, 15 Jul 2019 16:11:15 +0700 Subject: [PATCH] Netty factory: Close TCP connection when receive TLS close notify TLS has special TLS Alert message "close_notify". After receiving such packet any conversation should be immediately stopped. Some TLS implementations (OpenSSL for example) closes underlying TCP connection when TLS connection being closed, some (Docker SSL server and docker-java before current commit) does not close. AttachContainerCmd executes `onComplete` callback only when TCP connection closes, so without this patch `onComplete` called after a long timeout, with this patch it called when container exits. --- .../netty/NettyDockerCmdExecFactory.java | 26 +++++++- .../dockerjava/cmd/AttachContainerCmdIT.java | 66 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java b/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java index dfc740e35..e7aa7e790 100644 --- a/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java +++ b/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java @@ -15,6 +15,8 @@ import com.github.dockerjava.core.AbstractDockerCmdExecFactory; import com.github.dockerjava.core.WebTarget; +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.GenericFutureListener; import org.apache.commons.lang.SystemUtils; import com.github.dockerjava.api.command.DockerCmdExecFactory; @@ -232,12 +234,34 @@ public DuplexChannel connect(Bootstrap bootstrap) throws InterruptedException { throw new RuntimeException("no port configured for " + host); } - DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel(); + final DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel(); final SslHandler ssl = initSsl(dockerClientConfig); if (ssl != null) { channel.pipeline().addFirst(ssl); + + // https://tools.ietf.org/html/rfc5246#section-7.2.1 + // TLS has its own special message about connection termination. Because TLS is a + // session-level protocol, it can be covered by any transport-level protocol like + // TCP, UTP and so on. But we know exactly that data being transferred over TCP and + // that other side will never send any byte into this TCP connection, so this + // channel should be closed. + // RFC says that we must notify opposite side about closing. This could be done only + // in sun.security.ssl.SSLEngineImpl and unfortunately it does not send this + // message. On the other hand RFC does not enforce the opposite side to wait for + // such message. + ssl.sslCloseFuture().addListener(new GenericFutureListener>() { + @Override + public void operationComplete(Future future) { + channel.eventLoop().execute(new Runnable() { + @Override + public void run() { + channel.close(); + } + }); + } + }); } return channel; diff --git a/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java b/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java index bac45f63f..caaadde16 100644 --- a/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java +++ b/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java @@ -13,11 +13,14 @@ import org.slf4j.LoggerFactory; import java.io.ByteArrayInputStream; +import java.io.Closeable; import java.io.File; import java.io.InputStream; import java.io.PipedInputStream; import java.io.PipedOutputStream; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import static com.github.dockerjava.junit.DockerRule.DEFAULT_IMAGE; import static java.util.concurrent.TimeUnit.SECONDS; @@ -28,6 +31,7 @@ import static org.hamcrest.Matchers.isEmptyString; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeThat; /** @@ -205,6 +209,68 @@ public void onNext(Frame frame) { callback.close(); } + /** + * {@link AttachContainerResultCallback#onComplete()} should be called immediately after + * container exit. It was broken for Netty and TLS connection. + */ + @Test + public void attachContainerClosesStdoutWhenContainerExits() throws Exception { + DockerClient dockerClient = dockerRule.getClient(); + + CreateContainerResponse container = dockerClient.createContainerCmd(DEFAULT_IMAGE) + .withCmd("echo", "hello world") + .withTty(false) + .exec(); + LOG.info("Created container: {}", container.toString()); + + final CountDownLatch started = new CountDownLatch(1); + final AtomicLong startedAtNanos = new AtomicLong(); + final CountDownLatch gotLine = new CountDownLatch(1); + final CountDownLatch completed = new CountDownLatch(1); + final AtomicLong gotLineAtNanos = new AtomicLong(); + AttachContainerTestCallback callback = new AttachContainerTestCallback() { + @Override + public void onStart(Closeable stream) { + startedAtNanos.set(System.nanoTime()); + started.countDown(); + super.onStart(stream); + } + + @Override + public void onNext(Frame item) { + if (item.getStreamType() == StreamType.STDOUT) { + gotLineAtNanos.set(System.nanoTime()); + gotLine.countDown(); + } + super.onNext(item); + } + + @Override + public void onComplete() { + completed.countDown(); + super.onComplete(); + } + }; + + try (Closeable ignored = callback) { + dockerClient.attachContainerCmd(container.getId()) + .withStdOut(true) + .withFollowStream(true) + .exec(callback); + + dockerClient.startContainerCmd(container.getId()).exec(); + + assertTrue("Should start in a reasonable time", started.await(30, SECONDS)); + assertTrue("Should get first line quickly after the start", gotLine.await(15, SECONDS)); + + long gotLineDurationSeconds = (gotLineAtNanos.get() - startedAtNanos.get()) / 1_000_000_000L; + LOG.info("Got line from {} for {} seconds", container.getId(), gotLineDurationSeconds); + + boolean finished = completed.await(1L + gotLineDurationSeconds, SECONDS); + assertTrue("Should get EOF in a time close to time of getting the first line", finished); + } + } + public static class AttachContainerTestCallback extends AttachContainerResultCallback { private StringBuffer log = new StringBuffer();