Skip to content

Commit 391da84

Browse files
committed
Remove nonApplicationWrite from SSLDriver (elastic#41829)
Currently, when the SSLEngine needs to produce handshake or close data, we must manually call the nonApplicationWrite method. However, this data is only required when something triggers the need (starting handshake, reading from the wire, initiating close, etc). As we have a dedicated outbound buffer, this data can be produced automatically. Additionally, with this refactoring, we combine handshake and application mode into a single mode. This is necessary as there are non-application messages that are sent post handshake in TLS 1.3. Finally, this commit modifies the SSLDriver tests to test against TLS 1.3.
1 parent 2de919e commit 391da84

File tree

6 files changed

+249
-417
lines changed

6 files changed

+249
-417
lines changed

libs/nio/src/main/java/org/elasticsearch/nio/ChannelContext.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public boolean isOpen() {
9595
return closeContext.isDone() == false;
9696
}
9797

98-
void handleException(Exception e) {
98+
protected void handleException(Exception e) {
9999
exceptionHandler.accept(e);
100100
}
101101

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLChannelContext.java

+33-37
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.nio.WriteOperation;
1717

1818
import javax.net.ssl.SSLEngine;
19+
import javax.net.ssl.SSLException;
1920
import java.io.IOException;
2021
import java.nio.channels.ClosedChannelException;
2122
import java.util.LinkedList;
@@ -33,7 +34,8 @@
3334
public final class SSLChannelContext extends SocketChannelContext {
3435

3536
private static final long CLOSE_TIMEOUT_NANOS = new TimeValue(10, TimeUnit.SECONDS).nanos();
36-
private static final Runnable DEFAULT_TIMEOUT_CANCELLER = () -> {};
37+
private static final Runnable DEFAULT_TIMEOUT_CANCELLER = () -> {
38+
};
3739

3840
private final SSLDriver sslDriver;
3941
private final InboundChannelBuffer networkReadBuffer;
@@ -68,9 +70,17 @@ public void register() throws IOException {
6870
public void queueWriteOperation(WriteOperation writeOperation) {
6971
getSelector().assertOnSelectorThread();
7072
if (writeOperation instanceof CloseNotifyOperation) {
71-
sslDriver.initiateClose();
72-
long relativeNanos = CLOSE_TIMEOUT_NANOS + System.nanoTime();
73-
closeTimeoutCanceller = getSelector().getTaskScheduler().scheduleAtRelativeTime(this::channelCloseTimeout, relativeNanos);
73+
try {
74+
sslDriver.initiateClose();
75+
SSLOutboundBuffer outboundBuffer = sslDriver.getOutboundBuffer();
76+
if (outboundBuffer.hasEncryptedBytesToFlush()) {
77+
encryptedFlushes.addLast(outboundBuffer.buildNetworkFlushOperation());
78+
}
79+
long relativeNanos = CLOSE_TIMEOUT_NANOS + System.nanoTime();
80+
closeTimeoutCanceller = getSelector().getTaskScheduler().scheduleAtRelativeTime(this::channelCloseTimeout, relativeNanos);
81+
} catch (SSLException e) {
82+
handleException(e);
83+
}
7484
} else {
7585
super.queueWriteOperation(writeOperation);
7686
}
@@ -92,39 +102,25 @@ public void flushChannel() throws IOException {
92102
}
93103

94104
// If the driver is ready for application writes, we can attempt to proceed with any queued writes.
95-
if (sslDriver.readyForApplicationWrites()) {
96-
FlushOperation unencryptedFlush;
97-
while (pendingChannelFlush() == false && (unencryptedFlush = getPendingFlush()) != null) {
98-
if (unencryptedFlush.isFullyFlushed()) {
99-
currentFlushOperationComplete();
100-
} else {
101-
try {
102-
// Attempt to encrypt application write data. The encrypted data ends up in the
103-
// outbound write buffer.
104-
sslDriver.write(unencryptedFlush);
105-
SSLOutboundBuffer outboundBuffer = sslDriver.getOutboundBuffer();
106-
if (outboundBuffer.hasEncryptedBytesToFlush() == false) {
107-
break;
108-
}
109-
encryptedFlushes.addLast(outboundBuffer.buildNetworkFlushOperation());
110-
// Flush the write buffer to the channel
111-
flushEncryptedOperation();
112-
} catch (IOException e) {
113-
currentFlushOperationFailed(e);
114-
throw e;
105+
FlushOperation unencryptedFlush;
106+
while (pendingChannelFlush() == false && (unencryptedFlush = getPendingFlush()) != null) {
107+
if (unencryptedFlush.isFullyFlushed()) {
108+
currentFlushOperationComplete();
109+
} else {
110+
try {
111+
// Attempt to encrypt application write data. The encrypted data ends up in the
112+
// outbound write buffer.
113+
sslDriver.write(unencryptedFlush);
114+
SSLOutboundBuffer outboundBuffer = sslDriver.getOutboundBuffer();
115+
if (outboundBuffer.hasEncryptedBytesToFlush() == false) {
116+
break;
115117
}
116-
}
117-
}
118-
} else {
119-
// We are not ready for application writes, check if the driver has non-application writes. We
120-
// only want to continue producing new writes if the outbound write buffer is fully flushed.
121-
while (pendingChannelFlush() == false && sslDriver.needsNonApplicationWrite()) {
122-
sslDriver.nonApplicationWrite();
123-
// If non-application writes were produced, flush the outbound write buffer.
124-
SSLOutboundBuffer outboundBuffer = sslDriver.getOutboundBuffer();
125-
if (outboundBuffer.hasEncryptedBytesToFlush()) {
126-
encryptedFlushes.addFirst(outboundBuffer.buildNetworkFlushOperation());
118+
encryptedFlushes.addLast(outboundBuffer.buildNetworkFlushOperation());
119+
// Flush the write buffer to the channel
127120
flushEncryptedOperation();
121+
} catch (IOException e) {
122+
currentFlushOperationFailed(e);
123+
throw e;
128124
}
129125
}
130126
}
@@ -147,10 +143,10 @@ private void flushEncryptedOperation() throws IOException {
147143
@Override
148144
public boolean readyForFlush() {
149145
getSelector().assertOnSelectorThread();
150-
if (sslDriver.readyForApplicationWrites()) {
146+
if (sslDriver.readyForApplicationData()) {
151147
return pendingChannelFlush() || super.readyForFlush();
152148
} else {
153-
return pendingChannelFlush() || sslDriver.needsNonApplicationWrite();
149+
return pendingChannelFlush();
154150
}
155151
}
156152

0 commit comments

Comments
 (0)