From 57393b975d495379f713618323bad378e2e2c757 Mon Sep 17 00:00:00 2001 From: nitely Date: Sun, 20 Apr 2025 20:15:08 -0300 Subject: [PATCH 1/7] asyncnet ssl overhaul --- lib/pure/asyncnet.nim | 161 ++++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 86 deletions(-) diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim index fb37afa4271f1..5f81b9343aaa1 100644 --- a/lib/pure/asyncnet.nim +++ b/lib/pure/asyncnet.nim @@ -126,8 +126,6 @@ type when defineSsl: sslHandle: SslPtr sslContext: SslContext - bioIn: BIO - bioOut: BIO sslNoShutdown: bool domain: Domain sockType: SockType @@ -228,42 +226,23 @@ when defineSsl: raiseSSLError() else: raiseSSLError("Unknown Error") - proc sendPendingSslData(socket: AsyncSocket, - flags: set[SocketFlag]) {.async.} = - if socket.sslHandle == nil: - raiseSslHandleError() - let len = bioCtrlPending(socket.bioOut) - if len > 0: - var data = newString(len) - let read = bioRead(socket.bioOut, cast[cstring](addr data[0]), len) - assert read != 0 - if read < 0: - raiseSSLError() - data.setLen(read) - await socket.fd.AsyncFD.send(data, flags) - - proc appeaseSsl(socket: AsyncSocket, flags: set[SocketFlag], - sslError: cint): owned(Future[bool]) {.async.} = + proc appeaseSsl(socket: AsyncSocket, flags: set[SocketFlag], sslError: cint): Future[bool] = ## Returns `true` if `socket` is still connected, otherwise `false`. - result = true + let retFut = newFuture[bool]() case sslError of SSL_ERROR_WANT_WRITE: - await sendPendingSslData(socket, flags) + addWrite(socket.fd.AsyncFD, proc (sock: AsyncFD): bool = + retFut.complete(true) + return true + ) of SSL_ERROR_WANT_READ: - var data = await recv(socket.fd.AsyncFD, BufferSize, flags) - if socket.sslHandle == nil: - raiseSslHandleError() - let length = len(data) - if length > 0: - let ret = bioWrite(socket.bioIn, cast[cstring](addr data[0]), length.cint) - if ret < 0: - raiseSSLError() - elif length == 0: - # connection not properly closed by remote side or connection dropped - SSL_set_shutdown(socket.sslHandle, SSL_RECEIVED_SHUTDOWN) - result = false + addRead(socket.fd.AsyncFD, proc (sock: AsyncFD): bool = + retFut.complete(true) + return true + ) else: raiseSSLError("Cannot appease SSL.") + return retFut template sslLoop(socket: AsyncSocket, flags: set[SocketFlag], op: untyped) = @@ -279,15 +258,12 @@ when defineSsl: getSslError(socket, opResult.cint) else: SSL_ERROR_NONE - # Send any remaining pending SSL data. - await sendPendingSslData(socket, flags) # If the operation failed, try to see if SSL has some data to read # or write. if opResult < 0: - let fut = appeaseSsl(socket, flags, err.cint) - yield fut - if not fut.read(): + let connected = await appeaseSsl(socket, flags, err.cint) + if not connected: # Socket disconnected. if SocketFlag.SafeDisconn in flags: opResult = 0.cint @@ -323,8 +299,7 @@ proc connect*(socket: AsyncSocket, address: string, port: Port) {.async.} = discard SSL_set_tlsext_host_name(socket.sslHandle, address) let flags = {SocketFlag.SafeDisconn} - sslSetConnectState(socket.sslHandle) - sslLoop(socket, flags, sslDoHandshake(socket.sslHandle)) + sslLoop(socket, flags, SSL_connect(socket.sslHandle)) template readInto(buf: pointer, size: int, socket: AsyncSocket, flags: set[SocketFlag]): int = @@ -461,7 +436,6 @@ proc send*(socket: AsyncSocket, buf: pointer, size: int, when defineSsl: sslLoop(socket, flags, sslWrite(socket.sslHandle, cast[cstring](buf), size.cint)) - await sendPendingSslData(socket, flags) else: await send(socket.fd.AsyncFD, buf, size, flags) @@ -475,52 +449,9 @@ proc send*(socket: AsyncSocket, data: string, var copy = data sslLoop(socket, flags, sslWrite(socket.sslHandle, cast[cstring](addr copy[0]), copy.len.cint)) - await sendPendingSslData(socket, flags) else: await send(socket.fd.AsyncFD, data, flags) -proc acceptAddr*(socket: AsyncSocket, flags = {SocketFlag.SafeDisconn}, - inheritable = defined(nimInheritHandles)): - owned(Future[tuple[address: string, client: AsyncSocket]]) = - ## Accepts a new connection. Returns a future containing the client socket - ## corresponding to that connection and the remote address of the client. - ## - ## If `inheritable` is false (the default), the resulting client socket will - ## not be inheritable by child processes. - ## - ## The future will complete when the connection is successfully accepted. - var retFuture = newFuture[tuple[address: string, client: AsyncSocket]]("asyncnet.acceptAddr") - var fut = acceptAddr(socket.fd.AsyncFD, flags, inheritable) - fut.callback = - proc (future: Future[tuple[address: string, client: AsyncFD]]) = - assert future.finished - if future.failed: - retFuture.fail(future.readError) - else: - let resultTup = (future.read.address, - newAsyncSocket(future.read.client, socket.domain, - socket.sockType, socket.protocol, socket.isBuffered, inheritable)) - retFuture.complete(resultTup) - return retFuture - -proc accept*(socket: AsyncSocket, - flags = {SocketFlag.SafeDisconn}): owned(Future[AsyncSocket]) = - ## Accepts a new connection. Returns a future containing the client socket - ## corresponding to that connection. - ## If `inheritable` is false (the default), the resulting client socket will - ## not be inheritable by child processes. - ## The future will complete when the connection is successfully accepted. - var retFut = newFuture[AsyncSocket]("asyncnet.accept") - var fut = acceptAddr(socket, flags) - fut.callback = - proc (future: Future[tuple[address: string, client: AsyncSocket]]) = - assert future.finished - if future.failed: - retFut.fail(future.readError) - else: - retFut.complete(future.read.client) - return retFut - proc recvLineInto*(socket: AsyncSocket, resString: FutureVar[string], flags = {SocketFlag.SafeDisconn}, maxLength = MaxLineLength) {.async.} = ## Reads a line of data from `socket` into `resString`. @@ -776,9 +707,7 @@ when defineSsl: if socket.sslHandle == nil: raiseSSLError() - socket.bioIn = bioNew(bioSMem()) - socket.bioOut = bioNew(bioSMem()) - sslSetBio(socket.sslHandle, socket.bioIn, socket.bioOut) + discard SSL_set_fd(socket.sslHandle, socket.fd) socket.sslNoShutdown = true @@ -795,6 +724,8 @@ when defineSsl: ## ## **Disclaimer**: This code is not well tested, may be very unsafe and ## prone to security vulnerabilities. + if socket.isSsl: + return wrapSocket(ctx, socket) case handshake @@ -818,6 +749,64 @@ when defineSsl: else: result = getPeerCertificates(socket.sslHandle) +proc acceptAddrImpl(socket: AsyncSocket, flags = {SocketFlag.SafeDisconn}, + inheritable = defined(nimInheritHandles)): + owned(Future[tuple[address: string, client: AsyncSocket]]) = + var retFuture = newFuture[tuple[address: string, client: AsyncSocket]]("asyncnet.acceptAddr") + var fut = acceptAddr(socket.fd.AsyncFD, flags, inheritable) + fut.callback = + proc (future: Future[tuple[address: string, client: AsyncFD]]) = + assert future.finished + if future.failed: + retFuture.fail(future.readError) + else: + let resultTup = (future.read.address, + newAsyncSocket(future.read.client, socket.domain, + socket.sockType, socket.protocol, socket.isBuffered, inheritable)) + retFuture.complete(resultTup) + return retFuture + +proc acceptAddr*( + socket: AsyncSocket, + flags = {SocketFlag.SafeDisconn}, + inheritable = defined(nimInheritHandles) +): owned(Future[tuple[address: string, client: AsyncSocket]]) {.async.} = + ## Accepts a new connection. Returns a future containing the client socket + ## corresponding to that connection and the remote address of the client. + ## + ## If `inheritable` is false (the default), the resulting client socket will + ## not be inheritable by child processes. + ## + ## The future will complete when the connection is successfully accepted. + result = await acceptAddrImpl(socket, flags, inheritable) + if socket.isSsl: + when defineSsl: + if socket.sslContext == nil: + raiseSSLError("The SSL Context is closed/unset") + wrapSocket(socket.sslContext, result.client) + if result.client.sslHandle == nil: + raiseSslHandleError() + let flags = {SocketFlag.SafeDisconn} + sslLoop(result.client, flags, SSL_accept(result.client.sslHandle)) + +proc accept*(socket: AsyncSocket, + flags = {SocketFlag.SafeDisconn}): owned(Future[AsyncSocket]) = + ## Accepts a new connection. Returns a future containing the client socket + ## corresponding to that connection. + ## If `inheritable` is false (the default), the resulting client socket will + ## not be inheritable by child processes. + ## The future will complete when the connection is successfully accepted. + var retFut = newFuture[AsyncSocket]("asyncnet.accept") + var fut = acceptAddr(socket, flags) + fut.callback = + proc (future: Future[tuple[address: string, client: AsyncSocket]]) = + assert future.finished + if future.failed: + retFut.fail(future.readError) + else: + retFut.complete(future.read.client) + return retFut + proc getSockOpt*(socket: AsyncSocket, opt: SOBool, level = SOL_SOCKET): bool {. tags: [ReadIOEffect].} = ## Retrieves option `opt` as a boolean value. From 879ff2399e64e9f6c391ed400765d56d0dde4dc4 Mon Sep 17 00:00:00 2001 From: nitely Date: Mon, 21 Apr 2025 00:11:34 -0300 Subject: [PATCH 2/7] wip --- lib/pure/asyncnet.nim | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim index 5f81b9343aaa1..91d2d76788afb 100644 --- a/lib/pure/asyncnet.nim +++ b/lib/pure/asyncnet.nim @@ -230,7 +230,7 @@ when defineSsl: ## Returns `true` if `socket` is still connected, otherwise `false`. let retFut = newFuture[bool]() case sslError - of SSL_ERROR_WANT_WRITE: + of SSL_ERROR_WANT_WRITE, SSL_ERROR_WANT_CONNECT, SSL_ERROR_WANT_ACCEPT: addWrite(socket.fd.AsyncFD, proc (sock: AsyncFD): bool = retFut.complete(true) return true @@ -707,7 +707,8 @@ when defineSsl: if socket.sslHandle == nil: raiseSSLError() - discard SSL_set_fd(socket.sslHandle, socket.fd) + if SSL_set_fd(socket.sslHandle, socket.fd) != 1: + raiseSSLError() socket.sslNoShutdown = true From 1156ed46d15517baffdaa161a03ae3b63ddcdc90 Mon Sep 17 00:00:00 2001 From: nitely Date: Mon, 21 Apr 2025 07:35:19 -0300 Subject: [PATCH 3/7] wip --- lib/pure/asyncnet.nim | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim index 91d2d76788afb..a96b4f655be69 100644 --- a/lib/pure/asyncnet.nim +++ b/lib/pure/asyncnet.nim @@ -208,7 +208,7 @@ when defineSsl: proc raiseSslHandleError = raiseSSLError("The SSL Handle is closed/unset") - proc getSslError(socket: AsyncSocket, err: cint): cint = + proc getSslError(socket: AsyncSocket, flags: set[SocketFlag], err: cint): cint = assert socket.isSsl assert err < 0 var ret = SSL_get_error(socket.sslHandle, err.cint) @@ -221,7 +221,25 @@ when defineSsl: return ret of SSL_ERROR_WANT_X509_LOOKUP: raiseSSLError("Function for x509 lookup has been called.") - of SSL_ERROR_SYSCALL, SSL_ERROR_SSL: + of SSL_ERROR_SYSCALL: + socket.sslNoShutdown = true + let osErr = osLastError() + if not flags.isDisconnectionError(osErr): + var errStr = "IO error has occurred" + let sslErr = ERR_peek_last_error() + if sslErr == 0 and err == 0: + errStr.add ' ' + errStr.add "because an EOF was observed that violates the protocol" + elif sslErr == 0 and err == -1: + errStr.add ' ' + errStr.add "in the BIO layer" + else: + let errStr = $ERR_error_string(sslErr, nil) + raiseSSLError(errStr & ": " & errStr) + raiseOSError(osErr, errStr) + else: + return ret + of SSL_ERROR_SSL: socket.sslNoShutdown = true raiseSSLError() else: raiseSSLError("Unknown Error") @@ -240,6 +258,9 @@ when defineSsl: retFut.complete(true) return true ) + of SSL_ERROR_SYSCALL: + assert flags.isDisconnectionError(osLastError()) + retFut.complete(false) else: raiseSSLError("Cannot appease SSL.") return retFut @@ -253,15 +274,10 @@ when defineSsl: ErrClearError() # Call the desired operation. opResult = op - let err = - if opResult < 0: - getSslError(socket, opResult.cint) - else: - SSL_ERROR_NONE - # If the operation failed, try to see if SSL has some data to read # or write. if opResult < 0: + let err = getSslError(socket, flags, opResult.cint) let connected = await appeaseSsl(socket, flags, err.cint) if not connected: # Socket disconnected. From d1eaa35cde177442c3dd356241cac414b825bf28 Mon Sep 17 00:00:00 2001 From: nitely Date: Wed, 23 Apr 2025 19:18:46 -0300 Subject: [PATCH 4/7] wip --- lib/pure/asyncnet.nim | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim index a96b4f655be69..2c586c9b0e700 100644 --- a/lib/pure/asyncnet.nim +++ b/lib/pure/asyncnet.nim @@ -244,7 +244,7 @@ when defineSsl: raiseSSLError() else: raiseSSLError("Unknown Error") - proc appeaseSsl(socket: AsyncSocket, flags: set[SocketFlag], sslError: cint): Future[bool] = + proc handleSslFailure(socket: AsyncSocket, flags: set[SocketFlag], sslError: cint): Future[bool] = ## Returns `true` if `socket` is still connected, otherwise `false`. let retFut = newFuture[bool]() case sslError @@ -262,7 +262,7 @@ when defineSsl: assert flags.isDisconnectionError(osLastError()) retFut.complete(false) else: - raiseSSLError("Cannot appease SSL.") + raiseSSLError("Cannot handle SSL failure.") return retFut template sslLoop(socket: AsyncSocket, flags: set[SocketFlag], @@ -278,7 +278,7 @@ when defineSsl: # or write. if opResult < 0: let err = getSslError(socket, flags, opResult.cint) - let connected = await appeaseSsl(socket, flags, err.cint) + let connected = await handleSslFailure(socket, flags, err.cint) if not connected: # Socket disconnected. if SocketFlag.SafeDisconn in flags: @@ -766,28 +766,9 @@ when defineSsl: else: result = getPeerCertificates(socket.sslHandle) -proc acceptAddrImpl(socket: AsyncSocket, flags = {SocketFlag.SafeDisconn}, +proc acceptAddr*(socket: AsyncSocket, flags = {SocketFlag.SafeDisconn}, inheritable = defined(nimInheritHandles)): - owned(Future[tuple[address: string, client: AsyncSocket]]) = - var retFuture = newFuture[tuple[address: string, client: AsyncSocket]]("asyncnet.acceptAddr") - var fut = acceptAddr(socket.fd.AsyncFD, flags, inheritable) - fut.callback = - proc (future: Future[tuple[address: string, client: AsyncFD]]) = - assert future.finished - if future.failed: - retFuture.fail(future.readError) - else: - let resultTup = (future.read.address, - newAsyncSocket(future.read.client, socket.domain, - socket.sockType, socket.protocol, socket.isBuffered, inheritable)) - retFuture.complete(resultTup) - return retFuture - -proc acceptAddr*( - socket: AsyncSocket, - flags = {SocketFlag.SafeDisconn}, - inheritable = defined(nimInheritHandles) -): owned(Future[tuple[address: string, client: AsyncSocket]]) {.async.} = + owned(Future[tuple[address: string, client: AsyncSocket]]) {.async.} = ## Accepts a new connection. Returns a future containing the client socket ## corresponding to that connection and the remote address of the client. ## @@ -795,7 +776,10 @@ proc acceptAddr*( ## not be inheritable by child processes. ## ## The future will complete when the connection is successfully accepted. - result = await acceptAddrImpl(socket, flags, inheritable) + let (address, fd) = await acceptAddr(socket.fd.AsyncFD, flags, inheritable) + let client = newAsyncSocket(fd, socket.domain, socket.sockType, + socket.protocol, socket.isBuffered, inheritable) + result = (address, client) if socket.isSsl: when defineSsl: if socket.sslContext == nil: From ae7fc8664f4ca3c1ed4d85b228b41ca8448f79ff Mon Sep 17 00:00:00 2001 From: nitely Date: Wed, 23 Apr 2025 19:21:27 -0300 Subject: [PATCH 5/7] wip --- lib/pure/asyncnet.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim index 2c586c9b0e700..76bacb162e7a0 100644 --- a/lib/pure/asyncnet.nim +++ b/lib/pure/asyncnet.nim @@ -246,7 +246,7 @@ when defineSsl: proc handleSslFailure(socket: AsyncSocket, flags: set[SocketFlag], sslError: cint): Future[bool] = ## Returns `true` if `socket` is still connected, otherwise `false`. - let retFut = newFuture[bool]() + let retFut = newFuture[bool]("asyncnet.handleSslFailure") case sslError of SSL_ERROR_WANT_WRITE, SSL_ERROR_WANT_CONNECT, SSL_ERROR_WANT_ACCEPT: addWrite(socket.fd.AsyncFD, proc (sock: AsyncFD): bool = From b1295079b69dcbffd806045ef3b2ceeff41db319 Mon Sep 17 00:00:00 2001 From: nitely Date: Sun, 27 Apr 2025 20:50:21 -0300 Subject: [PATCH 6/7] test --- tests/async/t24895.nim | 78 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/async/t24895.nim diff --git a/tests/async/t24895.nim b/tests/async/t24895.nim new file mode 100644 index 0000000000000..a37bb505a7d3f --- /dev/null +++ b/tests/async/t24895.nim @@ -0,0 +1,78 @@ +discard """ + cmd: "nim $target --hints:on --define:ssl $options $file" +""" + +{.define: ssl.} + +import std/[asyncdispatch, asyncnet, net, openssl] + +var port0: Port +var checked = 0 + +proc server {.async.} = + let sock = newAsyncSocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, buffered = true) + doAssert sock != nil + defer: sock.close() + let sslCtx = newContext( + protSSLv23, + verifyMode = CVerifyNone, + certFile = "tests/testdata/mycert.pem", + keyFile = "tests/testdata/mycert.pem" + ) + doAssert sslCtx != nil + defer: sslCtx.destroyContext() + wrapSocket(sslCtx, sock) + #sock.bindAddr(Port 8181) + sock.bindAddr() + port0 = getLocalAddr(sock)[1] + sock.listen() + echo "accept" + let clientSocket = await sock.accept() + defer: clientSocket.close() + wrapConnectedSocket( + sslCtx, clientSocket, handshakeAsServer, "localhost" + ) + let sdata = "x" & newString(41) + let sfut = clientSocket.send(sdata) + let rdata = newString(42) + let rfut = clientSocket.recvInto(addr rdata[0], rdata.len) + echo "send" + await sfut + echo "recv" + let rLen = await rfut # it hang here until the client closes the connection or sends more data + doAssert rLen == 42, $rLen + doAssert rdata[0] == 'x', rdata[0] + echo "ok" + inc checked + +proc client {.async.} = + let sock = newAsyncSocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, buffered = true) + doAssert sock != nil + defer: sock.close() + let sslCtx = newContext( + protSSLv23, + verifyMode = CVerifyNone + ) + doAssert sslCtx != nil + defer: sslCtx.destroyContext() + wrapSocket(sslCtx, sock) + #await sock.connect("127.0.0.1", Port 8181) + await sock.connect("localhost", port0) + let sdata = "x" & newString(41) + echo "send" + await sock.send(sdata) + let rdata = newString(42) + echo "recv" + let rLen = await sock.recvInto(addr rdata[0], rdata.len) + doAssert rLen == 42, $rLen + doAssert rdata[0] == 'x', rdata[0] + #await sleepAsync(10_000) + #await sock.send("x") + echo "ok" + inc checked + +discard getGlobalDispatcher() +let serverFut = server() +waitFor client() +waitFor serverFut +doAssert checked == 2 From 2b5f267e0c4879fbf230301e69eb7300e49aad0c Mon Sep 17 00:00:00 2001 From: nitely Date: Sun, 27 Apr 2025 21:25:55 -0300 Subject: [PATCH 7/7] wip --- tests/async/t24895.nim | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/async/t24895.nim b/tests/async/t24895.nim index a37bb505a7d3f..56d0d1268c64c 100644 --- a/tests/async/t24895.nim +++ b/tests/async/t24895.nim @@ -41,7 +41,7 @@ proc server {.async.} = echo "recv" let rLen = await rfut # it hang here until the client closes the connection or sends more data doAssert rLen == 42, $rLen - doAssert rdata[0] == 'x', rdata[0] + doAssert rdata[0] == 'x', $rdata[0] echo "ok" inc checked @@ -65,7 +65,7 @@ proc client {.async.} = echo "recv" let rLen = await sock.recvInto(addr rdata[0], rdata.len) doAssert rLen == 42, $rLen - doAssert rdata[0] == 'x', rdata[0] + doAssert rdata[0] == 'x', $rdata[0] #await sleepAsync(10_000) #await sock.send("x") echo "ok" @@ -76,3 +76,4 @@ let serverFut = server() waitFor client() waitFor serverFut doAssert checked == 2 +doAssert not hasPendingOperations()