Skip to content

Commit 15af8fa

Browse files
fwbrasilfwbrasil
committed
[kyo-net] fatal TLS record: typed decrypt error on both drivers, not Closed
A fatal TLS record (RFC 5246 7.2.2) must tear the connection down surfaced as the typed NetConnectionIoException(Decrypt), never a bare Closed. The io_uring driver already did this. The poller delivered the typed failure only when the TLS engine threw (the JDK path). On the BoringSSL native path, where a fatal record is a readPlain == -2 onFatal signal rather than a throw, dispatchReadTls fell through to rearmTlsRead and completed Closed, so the fatal-record outcome was backend- and engine-dependent. PollerIoDriver.dispatchReadTls now records the fatal record and completes the typed decrypt failure with a bare completeDiscard before endDispatch, so the close bit onFatal's requestClose set cannot rewrite it into a Closed, and the re-arm path never re-deposits a read on the torn-down handle. IoUringDriverCorruptRecordTest asserted the pre-change Closed and now asserts the typed decrypt failure. A new PollerIoDriverCorruptRecordTest covers the poller. Both run on a real ring with BoringSSL staged.
1 parent 42cc21a commit 15af8fa

4 files changed

Lines changed: 192 additions & 30 deletions

File tree

‎kyo-net/jvm-native/src/test/scala/kyo/net/internal/posix/IoUringDriverCorruptRecordTest.scala‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package kyo.net.internal.posix
33
import kyo.*
44
import kyo.ffi.Buffer
55
import kyo.ffi.Ffi
6+
import kyo.net.NetConnectionIoException
67
import kyo.net.Test
78
import kyo.net.internal.TlsEngineLoopback
89
import kyo.net.internal.TlsRealEngines
@@ -11,18 +12,18 @@ import kyo.net.internal.transport.ReadOutcome
1112
/** io_uring-path parity guard for the fatal-record abort + read-produced-ciphertext drain (mirroring the poller's
1213
* [[TlsEngineIoCorruptRecordTest]] / [[TlsEngineIoReadDrainTest]]).
1314
*
14-
* Both reproductions and fixes for the fatal-record swallow (RFC 5246 §7.2.2: a fatal record-layer error must terminate the connection, not
15-
* be delivered behind good data) and the read-path WANT_WRITE / KeyUpdate stall (a `SSL_read` that queues outbound ciphertext must be sent)
16-
* live on the shared [[TlsEngineIo]] decrypt path. The io_uring read CQE completion drives that same path inside its engine FIFO op
17-
* ([[IoUringDriver]] `complete` -> `feedAndDecrypt`), then checks `PosixHandle.isClosing()` (the io_uring twin of the poller's rearmOwned /
18-
* endDispatch closing check) to fail the read `Closed` on a fatal record, and calls `flushTls` to send any ciphertext the read produced. This
19-
* test pins that the io_uring path behaves identically to the poller path on a REAL ring with a REAL BoringSSL engine, with no dedicated
20-
* io_uring-path coverage before now.
15+
* The fatal-record swallow (RFC 5246 §7.2.2: a fatal record-layer error must terminate the connection, not be delivered behind good data) and
16+
* the read-path KeyUpdate stall (a `SSL_read` that queues outbound ciphertext must send it) both live on the shared [[TlsEngineIo]] decrypt path.
17+
* The io_uring read CQE completion drives that same path inside its engine FIFO op
18+
* ([[IoUringDriver]] `complete` -> `feedAndDecrypt`), detects the fatal record via `feedAndDecrypt`'s `onFatal` signal, and fails the read with
19+
* the typed decrypt error (`ReadOutcome.Failed(NetConnectionIoException(Decrypt))`) while tearing the connection down, then calls `flushTls` to
20+
* send any ciphertext the read produced. This test pins that the io_uring path behaves identically to the poller path on a REAL ring with a REAL
21+
* BoringSSL engine.
2122
*
2223
* Leaf 1 (fatal-record abort) feeds `[good record][corrupted record]` (the corruption flips a body byte of the second record so its AEAD tag
23-
* fails) coalesced on the wire and reads via the driver. The driver's read CQE feeds the engine; `feedAndDecrypt` reaches the fatal `-2`,
24-
* calls `requestClose()`, and the completion observes the now-closing handle and fails the read `Closed` rather than delivering the good prefix
25-
* or re-arming a freed handle. Leaf 2 (read-produced-ciphertext drain) reads a real inbound record through the same CQE path and asserts the
24+
* fails) coalesced on the wire and reads via the driver. The driver's read CQE feeds the engine; `feedAndDecrypt` reaches the fatal record,
25+
* fires `onFatal` (tearing the connection down), and the completion fails the read with the typed decrypt error rather than delivering the good
26+
* prefix or re-arming a freed handle. Leaf 2 (read-produced-ciphertext drain) reads a real inbound record through the same CQE path and asserts the
2627
* engine's write side was drained (`drainCiphertext` ran), the wiring by which a TLS 1.3 KeyUpdate response queued during the read reaches the
2728
* wire via `flushTls` (a real end-to-end KeyUpdate is not drivable: the shims bind no `SSL_key_update`, the same limitation the poller's
2829
* read-drain leaf documents).
@@ -69,7 +70,7 @@ class IoUringDriverCorruptRecordTest extends Test:
6970

7071
"IoUringDriver corrupt-record + read-drain (real ring, real engine)" - {
7172

72-
"a fatal TLS record on a read CQE fails the read Closed and tears the connection down, never delivering the good prefix" in {
73+
"a fatal TLS record on a read CQE fails the read with a typed decrypt error and tears the connection down, never delivering the good prefix" in {
7374
PosixTestSockets.assumeUring()
7475
TlsRealEngines.assumeTlsReady()
7576
TlsRealEngines.withEngines { (clientEngine, serverEngine) =>
@@ -112,26 +113,26 @@ class IoUringDriverCorruptRecordTest extends Test:
112113
drv.closeHandle(handle)
113114
discard(sock.close(peerFd))
114115
outcome match
115-
case Result.Failure(_: Closed) =>
116-
// RFC 5246 §7.2.2: the fatal record tears the connection down. The driver must NOT have delivered the good
117-
// prefix as a normal read, and the handle must be closing (requestClose fired), not re-armed.
116+
case Result.Success(ReadOutcome.Failed(e: NetConnectionIoException)) =>
117+
// RFC 5246 §7.2.2: the fatal record tears the connection down, surfaced as the typed decrypt failure (never a
118+
// misleading Closed, and never the good prefix). The handle must be closing (onFatal tore it down), not re-armed.
119+
assert(
120+
e.operation == NetConnectionIoException.Operation.Decrypt,
121+
s"the fatal record must surface as a TLS decrypt failure; got operation ${e.operation}"
122+
)
118123
assert(
119124
closing,
120-
"the fatal record must mark the handle closing (requestClose), tearing it down rather than re-arming a freed handle"
125+
"the fatal record must mark the handle closing (onFatal), tearing it down rather than re-arming a freed handle"
121126
)
122127
case Result.Success(ReadOutcome.Bytes(got)) =>
123128
fail(
124129
s"a fatal TLS record was swallowed: the read delivered ${got.size} bytes (${got.toArray.toList}) " +
125-
"instead of failing Closed; RFC 5246 §7.2.2 requires the connection to be torn down"
126-
)
127-
case Result.Success(other) =>
128-
fail(
129-
s"a fatal TLS record was swallowed: the read produced $other " +
130-
"instead of failing Closed; RFC 5246 §7.2.2 requires the connection to be torn down"
130+
"instead of the typed decrypt failure; RFC 5246 §7.2.2 requires the connection to be torn down"
131131
)
132132
case Result.Failure(_: Timeout) =>
133-
fail("the read hung on a fatal record: the fatal abort never failed the read Closed")
134-
case other => fail(s"unexpected read outcome: $other")
133+
fail("the read hung on a fatal record: the fatal abort never completed the read")
134+
case other =>
135+
fail(s"a fatal TLS record must surface as the typed decrypt failure; got: $other")
135136
end match
136137
}
137138
}

‎kyo-net/jvm-native/src/test/scala/kyo/net/internal/posix/PollerIoDriverCorruptRecordTest.scala‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package kyo.net.internal.posix
2+
3+
import kyo.*
4+
import kyo.ffi.Buffer
5+
import kyo.ffi.Ffi
6+
import kyo.net.NetConnectionIoException
7+
import kyo.net.Test
8+
import kyo.net.internal.TlsEngineLoopback
9+
import kyo.net.internal.TlsRealEngines
10+
import kyo.net.internal.transport.ReadOutcome
11+
12+
/** Poller-path fatal-record parity guard, the epoll/kqueue sibling of [[IoUringDriverCorruptRecordTest]].
13+
*
14+
* A fatal record-layer error (RFC 5246 §7.2.2) must terminate the connection surfaced as the typed decrypt failure
15+
* `ReadOutcome.Failed(NetConnectionIoException(Decrypt))`, identical to the io_uring driver, never a bare `Closed` and never the good
16+
* prefix delivered behind the corruption. On BoringSSL a fatal record is signalled by `feedAndDecrypt`'s `onFatal` (a `readPlain == -2`
17+
* return, not a thrown exception), so `dispatchReadTls` must complete the typed decrypt failure on that path directly: `onFatal`'s
18+
* `requestClose` marks the handle closing, and the read completion is what distinguishes a corrupt record (a typed decrypt failure) from an
19+
* ordinary local close (`Closed`).
20+
*
21+
* The leaf feeds `[good record][corrupted record]` (the corruption flips the last body byte of the second record so its AEAD tag fails)
22+
* coalesced in one wire write, so both records arrive in a single recv, and reads once via the driver. It asserts the read fails with the
23+
* typed decrypt failure, the handle is closing, and neither the good prefix nor a bare `Closed` is delivered.
24+
*
25+
* Gate: [[PosixTestSockets.assumePoller]] (Linux epoll or macOS kqueue) and [[TlsRealEngines.assumeTlsReady]] (a staged BoringSSL/OpenSSL
26+
* provider); cancels cleanly on JS, off a poller platform, or where no provider is staged, so this is CI-validated on native Linux with
27+
* BoringSSL staged.
28+
*
29+
* Anti-flakiness: the read synchronizes on the read promise (completed only when the recv edge dispatches and the decrypt engine op runs),
30+
* never a timer; `Async.timeout` is only the deadlock ceiling. The settle-wait on `tls` going Absent lets the driver's own queued TLS
31+
* teardown run its engine free before `withEngines` frees the same engine out of band, so that becomes a harmless CAS-guarded second free.
32+
* No sleep, no busy-spin.
33+
*/
34+
class PollerIoDriverCorruptRecordTest extends Test:
35+
36+
import AllowUnsafe.embrace.danger
37+
38+
private def sock = Ffi.load[SocketBindings]
39+
40+
/** Poll a real condition until it holds or the bound elapses, re-checking each turn after a short Async.sleep. */
41+
private def awaitCondition(bound: Duration)(cond: => Boolean)(using Frame): Boolean < Async =
42+
val deadline = java.lang.System.nanoTime() + bound.toNanos
43+
Loop(()) { _ =>
44+
if cond then Loop.done(true)
45+
else if java.lang.System.nanoTime() >= deadline then Loop.done(false)
46+
else Async.sleep(2.millis).andThen(Loop.continue(()))
47+
}
48+
end awaitCondition
49+
50+
"PollerIoDriver corrupt-record (real epoll/kqueue, real engine)" - {
51+
52+
"a fatal TLS record on a read fails the read with a typed decrypt error and tears the connection down, never delivering the good prefix" in {
53+
if kyo.internal.Platform.isJS then Sync.defer(succeed)
54+
else
55+
TlsRealEngines.assumeTlsReady()
56+
PosixTestSockets.assumePoller()
57+
TlsRealEngines.withEngines { (clientEngine, serverEngine) =>
58+
val driver = PollerIoDriver.init()
59+
discard(driver.start())
60+
Sync.ensure(Sync.defer(driver.close())) {
61+
PosixTestSockets.loopbackPair().map { case (client, accepted) =>
62+
val handshakeDone = TlsEngineLoopback.handshake(clientEngine, serverEngine)
63+
assert(handshakeDone, "TLS handshake must complete before the read")
64+
val acceptedH = PosixHandle.socket(accepted, PosixHandle.DefaultReadBufferSize, Absent, Frame.internal)
65+
acceptedH.tls = Present(serverEngine)
66+
67+
val good = "GOOD-application-record".getBytes("UTF-8")
68+
val bad = "TAMPERED-application-record".getBytes("UTF-8")
69+
// One writePlain per record yields one TLS record each.
70+
val goodRecord = TlsEngineLoopback.encrypt(clientEngine, good)
71+
val badRecord = TlsEngineLoopback.encrypt(clientEngine, bad)
72+
assert(
73+
goodRecord.length > 5 && badRecord.length > 5,
74+
"expected real TLS records with a 5-byte header plus body"
75+
)
76+
// Corrupt the body of the SECOND record (skip the 5-byte header) so its AEAD tag fails; the first stays intact.
77+
val corrupted = badRecord.clone()
78+
corrupted(corrupted.length - 1) = (corrupted(corrupted.length - 1) ^ 0xff).toByte
79+
// Coalesce [good record][corrupted record] in one wire write, exactly the on-wire batching the driver sees under load.
80+
val coalesced = new Array[Byte](goodRecord.length + corrupted.length)
81+
java.lang.System.arraycopy(goodRecord, 0, coalesced, 0, goodRecord.length)
82+
java.lang.System.arraycopy(corrupted, 0, coalesced, goodRecord.length, corrupted.length)
83+
// Send BEFORE arming the read so both records are in the accepted side's kernel buffer, delivered in one recv.
84+
val cipherBuf = Buffer.fromArray[Byte](coalesced)
85+
val sendR =
86+
try sock.sendNow(client, cipherBuf, coalesced.length.toLong, PosixConstants.MSG_NOSIGNAL)
87+
finally cipherBuf.close()
88+
assert(sendR.value.toInt == coalesced.length, s"send failed: errno=${sendR.errorCode}")
89+
90+
val promise = Promise.Unsafe.init[ReadOutcome, Abort[Closed]]()
91+
driver.awaitRead(acceptedH, promise)
92+
Abort.run[Timeout | Closed](Async.timeout(5.seconds)(promise.safe.get)).map { outcome =>
93+
// requestClose fired inside the fatal completion (poll carrier), which happens-before this outcome, so isClosing is
94+
// already settled here. Capture it before our own closeHandle below.
95+
val closing = acceptedH.isClosing()
96+
driver.closeHandle(acceptedH)
97+
discard(sock.close(client))
98+
// Let the driver's queued TLS teardown run its own engine free before withEngines frees the same engine out of band.
99+
awaitCondition(5.seconds)(!acceptedH.tls.isDefined).map { settled =>
100+
assert(
101+
settled,
102+
"the driver's own TLS teardown never settled (a hang, not the fatal-record path this guard targets)"
103+
)
104+
outcome match
105+
case Result.Success(ReadOutcome.Failed(e: NetConnectionIoException)) =>
106+
// RFC 5246 §7.2.2: the fatal record tears the connection down, surfaced as the typed decrypt failure
107+
// (never a misleading Closed, and never the good prefix). The handle must be closing, not re-armed.
108+
assert(
109+
e.operation == NetConnectionIoException.Operation.Decrypt,
110+
s"the fatal record must surface as a TLS decrypt failure; got operation ${e.operation}"
111+
)
112+
assert(
113+
closing,
114+
"the fatal record must mark the handle closing (requestClose), tearing it down rather than re-arming a freed handle"
115+
)
116+
case Result.Success(ReadOutcome.Bytes(got)) =>
117+
fail(
118+
s"a fatal TLS record was swallowed: the read delivered ${got.size} bytes (${got.toArray.toList}) " +
119+
"instead of the typed decrypt failure; RFC 5246 §7.2.2 requires the connection to be torn down"
120+
)
121+
case Result.Failure(_: Closed) =>
122+
fail(
123+
"a fatal TLS record surfaced as a bare Closed instead of the typed decrypt failure: the fatal " +
124+
"path fell through to the endDispatch closing check. io_uring and the poller must agree"
125+
)
126+
case Result.Failure(_: Timeout) =>
127+
fail(
128+
"the read hung on a fatal record: the fatal path did not complete the read (a torn-down handle was re-armed)"
129+
)
130+
case other =>
131+
fail(s"a fatal TLS record must surface as the typed decrypt failure; got: $other")
132+
end match
133+
}
134+
}
135+
}
136+
}
137+
}
138+
}
139+
}
140+
141+
end PollerIoDriverCorruptRecordTest

‎kyo-net/shared/src/main/scala/kyo/net/internal/posix/IoUringDriver.scala‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,11 +2007,11 @@ final private[net] class IoUringDriver private[posix] (
20072007
() =>
20082008
fatalRecord = true; closeHandle(h)
20092009
)
2010-
// Fatal-record check (mirrors the poller's rearmOwned endDispatch closing check): fatalRecord is set
2011-
// exactly when THIS call's onFatal fired; isClosing() is kept too for any other concurrent close that
2012-
// already ran the guard's free (a rare path independent of this read). Either way, failing the read
2013-
// promise Closed here tears the connection down on io_uring exactly as the poller's rearmOwned /
2014-
// endDispatch does when the dispatch guard observes the close bit.
2010+
// Fatal-record check: fatalRecord is set exactly when THIS call's onFatal fired. isClosing() is kept
2011+
// too for any other concurrent close that already ran the guard's free (a rare path independent of
2012+
// this read). Either way the read completes with the typed decrypt failure, never a bare Closed and
2013+
// never the good prefix (feedAndDecrypt discarded it per RFC 5246 7.2.2). The closeHandle queued by
2014+
// onFatal runs the actual teardown behind this op on the engine FIFO.
20152015
if fatalRecord || h.isClosing() then
20162016
promise.completeDiscard(Result.succeed(ReadOutcome.Failed(NetConnectionIoException(
20172017
s"connection ${handleLabel(h)}",

0 commit comments

Comments
 (0)