[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user asfgit closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov reopened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. ## Fixed networking issues/bugs in UnifiedServerSocket - don't crash the `accept()` thread if the client closes the connection without sending any data - don't corrupt the connection if the client sends fewer than 5 bytes for the initial read - delay the detection of TLS vs. plaintext mode until a socket stream is read from or written to. This prevents the `accept()` thread from getting blocked on a `read()` operation from the newly connected socket. - prepending 5 bytes to `PrependableSocket` and then trying to read >5 bytes would only return the first 5 bytes, even if more bytes were available. This is fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r235070776
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov reopened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. ## Fixed networking issues/bugs in UnifiedServerSocket - don't crash the `accept()` thread if the client closes the connection without sending any data - don't corrupt the connection if the client sends fewer than 5 bytes for the initial read - delay the detection of TLS vs. plaintext mode until a socket stream is read from or written to. This prevents the `accept()` thread from getting blocked on a `read()` operation from the newly connected socket. - prepending 5 bytes to `PrependableSocket` and then trying to read >5 bytes would only return the first 5 bytes, even if more bytes were available. This is fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit 367bef0980193e2761c7008844c5e9fe029d8a66 Author: Ilya Maykov Date: 2018-10-25T01:22:24Z ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r234053630
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r234053611
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
--- End diff --
Done
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r234052921
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233668370
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233662723
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233661888
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233661917
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
--- End diff --
Thanks.
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233661570
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/PrependableSocket.java
---
@@ -18,32 +18,47 @@
package org.apache.zookeeper.server.quorum;
-import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.SequenceInputStream;
+import java.io.PushbackInputStream;
import java.net.Socket;
import java.net.SocketImpl;
public class PrependableSocket extends Socket {
- private SequenceInputStream sequenceInputStream;
+ private PushbackInputStream pushbackInputStream;
--- End diff --
Yes, SequenceInputStream likely still works in practice, but exposes the
discontinuity between the underlying streams to the caller. Using
PushbackInputStream hides it and I think is preferable.
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233661093
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/PrependableSocket.java
---
@@ -18,32 +18,47 @@
package org.apache.zookeeper.server.quorum;
-import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.SequenceInputStream;
+import java.io.PushbackInputStream;
import java.net.Socket;
import java.net.SocketImpl;
public class PrependableSocket extends Socket {
- private SequenceInputStream sequenceInputStream;
+ private PushbackInputStream pushbackInputStream;
--- End diff --
But I suspect the second read() will read from the other stream of the
Sequence. Which is okay, because client has to initiate another read call if it
hasn't received everything it needed.
Anyway I wouldn't say go back to that impl, just speculating. I believe
handling the 2 streams seamlessly is desirable.
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233657773
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233657163
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233656740
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
--- End diff --
Sure, I could do that if you prefer.
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233656201
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -350,14 +389,22 @@ public static X509TrustManager createTrustManager(
public SSLSocket createSSLSocket() throws X509Exception, IOException {
SSLSocket sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket();
configureSSLSocket(sslSocket);
-
+sslSocket.setUseClientMode(true);
return sslSocket;
}
-public SSLSocket createSSLSocket(Socket socket) throws X509Exception,
IOException {
-SSLSocket sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket(socket, null,
socket.getPort(), true);
+public SSLSocket createSSLSocket(Socket socket, byte[] pushbackBytes)
throws X509Exception, IOException {
+SSLSocket sslSocket;
+if (pushbackBytes != null && pushbackBytes.length > 0) {
+sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket(
+socket, new ByteArrayInputStream(pushbackBytes), true);
+} else {
+sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket(
+socket, null, socket.getPort(), true);
+}
configureSSLSocket(sslSocket);
-
+sslSocket.setUseClientMode(false);
--- End diff --
Yes and yes. In #681 I make the client auth setting configurable.
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233656579
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/PrependableSocket.java
---
@@ -18,32 +18,47 @@
package org.apache.zookeeper.server.quorum;
-import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.SequenceInputStream;
+import java.io.PushbackInputStream;
import java.net.Socket;
import java.net.SocketImpl;
public class PrependableSocket extends Socket {
- private SequenceInputStream sequenceInputStream;
+ private PushbackInputStream pushbackInputStream;
--- End diff --
I don't know why it worked like that, but that is what I observed in tests.
It looks like SequenceInputStream does not join returned data across boundaries
of the underlying streams when the first stream gets to EOF. I don't think this
is desired behavior since it would cause the stream to return 5 bytes when more
than 5 bytes are actually available.
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233632959
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -350,14 +389,22 @@ public static X509TrustManager createTrustManager(
public SSLSocket createSSLSocket() throws X509Exception, IOException {
SSLSocket sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket();
configureSSLSocket(sslSocket);
-
+sslSocket.setUseClientMode(true);
return sslSocket;
}
-public SSLSocket createSSLSocket(Socket socket) throws X509Exception,
IOException {
-SSLSocket sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket(socket, null,
socket.getPort(), true);
+public SSLSocket createSSLSocket(Socket socket, byte[] pushbackBytes)
throws X509Exception, IOException {
+SSLSocket sslSocket;
+if (pushbackBytes != null && pushbackBytes.length > 0) {
+sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket(
+socket, new ByteArrayInputStream(pushbackBytes), true);
+} else {
+sslSocket = (SSLSocket)
getDefaultSSLContext().getSocketFactory().createSocket(
+socket, null, socket.getPort(), true);
+}
configureSSLSocket(sslSocket);
-
+sslSocket.setUseClientMode(false);
--- End diff --
Just to double check what you changed here:
- setting the client mode explicitly on both client/server side,
- requesting client authentication in TLS mode: so without client
authentication, quorum TLS cannot be established anymore
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233645255
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
--- End diff --
Using `InetAddress.getLoopbackAddress()` ?
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233647195
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233633340
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/PrependableSocket.java
---
@@ -18,32 +18,47 @@
package org.apache.zookeeper.server.quorum;
-import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.SequenceInputStream;
+import java.io.PushbackInputStream;
import java.net.Socket;
import java.net.SocketImpl;
public class PrependableSocket extends Socket {
- private SequenceInputStream sequenceInputStream;
+ private PushbackInputStream pushbackInputStream;
--- End diff --
Please explain again what exactly was the problem with
`SequenceInputStream`?
Why did it return only the first 5 bytes after it had been prepended?
---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/679#discussion_r233643417
--- Diff:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
---
@@ -17,156 +17,644 @@
*/
package org.apache.zookeeper.server.quorum;
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.SSLSocket;
+
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.BaseX509ParameterizedTestCase;
import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.Time;
+import org.apache.zookeeper.common.KeyStoreFileType;
+import org.apache.zookeeper.common.X509Exception;
+import org.apache.zookeeper.common.X509KeyType;
+import org.apache.zookeeper.common.X509TestContext;
import org.apache.zookeeper.common.X509Util;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
-import javax.net.ssl.SSLSocket;
-import java.io.IOException;
-import java.net.ConnectException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+@RunWith(Parameterized.class)
+public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase
{
-public class UnifiedServerSocketTest {
+@Parameterized.Parameters
+public static Collection params() {
+ArrayList result = new ArrayList<>();
+int paramIndex = 0;
+for (X509KeyType caKeyType : X509KeyType.values()) {
+for (X509KeyType certKeyType : X509KeyType.values()) {
+for (Boolean hostnameVerification : new Boolean[] { true,
false }) {
+result.add(new Object[]{
+caKeyType,
+certKeyType,
+hostnameVerification,
+paramIndex++
+});
+}
+}
+}
+return result;
+}
private static final int MAX_RETRIES = 5;
private static final int TIMEOUT = 1000;
+private static final byte[] DATA_TO_CLIENT = "hello client".getBytes();
+private static final byte[] DATA_FROM_CLIENT = "hello
server".getBytes();
private X509Util x509Util;
private int port;
-private volatile boolean handshakeCompleted;
+private InetSocketAddress localServerAddress;
+private final Object handshakeCompletedLock = new Object();
+// access only inside synchronized(handshakeCompletedLock) { ... }
blocks
+private boolean handshakeCompleted = false;
+
+public UnifiedServerSocketTest(
+final X509KeyType caKeyType,
+final X509KeyType certKeyType,
+final Boolean hostnameVerification,
+final Integer paramIndex) {
+super(paramIndex, () -> {
+try {
+return X509TestContext.newBuilder()
+.setTempDir(tempDir)
+.setKeyStoreKeyType(certKeyType)
+.setTrustStoreKeyType(caKeyType)
+.setHostnameVerification(hostnameVerification)
+.build();
+} catch (Exception e) {
+throw new RuntimeException(e);
+}
+});
+}
@Before
public void setUp() throws Exception {
-handshakeCompleted = false;
-
port = PortAssignment.unique();
+localServerAddress = new InetSocketAddress("localhost", port);
-String testDataPath = System.getProperty("test.data.dir",
"build/test/data");
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
"org.apache.zookeeper.server.NettyServerCnxnFactory");
System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
"org.apache.zookeeper.ClientCnxnSocketNetty");
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov reopened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. ## Fixed networking issues/bugs in UnifiedServerSocket - don't crash the `accept()` thread if the client closes the connection without sending any data - don't corrupt the connection if the client sends fewer than 5 bytes for the initial read - delay the detection of TLS vs. plaintext mode until a socket stream is read from or written to. This prevents the `accept()` thread from getting blocked on a `read()` operation from the newly connected socket. - prepending 5 bytes to `PrependableSocket` and then trying to read >5 bytes would only return the first 5 bytes, even if more bytes were available. This is fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit 33f7aaab6fe16122b7e1faedbb408d739bbe8a30 Author: Ilya Maykov Date: 2018-10-25T01:22:24Z ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov reopened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. Note that this is stacked on top of #678 and thus includes it. Please only consider the ZOOKEEPER-3172 commit when reviewing. Once the other PR is merged upstream, I will rebase this so it only contains one commit. ## Fixed networking issues/bugs in UnifiedServerSocket - don't crash the `accept()` thread if the client closes the connection without sending any data - don't corrupt the connection if the client sends fewer than 5 bytes for the initial read - delay the detection of TLS vs. plaintext mode until a socket stream is read from or written to. This prevents the `accept()` thread from getting blocked on a `read()` operation from the newly connected socket. - prepending 5 bytes to `PrependableSocket` and then trying to read >5 bytes would only return the first 5 bytes, even if more bytes were available. This is fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit 2122c8c23a0dbb27f9b2aff55e800e48d253f943 Author: Ilya Maykov Date: 2018-10-25T00:41:48Z ZOOKEEPER-3173: Quorum TLS - support PEM trust/key stores ZOOKEEPER-3175: Quorum TLS - test improvements Add support for loading key and trust stores from PEM files. Also added test utils for testing X509-related code, because it was very difficult to untangle them from the PEM support code. commit 514d48a26aeeca37290ad14ff8f0cdae69b53eb2 Author: Ilya Maykov Date: 2018-10-25T01:22:24Z ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov reopened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. Note that this is stacked on top of #678 and thus includes it. Please only consider the ZOOKEEPER-3172 commit when reviewing. Once the other PR is merged upstream, I will rebase this so it only contains one commit. ## Fixed networking issues/bugs in UnifiedServerSocket - don't crash the `accept()` thread if the client closes the connection without sending any data - don't corrupt the connection if the client sends fewer than 5 bytes for the initial read - delay the detection of TLS vs. plaintext mode until a socket stream is read from or written to. This prevents the `accept()` thread from getting blocked on a `read()` operation from the newly connected socket. - prepending 5 bytes to `PrependableSocket` and then trying to read >5 bytes would only return the first 5 bytes, even if more bytes were available. This is fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit 2122c8c23a0dbb27f9b2aff55e800e48d253f943 Author: Ilya Maykov Date: 2018-10-25T00:41:48Z ZOOKEEPER-3173: Quorum TLS - support PEM trust/key stores ZOOKEEPER-3175: Quorum TLS - test improvements Add support for loading key and trust stores from PEM files. Also added test utils for testing X509-related code, because it was very difficult to untangle them from the PEM support code. commit 514d48a26aeeca37290ad14ff8f0cdae69b53eb2 Author: Ilya Maykov Date: 2018-10-25T01:22:24Z ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/679 ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov reopened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. Note that this is stacked on top of #678 and thus includes it. Please only consider the ZOOKEEPER-3172 commit when reviewing. Once the other PR is merged upstream, I will rebase this so it only contains one commit. ## Fixed networking issues/bugs in UnifiedServerSocket - don't crash the `accept()` thread if the client closes the connection without sending any data - don't corrupt the connection if the client sends fewer than 5 bytes for the initial read - delay the detection of TLS vs. plaintext mode until a socket stream is read from or written to. This prevents the `accept()` thread from getting blocked on a `read()` operation from the newly connected socket. - prepending 5 bytes to `PrependableSocket` and then trying to read >5 bytes would only return the first 5 bytes, even if more bytes were available. This is fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit 2122c8c23a0dbb27f9b2aff55e800e48d253f943 Author: Ilya Maykov Date: 2018-10-25T00:41:48Z ZOOKEEPER-3173: Quorum TLS - support PEM trust/key stores ZOOKEEPER-3175: Quorum TLS - test improvements Add support for loading key and trust stores from PEM files. Also added test utils for testing X509-related code, because it was very difficult to untangle them from the PEM support code. commit 69f5185c8c14720e94c81f0147ee9cbc2ae42f89 Author: Ilya Maykov Date: 2018-10-25T01:22:24Z ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades ---
[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...
GitHub user ivmaykov opened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any data or crashing if less than 5 bytes are read from the socket in the initial read. Re-enable the "portUnification" config option. Note that this is stacked on top of #678 and thus includes it. Please only consider the ZOOKEEPER-3172 commit when reviewing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit b8b687ae4dea912ef18ee2ee1ace406800f3fce7 Author: Ilya Maykov Date: 2018-10-25T00:41:48Z ZOOKEEPER-3173: Quorum TLS - support PEM trust/key stores ZOOKEEPER-3175: Quorum TLS - test improvements Add support for loading key and trust stores from PEM files. Also added test utils for testing X509-related code, because it was very difficult to untangle them from the PEM support code. commit f9fb9c69f15f4d23acc714de75efe4592c6578b9 Author: Ilya Maykov Date: 2018-10-25T01:22:24Z ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades ---
