Skip to content

Commit 7436f38

Browse files
youknowoneyouknowone
committed
Fix ssl shutdown
1 parent 7eceb14 commit 7436f38

File tree

2 files changed

+186
-63
lines changed

2 files changed

+186
-63
lines changed

‎crates/stdlib/src/openssl.rs‎

Lines changed: 93 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2805,49 +2805,112 @@ mod _ssl {
28052805
let stream = self.connection.read();
28062806
let ssl_ptr = stream.ssl().as_ptr();
28072807

2808-
// Perform SSL shutdown - may need to be called twice:
2809-
// 1st call: sends close-notify, returns 0
2810-
// 2nd call: reads peer's close-notify, returns 1
2811-
let mut ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2812-
2813-
// If ret == 0, try once more to complete the bidirectional shutdown
2814-
// This handles the case where peer's close-notify is already available
2815-
if ret == 0 {
2816-
ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2808+
// BIO mode: just try shutdown once and raise SSLWantReadError if needed
2809+
if stream.is_bio() {
2810+
let ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2811+
if ret < 0 {
2812+
let err = unsafe { sys::SSL_get_error(ssl_ptr, ret) };
2813+
if err == sys::SSL_ERROR_WANT_READ {
2814+
return Err(create_ssl_want_read_error(vm).upcast());
2815+
} else if err == sys::SSL_ERROR_WANT_WRITE {
2816+
return Err(create_ssl_want_write_error(vm).upcast());
2817+
} else {
2818+
return Err(new_ssl_error(
2819+
vm,
2820+
format!("SSL shutdown failed: error code {}", err),
2821+
));
2822+
}
2823+
} else if ret == 0 {
2824+
// Sent close-notify, waiting for peer's - raise SSLWantReadError
2825+
return Err(create_ssl_want_read_error(vm).upcast());
2826+
}
2827+
return Ok(None);
28172828
}
28182829

2819-
if ret < 0 {
2820-
// Error occurred
2830+
// Socket mode: loop with select to wait for peer's close-notify
2831+
let socket_stream = stream.get_ref().expect("get_ref() failed for socket mode");
2832+
let deadline = socket_stream.timeout_deadline();
2833+
2834+
// Track how many times we've seen ret == 0 (max 2 tries)
2835+
let mut zeros = 0;
2836+
2837+
loop {
2838+
let ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2839+
2840+
// ret > 0: complete shutdown
2841+
if ret > 0 {
2842+
break;
2843+
}
2844+
2845+
// ret == 0: sent our close-notify, need to receive peer's
2846+
if ret == 0 {
2847+
zeros += 1;
2848+
if zeros > 1 {
2849+
// Already tried twice, break out (legacy behavior)
2850+
break;
2851+
}
2852+
// Wait briefly for peer's close_notify before retrying
2853+
match socket_stream.select(SslNeeds::Read, &deadline) {
2854+
SelectRet::TimedOut => break, // Timeout waiting for peer
2855+
SelectRet::Closed => break, // Socket closed
2856+
SelectRet::Nonblocking => {
2857+
// Non-blocking, just continue
2858+
}
2859+
SelectRet::Ok => {
2860+
// Data available, continue to retry
2861+
}
2862+
}
2863+
continue;
2864+
}
2865+
2866+
// ret < 0: error or would-block
28212867
let err = unsafe { sys::SSL_get_error(ssl_ptr, ret) };
28222868

2823-
if err == sys::SSL_ERROR_WANT_READ {
2824-
return Err(create_ssl_want_read_error(vm).upcast());
2869+
let needs = if err == sys::SSL_ERROR_WANT_READ {
2870+
SslNeeds::Read
28252871
} else if err == sys::SSL_ERROR_WANT_WRITE {
2826-
return Err(create_ssl_want_write_error(vm).upcast());
2872+
SslNeeds::Write
28272873
} else {
2874+
// Real error
28282875
return Err(new_ssl_error(
28292876
vm,
28302877
format!("SSL shutdown failed: error code {}", err),
28312878
));
2832-
}
2833-
} else if ret == 0 {
2834-
// Still waiting for peer's close-notify after retry
2835-
// In BIO mode, raise SSLWantReadError
2836-
if stream.is_bio() {
2837-
return Err(create_ssl_want_read_error(vm).upcast());
2838-
}
2839-
}
2879+
};
28402880

2841-
// BIO mode doesn't have an underlying socket to return
2842-
if stream.is_bio() {
2843-
return Ok(None);
2881+
// Wait on the socket
2882+
match socket_stream.select(needs, &deadline) {
2883+
SelectRet::TimedOut => {
2884+
let msg = if err == sys::SSL_ERROR_WANT_READ {
2885+
"The read operation timed out"
2886+
} else {
2887+
"The write operation timed out"
2888+
};
2889+
return Err(vm.new_exception_msg(
2890+
vm.ctx.exceptions.timeout_error.to_owned(),
2891+
msg.to_owned(),
2892+
));
2893+
}
2894+
SelectRet::Closed => {
2895+
return Err(socket_closed_error(vm));
2896+
}
2897+
SelectRet::Nonblocking => {
2898+
// Non-blocking socket, raise SSLWantReadError/SSLWantWriteError
2899+
if err == sys::SSL_ERROR_WANT_READ {
2900+
return Err(create_ssl_want_read_error(vm).upcast());
2901+
} else {
2902+
return Err(create_ssl_want_write_error(vm).upcast());
2903+
}
2904+
}
2905+
SelectRet::Ok => {
2906+
// Socket is ready, retry shutdown
2907+
continue;
2908+
}
2909+
}
28442910
}
28452911

2846-
// Return the underlying socket for socket mode
2847-
let socket = stream
2848-
.get_ref()
2849-
.expect("unwrap() called on bio mode; should only be called in socket mode");
2850-
Ok(Some(socket.0.clone()))
2912+
// Return the underlying socket
2913+
Ok(Some(socket_stream.0.clone()))
28512914
}
28522915

28532916
#[cfg(osslconf = "OPENSSL_NO_COMP")]

‎crates/stdlib/src/ssl.rs‎

Lines changed: 93 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4119,45 +4119,105 @@ mod _ssl {
41194119
peer_closed = true;
41204120
}
41214121
} else if let Some(timeout) = timeout_mode {
4122-
// All socket modes (blocking, timeout, non-blocking):
4123-
// Return immediately after sending our close_notify.
4124-
//
4125-
// This matches CPython/OpenSSL behavior where SSL_shutdown()
4126-
// returns after sending close_notify, allowing the app to
4127-
// close the socket without waiting for peer's close_notify.
4128-
//
4129-
// Waiting for peer's close_notify can cause deadlock with
4130-
// asyncore-based servers where both sides wait for the other's
4131-
// close_notify before closing the connection.
4132-
4133-
// Ensure all pending TLS data is sent before returning
4134-
// This prevents data loss when rustls drains its buffer
4135-
// but the socket couldn't accept all data immediately
4136-
drop(conn_guard);
4137-
4138-
// Respect socket timeout settings for flushing pending TLS data
41394122
match timeout {
41404123
Some(0.0) => {
4141-
// Non-blocking: best-effort flush, ignore errors
4142-
// to avoid deadlock with asyncore-based servers
4124+
// Non-blocking: return immediately after sending close_notify.
4125+
// Don't wait for peer's close_notify to avoid blocking.
4126+
drop(conn_guard);
41434127
let _ = self.flush_pending_tls_output(vm, None);
4128+
*self.shutdown_state.lock() = ShutdownState::Completed;
4129+
*self.connection.lock() = None;
4130+
return Ok(self.sock.clone());
41444131
}
4145-
Some(_t) => {
4146-
// Timeout mode: use flush with socket's timeout
4147-
// Errors (including timeout) are propagated to caller
4148-
self.flush_pending_tls_output(vm, None)?;
4149-
}
4150-
None => {
4151-
// Blocking mode: wait until all pending data is sent
4152-
self.blocking_flush_all_pending(vm)?;
4132+
_ => {
4133+
// Blocking or timeout mode: wait for peer's close_notify.
4134+
// This is proper TLS shutdown - we should receive peer's
4135+
// close_notify before closing the connection.
4136+
drop(conn_guard);
4137+
4138+
// Flush our close_notify first
4139+
if timeout.is_none() {
4140+
self.blocking_flush_all_pending(vm)?;
4141+
} else {
4142+
self.flush_pending_tls_output(vm, None)?;
4143+
}
4144+
4145+
// Calculate deadline for timeout mode
4146+
let deadline = timeout.map(|t| {
4147+
std::time::Instant::now() + std::time::Duration::from_secs_f64(t)
4148+
});
4149+
4150+
// Wait for peer's close_notify
4151+
loop {
4152+
// Re-acquire connection lock for each iteration
4153+
let mut conn_guard = self.connection.lock();
4154+
let conn = match conn_guard.as_mut() {
4155+
Some(c) => c,
4156+
None => break, // Connection already closed
4157+
};
4158+
4159+
// Check if peer already sent close_notify
4160+
if self.check_peer_closed(conn, vm)? {
4161+
break;
4162+
}
4163+
4164+
drop(conn_guard);
4165+
4166+
// Check timeout
4167+
let remaining_timeout = if let Some(dl) = deadline {
4168+
let now = std::time::Instant::now();
4169+
if now >= dl {
4170+
// Timeout reached - complete shutdown anyway
4171+
break;
4172+
}
4173+
Some(dl - now)
4174+
} else {
4175+
None // Blocking mode: no timeout
4176+
};
4177+
4178+
// Wait for socket to be readable
4179+
let timed_out = self.sock_wait_for_io_with_timeout(
4180+
SelectKind::Read,
4181+
remaining_timeout,
4182+
vm,
4183+
)?;
4184+
4185+
if timed_out {
4186+
// Timeout waiting for peer's close_notify
4187+
break;
4188+
}
4189+
4190+
// Try to read data from socket
4191+
let mut conn_guard = self.connection.lock();
4192+
let conn = match conn_guard.as_mut() {
4193+
Some(c) => c,
4194+
None => break,
4195+
};
4196+
4197+
// Read and process any incoming TLS data
4198+
match self.try_read_close_notify(conn, vm) {
4199+
Ok(closed) => {
4200+
if closed {
4201+
break;
4202+
}
4203+
// Check again after processing
4204+
if self.check_peer_closed(conn, vm)? {
4205+
break;
4206+
}
4207+
}
4208+
Err(_) => {
4209+
// Socket error - peer likely closed connection
4210+
break;
4211+
}
4212+
}
4213+
}
4214+
4215+
// Shutdown complete
4216+
*self.shutdown_state.lock() = ShutdownState::Completed;
4217+
*self.connection.lock() = None;
4218+
return Ok(self.sock.clone());
41534219
}
41544220
}
4155-
4156-
// Set shutdown_state first to ensure atomic visibility
4157-
// This prevents read/write race conditions during shutdown
4158-
*self.shutdown_state.lock() = ShutdownState::Completed;
4159-
*self.connection.lock() = None;
4160-
return Ok(self.sock.clone());
41614221
}
41624222

41634223
// Step 3: Check again if peer has sent close_notify (non-blocking/BIO mode only)

0 commit comments

Comments
 (0)