You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2020/04/27 19:20:03 UTC
[geode] branch support/1.12 updated: GEODE-8020: buffer corruption
in SSL communications (#4994)
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.12 by this push:
new 7850daf GEODE-8020: buffer corruption in SSL communications (#4994)
7850daf is described below
commit 7850daf4f3f7a22dec2c832ad2e563ae6e0a268e
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Mon Apr 27 12:09:26 2020 -0700
GEODE-8020: buffer corruption in SSL communications (#4994)
revert change in GEODE-6661 that made NioSslEngine use a direct buffer.
This class is not designed to share its buffer with a pool in
BufferPool. Connection is also modified to use a heap buffer for
reading encrypted SSL packets for consistency. New tests ensure that
these buffers are the correct type when using SSL or plain sockets.
(cherry picked from commit ec8db54ad7f342542762beb8f3e912dff44e3a53)
---
.../geode/distributed/internal/DistributionImpl.java | 3 +++
.../java/org/apache/geode/internal/net/NioSslEngine.java | 4 ++--
.../java/org/apache/geode/internal/tcp/Connection.java | 16 ++++++++++------
.../org/apache/geode/internal/net/NioSslEngineTest.java | 5 +++++
4 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java
index a89d23f..724995d 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java
@@ -794,6 +794,9 @@ public class DistributionImpl implements Distribution {
return result;
}
+ public DirectChannel getDirectChannel() {
+ return directChannel;
+ }
/**
* Insert our own MessageReceiver between us and the direct channel, in order to correctly filter
diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java b/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java
index 9d67594..f7d9f41 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java
@@ -74,7 +74,7 @@ public class NioSslEngine implements NioFilter {
int packetBufferSize = engine.getSession().getPacketBufferSize();
this.engine = engine;
this.bufferPool = bufferPool;
- this.myNetData = bufferPool.acquireDirectSenderBuffer(packetBufferSize);
+ this.myNetData = bufferPool.acquireNonDirectSenderBuffer(packetBufferSize);
this.peerAppData = bufferPool.acquireNonDirectReceiveBuffer(appBufferSize);
}
@@ -301,7 +301,7 @@ public class NioSslEngine implements NioFilter {
ByteBuffer buffer = wrappedBuffer;
int requiredSize = engine.getSession().getPacketBufferSize();
if (buffer == null) {
- buffer = bufferPool.acquireDirectBuffer(bufferType, requiredSize);
+ buffer = bufferPool.acquireNonDirectBuffer(bufferType, requiredSize);
} else if (buffer.capacity() < requiredSize) {
buffer = bufferPool.expandWriteBufferIfNeeded(bufferType, buffer, requiredSize);
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
index d266688..3f114f3 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
@@ -1628,8 +1628,8 @@ public class Connection implements Runnable {
} catch (IOException e) {
// "Socket closed" check needed for Solaris jdk 1.4.2_08
if (!isSocketClosed() && !"Socket closed".equalsIgnoreCase(e.getMessage())) {
- if (logger.isDebugEnabled() && !isIgnorableIOException(e)) {
- logger.debug("{} io exception for {}", p2pReaderName(), this, e);
+ if (logger.isInfoEnabled() && !isIgnorableIOException(e)) {
+ logger.info("{} io exception for {}", p2pReaderName(), this, e);
}
if (e.getMessage().contains("interrupted by a call to WSACancelBlockingCall")) {
if (logger.isDebugEnabled()) {
@@ -1684,7 +1684,7 @@ public class Connection implements Runnable {
if (inputBuffer != null) {
getBufferPool().releaseReceiveBuffer(inputBuffer);
}
- inputBuffer = getBufferPool().acquireDirectReceiveBuffer(packetBufferSize);
+ inputBuffer = getBufferPool().acquireNonDirectReceiveBuffer(packetBufferSize);
}
if (channel.socket().getReceiveBufferSize() < packetBufferSize) {
channel.socket().setReceiveBufferSize(packetBufferSize);
@@ -1727,9 +1727,13 @@ public class Connection implements Runnable {
}
msg = msg.toLowerCase();
- return msg.contains("forcibly closed")
- || msg.contains("reset by peer")
- || msg.contains("connection reset");
+
+ if (e instanceof SSLException && msg.contains("status = closed")) {
+ return true; // engine has been closed - this is normal
+ }
+
+ return (msg.contains("forcibly closed") || msg.contains("reset by peer")
+ || msg.contains("connection reset") || msg.contains("socket is closed"));
}
private static boolean validMsgType(int msgType) {
diff --git a/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java b/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java
index 1d5e4a6..f468d6d 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java
@@ -85,6 +85,11 @@ public class NioSslEngineTest {
}
@Test
+ public void engineUsesHeapBuffers() {
+ assertThat(nioSslEngine.myNetData.isDirect()).isFalse();
+ }
+
+ @Test
public void handshake() throws Exception {
SocketChannel mockChannel = mock(SocketChannel.class);
when(mockChannel.read(any(ByteBuffer.class))).thenReturn(100, 100, 100, 0);