From 87744f14405e5e07266bc868633388525b207863 Mon Sep 17 00:00:00 2001 From: Sam O'Connor Date: Sun, 4 Mar 2018 22:14:35 +1100 Subject: [PATCH 1/4] refine fix for deadlock by using non-blocking f_recv --- src/ssl.jl | 70 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index f8d845e..4acd251 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -24,7 +24,6 @@ Base.show(io::IO, c::SSLConfig) = print(io, "MbedTLS.SSLConfig()") mutable struct SSLContext <: IO data::Ptr{Cvoid} datalock::ReentrantLock - nonblocking::Bool config::SSLConfig isopen::Bool bio @@ -33,7 +32,6 @@ mutable struct SSLContext <: IO ctx = new() ctx.data = Libc.malloc(1000) # 488 ctx.datalock = ReentrantLock() - ctx.nonblocking = false ccall((:mbedtls_ssl_init, libmbedtls), Cvoid, (Ptr{Cvoid},), ctx.data) @compat finalizer(ctx->begin ccall((:mbedtls_ssl_free, libmbedtls), Cvoid, (Ptr{Cvoid},), ctx.data) @@ -113,24 +111,23 @@ end function f_send(c_ctx, c_msg, sz) jl_ctx = unsafe_pointer_to_objref(c_ctx) - jl_msg = unsafe_wrap(Array, c_msg, sz) - return Cint(write(jl_ctx.bio, jl_msg)) + return Cint(unsafe_write(jl_ctx, c_msg, sz)) end function f_recv(c_ctx, c_msg, sz) jl_ctx = unsafe_pointer_to_objref(c_ctx) - jl_msg = unsafe_wrap(Array, c_msg, sz) - if jl_ctx.nonblocking && nb_available(jl_ctx.bio) == 0 - n = 0 - else - n = readbytes!(jl_ctx.bio, jl_msg, sz) + n = nb_available(jl_ctx) + if n == 0 + return Cint(MBEDTLS_ERR_SSL_WANT_READ) end + n = min(sz, n) + unsafe_read(jl_ctx, c_msg, n) return Cint(n) end function set_bio!(ssl_ctx::SSLContext, jl_ctx::T) where {T<:IO} ssl_ctx.bio = jl_ctx - set_bio!(ssl_ctx, pointer_from_objref(ssl_ctx), c_send[], c_recv[]) + set_bio!(ssl_ctx, pointer_from_objref(ssl_ctx.bio), c_send[], c_recv[]) nothing end @@ -165,12 +162,21 @@ function set_dbg_level(level) end function handshake(ctx::SSLContext) - @lockdata ctx begin - @err_check ccall((:mbedtls_ssl_handshake, libmbedtls), Cint, - (Ptr{Cvoid},), ctx.data) - ctx.isopen = true + while true + n = @lockdata ctx begin + ccall((:mbedtls_ssl_handshake, libmbedtls), Cint, + (Ptr{Cvoid},), ctx.data) + end + if n == 0 + break + end + if n != MBEDTLS_ERR_SSL_WANT_READ + mbed_err(n) + end + Base.wait_readnb(ctx.bio, 1) end - nothing + ctx.isopen = true + return end function set_alpn!(conf::SSLConfig, protos) @@ -216,9 +222,11 @@ function Base.unsafe_read(ctx::SSLContext, buf::Ptr{UInt8}, nbytes::UInt; err=tr if n == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY || n == 0 ctx.isopen = false err ? throw(EOFError()) : return nread - end - if n != MBEDTLS_ERR_SSL_WANT_READ - n < 0 && mbed_err(n) + elseif n == MBEDTLS_ERR_SSL_WANT_READ + Base.wait_readnb(ctx.bio, 1) + elseif n < 0 + mbed_err(n) + else nread += n end end @@ -281,19 +289,21 @@ function get_ciphersuite(ctx::SSLContext) end function Base.nb_available(ctx::SSLContext) + @lockdata ctx begin - # First try to read from the socket and decrypt incoming data if - # possible. https://esp32.com/viewtopic.php?t=1101#p4884 - ctx.nonblocking = true - try - ccall((:mbedtls_ssl_read, libmbedtls), - Cint, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), ctx.data, C_NULL, 0) - finally - ctx.nonblocking = false - end - n = ccall((:mbedtls_ssl_get_bytes_avail, libmbedtls), - Csize_t, (Ptr{Cvoid},), ctx.data) - return Int(n) + + # First do a zero-byte read. + # This causes MbedTLS to call f_recv (which is always non-blocking) + # and decrypt any bytes that are already in the LibuvStream read buffer. + # https://esp32.com/viewtopic.php?t=1101#p4884 + ccall((:mbedtls_ssl_read, libmbedtls), Cint, + (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), + ctx.data, C_NULL, 0) + + # Now that the bufferd bytes have been processed, find out how many + # decrypted bytes are available. + return Int(ccall((:mbedtls_ssl_get_bytes_avail, libmbedtls), + Csize_t, (Ptr{Cvoid},), ctx.data)) end end From 5afe034f1b7f53d6249487c5600e14231af797ef Mon Sep 17 00:00:00 2001 From: Sam O'Connor Date: Mon, 5 Mar 2018 07:48:32 +1100 Subject: [PATCH 2/4] use eof instead of wait_readnb --- src/ssl.jl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 4acd251..37bad82 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -162,7 +162,7 @@ function set_dbg_level(level) end function handshake(ctx::SSLContext) - while true + while !eof(ctx.bio) n = @lockdata ctx begin ccall((:mbedtls_ssl_handshake, libmbedtls), Cint, (Ptr{Cvoid},), ctx.data) @@ -173,7 +173,6 @@ function handshake(ctx::SSLContext) if n != MBEDTLS_ERR_SSL_WANT_READ mbed_err(n) end - Base.wait_readnb(ctx.bio, 1) end ctx.isopen = true return @@ -213,7 +212,7 @@ Base.write(ctx::SSLContext, msg::UInt8) = write(ctx, Ref(msg)) function Base.unsafe_read(ctx::SSLContext, buf::Ptr{UInt8}, nbytes::UInt; err=true) nread::UInt = 0 - while nread < nbytes + while !eof(ctx.bio) && nread < nbytes n = @lockdata ctx begin ccall((:mbedtls_ssl_read, libmbedtls), Cint, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), @@ -223,7 +222,7 @@ function Base.unsafe_read(ctx::SSLContext, buf::Ptr{UInt8}, nbytes::UInt; err=tr ctx.isopen = false err ? throw(EOFError()) : return nread elseif n == MBEDTLS_ERR_SSL_WANT_READ - Base.wait_readnb(ctx.bio, 1) + continue elseif n < 0 mbed_err(n) else From 6a0391a0bfd42c28ab02d52d605b0b25db7035b7 Mon Sep 17 00:00:00 2001 From: Sam O'Connor Date: Mon, 5 Mar 2018 08:13:22 +1100 Subject: [PATCH 3/4] dont call eof until after calling mbedtls ccalls. otherwise eof() will block before mbedtls has a chance to do its thing --- src/ssl.jl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 37bad82..0ed7fa6 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -162,7 +162,7 @@ function set_dbg_level(level) end function handshake(ctx::SSLContext) - while !eof(ctx.bio) + while true n = @lockdata ctx begin ccall((:mbedtls_ssl_handshake, libmbedtls), Cint, (Ptr{Cvoid},), ctx.data) @@ -173,6 +173,7 @@ function handshake(ctx::SSLContext) if n != MBEDTLS_ERR_SSL_WANT_READ mbed_err(n) end + eof(ctx.bio) # Wait for more data to arrive end ctx.isopen = true return @@ -212,7 +213,7 @@ Base.write(ctx::SSLContext, msg::UInt8) = write(ctx, Ref(msg)) function Base.unsafe_read(ctx::SSLContext, buf::Ptr{UInt8}, nbytes::UInt; err=true) nread::UInt = 0 - while !eof(ctx.bio) && nread < nbytes + while nread < nbytes n = @lockdata ctx begin ccall((:mbedtls_ssl_read, libmbedtls), Cint, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), @@ -222,7 +223,7 @@ function Base.unsafe_read(ctx::SSLContext, buf::Ptr{UInt8}, nbytes::UInt; err=tr ctx.isopen = false err ? throw(EOFError()) : return nread elseif n == MBEDTLS_ERR_SSL_WANT_READ - continue + eof(ctx.bio) # Wait for more data to arrive elseif n < 0 mbed_err(n) else From 961b185a7f1ae423e33513920c692da1fcdf14d1 Mon Sep 17 00:00:00 2001 From: Sam O'Connor Date: Mon, 5 Mar 2018 09:06:32 +1100 Subject: [PATCH 4/4] define wait(ctx) = eof(ctx.bio) to make intention to block/wait more explicit --- src/ssl.jl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 0ed7fa6..8f67e72 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -161,6 +161,9 @@ function set_dbg_level(level) nothing end +Base.wait(ctx::SSLContext) = (eof(ctx.bio); nothing) + # eof blocks if the receive buffer is empty + function handshake(ctx::SSLContext) while true n = @lockdata ctx begin @@ -173,7 +176,7 @@ function handshake(ctx::SSLContext) if n != MBEDTLS_ERR_SSL_WANT_READ mbed_err(n) end - eof(ctx.bio) # Wait for more data to arrive + wait(ctx) end ctx.isopen = true return @@ -223,7 +226,7 @@ function Base.unsafe_read(ctx::SSLContext, buf::Ptr{UInt8}, nbytes::UInt; err=tr ctx.isopen = false err ? throw(EOFError()) : return nread elseif n == MBEDTLS_ERR_SSL_WANT_READ - eof(ctx.bio) # Wait for more data to arrive + wait(ctx) elseif n < 0 mbed_err(n) else