Skip to content

Commit 6418dad

Browse files
committed
[fix] read_nonblock(exception: false) throwing on TLS 1.3
propagate exception flag through read() and readAndUnwrap()
1 parent e12ae01 commit 6418dad

File tree

2 files changed

+554
-7
lines changed

2 files changed

+554
-7
lines changed

src/main/java/org/jruby/ext/openssl/SSLSocket.java

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,18 @@ public void wakeup() {
539539
}
540540
}
541541

542-
private static final int READ_WOULD_BLOCK_RESULT = Integer.MIN_VALUE + 1;
543-
private static final int WRITE_WOULD_BLOCK_RESULT = Integer.MIN_VALUE + 2;
542+
// Legitimate return values are -1 (EOF) and >= 0 (byte counts), so any value < -1 is safely in sentinel territory.
543+
private static final int READ_WOULD_BLOCK_RESULT = -2;
544+
private static final int WRITE_WOULD_BLOCK_RESULT = -3;
545+
546+
private static boolean isWouldBlockResult(final int result) {
547+
return result < -1;
548+
}
549+
550+
private RubySymbol wouldBlockSymbol(final int result) {
551+
assert isWouldBlockResult(result) : "unexpected result: " + result;
552+
return getRuntime().newSymbol(result == READ_WOULD_BLOCK_RESULT ? "wait_readable" : "wait_writable");
553+
}
544554

545555
private static void readWouldBlock(final Ruby runtime, final boolean exception, final int[] result) {
546556
if ( exception ) throw newSSLErrorWaitReadable(runtime, "read would block");
@@ -577,7 +587,11 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception)
577587
doTasks();
578588
break;
579589
case NEED_UNWRAP:
580-
if (readAndUnwrap(blocking) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
590+
int unwrapResult = readAndUnwrap(blocking, exception);
591+
if (isWouldBlockResult(unwrapResult)) {
592+
return wouldBlockSymbol(unwrapResult);
593+
}
594+
if (unwrapResult == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
581595
throw new SSLHandshakeException("Socket closed");
582596
}
583597
// during initialHandshake, calling readAndUnwrap that results UNDERFLOW does not mean writable.
@@ -703,12 +717,16 @@ public int write(ByteBuffer src, boolean blocking) throws SSLException, IOExcept
703717
}
704718

705719
public int read(final ByteBuffer dst, final boolean blocking) throws IOException {
720+
return read(dst, blocking, true);
721+
}
722+
723+
private int read(final ByteBuffer dst, final boolean blocking, final boolean exception) throws IOException {
706724
if ( initialHandshake ) return 0;
707725
if ( engine.isInboundDone() ) return -1;
708726

709727
if ( ! appReadData.hasRemaining() ) {
710-
int appBytesProduced = readAndUnwrap(blocking);
711-
if (appBytesProduced == -1 || appBytesProduced == 0) {
728+
final int appBytesProduced = readAndUnwrap(blocking, exception);
729+
if (appBytesProduced == -1 || appBytesProduced == 0 || isWouldBlockResult(appBytesProduced)) {
712730
return appBytesProduced;
713731
}
714732
}
@@ -719,6 +737,18 @@ public int read(final ByteBuffer dst, final boolean blocking) throws IOException
719737
}
720738

721739
private int readAndUnwrap(final boolean blocking) throws IOException {
740+
return readAndUnwrap(blocking, true);
741+
}
742+
743+
/**
744+
* @param blocking whether to block on I/O
745+
* @param exception when false, returns {@link #READ_WOULD_BLOCK_RESULT} or
746+
* {@link #WRITE_WOULD_BLOCK_RESULT} instead of throwing if the
747+
* post-handshake processing would block
748+
* @return application bytes available, -1 on EOF/close, 0 when no app data
749+
* produced, or a WOULD_BLOCK sentinel when would-block with exception=false
750+
*/
751+
private int readAndUnwrap(final boolean blocking, final boolean exception) throws IOException {
722752
final int bytesRead = socketChannelImpl().read(netReadData);
723753
if ( bytesRead == -1 ) {
724754
if ( ! netReadData.hasRemaining() ||
@@ -767,7 +797,14 @@ private int readAndUnwrap(final boolean blocking) throws IOException {
767797
handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_TASK ||
768798
handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_WRAP ||
769799
handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED ) ) {
770-
doHandshake(blocking);
800+
IRubyObject wouldBlock = doHandshake(blocking, exception);
801+
if ( wouldBlock != null ) {
802+
// Propagate :wait_readable vs :wait_writable from doHandshake
803+
if ("wait_writable".equals(wouldBlock.asJavaString())) {
804+
return WRITE_WOULD_BLOCK_RESULT;
805+
}
806+
return READ_WOULD_BLOCK_RESULT;
807+
}
771808
}
772809
return appReadData.remaining();
773810
}
@@ -843,14 +880,19 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l
843880
if ( engine == null ) {
844881
read = socketChannelImpl().read(dst);
845882
} else {
846-
read = read(dst, blocking);
883+
read = read(dst, blocking, exception);
847884
}
848885

849886
if ( read == -1 ) {
850887
if ( exception ) throw runtime.newEOFError();
851888
return context.nil;
852889
}
853890

891+
if ( isWouldBlockResult(read) ) {
892+
// Post-handshake processing (e.g. TLS 1.3 NewSessionTicket) signaled would-block
893+
return wouldBlockSymbol(read);
894+
}
895+
854896
if ( read == 0 && status == SSLEngineResult.Status.BUFFER_UNDERFLOW ) {
855897
// If we didn't get any data back because we only read in a partial TLS record,
856898
// instead of spinning until the rest comes in, call waitSelect to either block

0 commit comments

Comments
 (0)