diff --git a/crates/stdlib/src/openssl.rs b/crates/stdlib/src/openssl.rs index e07ad552f17..b6d5f5c2035 100644 --- a/crates/stdlib/src/openssl.rs +++ b/crates/stdlib/src/openssl.rs @@ -53,11 +53,10 @@ cfg_if::cfg_if! { mod _ssl { use super::{bio, probe}; - // Import error types used in this module (others are exposed via pymodule(with(...))) + // Import error types and helpers used in this module (others are exposed via pymodule(with(...))) use super::ssl_error::{ - PySSLCertVerificationError as PySslCertVerificationError, PySSLEOFError as PySslEOFError, - PySSLError as PySslError, PySSLWantReadError as PySslWantReadError, - PySSLWantWriteError as PySslWantWriteError, + PySSLCertVerificationError, PySSLError, create_ssl_eof_error, create_ssl_want_read_error, + create_ssl_want_write_error, }; use crate::{ common::lock::{ @@ -249,8 +248,6 @@ mod _ssl { #[pyattr] const VERIFY_DEFAULT: u32 = 0; #[pyattr] - const SSL_ERROR_EOF: u32 = 8; // custom for python - #[pyattr] const HAS_SNI: bool = true; #[pyattr] const HAS_ECDH: bool = true; @@ -709,21 +706,124 @@ mod _ssl { } } + // OpenSSL record type constants for msg_callback + const SSL3_RT_CHANGE_CIPHER_SPEC: i32 = 20; + const SSL3_RT_ALERT: i32 = 21; + const SSL3_RT_HANDSHAKE: i32 = 22; + const SSL3_RT_HEADER: i32 = 256; + const SSL3_RT_INNER_CONTENT_TYPE: i32 = 257; + // Special value for change cipher spec (CPython compatibility) + const SSL3_MT_CHANGE_CIPHER_SPEC: i32 = 0x0101; + // Message callback function called by OpenSSL - // NOTE: This callback is intentionally a no-op to avoid deadlocks. - // The msg_callback can be called during various SSL operations (read, write, handshake), - // and invoking Python code from within these operations can cause deadlocks - // (see CPython bpo-43577). A proper implementation would require careful lock ordering. + // Called during SSL operations to report protocol messages. + // debughelpers.c:_PySSL_msg_callback unsafe extern "C" fn _msg_callback( - _write_p: libc::c_int, - _version: libc::c_int, - _content_type: libc::c_int, - _buf: *const libc::c_void, - _len: usize, - _ssl_ptr: *mut sys::SSL, + write_p: libc::c_int, + mut version: libc::c_int, + content_type: libc::c_int, + buf: *const libc::c_void, + len: usize, + ssl_ptr: *mut sys::SSL, _arg: *mut libc::c_void, ) { - // Intentionally empty to avoid deadlocks + if ssl_ptr.is_null() { + return; + } + + unsafe { + // Get SSL socket from SSL_get_ex_data (index 0) + let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, 0); + if ssl_socket_ptr.is_null() { + return; + } + + // ssl_socket_ptr is a pointer to Py, set in _wrap_socket/_wrap_bio + let ssl_socket: &Py = &*(ssl_socket_ptr as *const Py); + + // Get the callback from the context + let callback_opt = ssl_socket.ctx.read().msg_callback.lock().clone(); + let Some(callback) = callback_opt else { + return; + }; + + // Get VM from thread-local storage (set by HandshakeVmGuard in do_handshake) + let Some(vm_ptr) = HANDSHAKE_VM.with(|cell| cell.get()) else { + // VM not available - this shouldn't happen during handshake + return; + }; + let vm = &*vm_ptr; + + // Get SSL socket owner object + let ssl_socket_obj = ssl_socket + .owner + .read() + .as_ref() + .and_then(|weak| weak.upgrade()) + .unwrap_or_else(|| vm.ctx.none()); + + // Create the message bytes + let buf_slice = std::slice::from_raw_parts(buf as *const u8, len); + let msg_bytes = vm.ctx.new_bytes(buf_slice.to_vec()); + + // Determine direction string + let direction_str = if write_p != 0 { "write" } else { "read" }; + + // Calculate msg_type based on content_type (debughelpers.c behavior) + let msg_type = match content_type { + SSL3_RT_CHANGE_CIPHER_SPEC => SSL3_MT_CHANGE_CIPHER_SPEC, + SSL3_RT_ALERT => { + // byte 1 is alert type + if len >= 2 { buf_slice[1] as i32 } else { -1 } + } + SSL3_RT_HANDSHAKE => { + // byte 0 is handshake type + if !buf_slice.is_empty() { + buf_slice[0] as i32 + } else { + -1 + } + } + SSL3_RT_HEADER => { + // Frame header: version in bytes 1..2, type in byte 0 + if len >= 3 { + version = ((buf_slice[1] as i32) << 8) | (buf_slice[2] as i32); + buf_slice[0] as i32 + } else { + -1 + } + } + SSL3_RT_INNER_CONTENT_TYPE => { + // Inner content type in byte 0 + if !buf_slice.is_empty() { + buf_slice[0] as i32 + } else { + -1 + } + } + _ => -1, + }; + + // Call the Python callback + // Signature: callback(conn, direction, version, content_type, msg_type, data) + match callback.call( + ( + ssl_socket_obj, + vm.ctx.new_str(direction_str), + vm.ctx.new_int(version), + vm.ctx.new_int(content_type), + vm.ctx.new_int(msg_type), + msg_bytes, + ), + vm, + ) { + Ok(_) => {} + Err(exc) => { + // Log the exception but don't propagate it + vm.run_unraisable(exc, None, vm.ctx.none()); + } + } + } } #[pyfunction(name = "RAND_pseudo_bytes")] @@ -794,6 +894,7 @@ mod _ssl { SslVerifyMode::NONE }); + // Start with OP_ALL but remove options that CPython doesn't include by default let mut options = SslOptions::ALL & !SslOptions::DONT_INSERT_EMPTY_FRAGMENTS; if proto != SslVersion::Ssl2 { options |= SslOptions::NO_SSLV2; @@ -807,6 +908,8 @@ mod _ssl { options |= SslOptions::SINGLE_ECDH_USE; options |= SslOptions::ENABLE_MIDDLEBOX_COMPAT; builder.set_options(options); + // Remove NO_TLSv1 and NO_TLSv1_1 which newer OpenSSL adds to OP_ALL + builder.clear_options(SslOptions::NO_TLSV1 | SslOptions::NO_TLSV1_1); let mode = ssl::SslMode::ACCEPT_MOVING_WRITE_BUFFER | ssl::SslMode::AUTO_RETRY; builder.set_mode(mode); @@ -1242,7 +1345,7 @@ mod _ssl { if let Some(cadata) = args.cadata { let certs = match cadata { Either::A(s) => { - if !s.is_ascii() { + if !s.as_str().is_ascii() { return Err(invalid_cadata(vm)); } X509::stack_from_pem(s.as_bytes()) @@ -1261,6 +1364,29 @@ mod _ssl { if args.cafile.is_some() || args.capath.is_some() { let cafile_path = args.cafile.map(|p| p.to_path_buf(vm)).transpose()?; let capath_path = args.capath.map(|p| p.to_path_buf(vm)).transpose()?; + // Check file/directory existence before calling OpenSSL to get proper errno + if let Some(ref path) = cafile_path { + if !path.exists() { + return Err(vm + .new_os_subtype_error( + vm.ctx.exceptions.file_not_found_error.to_owned(), + Some(libc::ENOENT), + format!("No such file or directory: '{}'", path.display()), + ) + .upcast()); + } + } + if let Some(ref path) = capath_path { + if !path.exists() { + return Err(vm + .new_os_subtype_error( + vm.ctx.exceptions.file_not_found_error.to_owned(), + Some(libc::ENOENT), + format!("No such file or directory: '{}'", path.display()), + ) + .upcast()); + } + } ctx.load_verify_locations(cafile_path.as_deref(), capath_path.as_deref()) .map_err(|e| convert_openssl_error(vm, e))?; } @@ -1387,7 +1513,7 @@ mod _ssl { std::io::ErrorKind::NotFound => vm .new_os_subtype_error( vm.ctx.exceptions.file_not_found_error.to_owned(), - None, + Some(libc::ENOENT), e.to_string(), ) .upcast(), @@ -1559,6 +1685,27 @@ mod _ssl { let mut ctx = self.builder(); let key_path = keyfile.map(|path| path.to_path_buf(vm)).transpose()?; let cert_path = certfile.to_path_buf(vm)?; + // Check file existence before calling OpenSSL to get proper errno + if !cert_path.exists() { + return Err(vm + .new_os_subtype_error( + vm.ctx.exceptions.file_not_found_error.to_owned(), + Some(libc::ENOENT), + format!("No such file or directory: '{}'", cert_path.display()), + ) + .upcast()); + } + if let Some(ref kp) = key_path { + if !kp.exists() { + return Err(vm + .new_os_subtype_error( + vm.ctx.exceptions.file_not_found_error.to_owned(), + Some(libc::ENOENT), + format!("No such file or directory: '{}'", kp.display()), + ) + .upcast()); + } + } ctx.set_certificate_chain_file(&cert_path) .and_then(|()| { ctx.set_private_key_file( @@ -1623,7 +1770,7 @@ mod _ssl { )); } if hostname_str.contains('\0') { - return Err(vm.new_value_error("embedded null byte in server_hostname")); + return Err(vm.new_type_error("embedded null character")); } let ip = hostname_str.parse::(); if ip.is_err() { @@ -1704,12 +1851,19 @@ mod _ssl { // Check if SNI callback is configured (minimize lock time) let has_sni_callback = zelf.sni_callback.lock().is_some(); - // Set SNI callback data if needed (after releasing the lock) - if has_sni_callback { - let ssl_socket_weak = py_ref.as_object().downgrade(None, vm)?; - unsafe { - let ssl_ptr = py_ref.connection.read().ssl().as_ptr(); + // Set up ex_data for callbacks + unsafe { + let ssl_ptr = py_ref.connection.read().ssl().as_ptr(); + + // Store ssl_socket pointer in index 0 for msg_callback (like CPython's SSL_set_app_data) + // This is safe because ssl_socket owns the SSL object and outlives it + // We store a pointer to Py, which msg_callback can dereference + let py_ptr: *const Py = &*py_ref; + sys::SSL_set_ex_data(ssl_ptr, 0, py_ptr as *mut _); + // Set SNI callback data if needed + if has_sni_callback { + let ssl_socket_weak = py_ref.as_object().downgrade(None, vm)?; // Store callback data in SSL ex_data - use weak reference to avoid cycle let callback_data = Box::new(SniCallbackData { ssl_context: zelf.clone(), @@ -1765,12 +1919,19 @@ mod _ssl { // Check if SNI callback is configured (minimize lock time) let has_sni_callback = zelf.sni_callback.lock().is_some(); - // Set SNI callback data if needed (after releasing the lock) - if has_sni_callback { - let ssl_socket_weak = py_ref.as_object().downgrade(None, vm)?; - unsafe { - let ssl_ptr = py_ref.connection.read().ssl().as_ptr(); + // Set up ex_data for callbacks + unsafe { + let ssl_ptr = py_ref.connection.read().ssl().as_ptr(); + // Store ssl_socket pointer in index 0 for msg_callback (like CPython's SSL_set_app_data) + // This is safe because ssl_socket owns the SSL object and outlives it + // We store a pointer to Py, which msg_callback can dereference + let py_ptr: *const Py = &*py_ref; + sys::SSL_set_ex_data(ssl_ptr, 0, py_ptr as *mut _); + + // Set SNI callback data if needed + if has_sni_callback { + let ssl_socket_weak = py_ref.as_object().downgrade(None, vm)?; // Store callback data in SSL ex_data - use weak reference to avoid cycle let callback_data = Box::new(SniCallbackData { ssl_context: zelf.clone(), @@ -1866,12 +2027,14 @@ mod _ssl { Some(s) => s, None => return SelectRet::Closed, }; - let deadline = match &deadline { + // For blocking sockets without timeout, call sock_select with None timeout + // to actually block waiting for data instead of busy-looping + let timeout = match &deadline { Ok(deadline) => match deadline.checked_duration_since(Instant::now()) { - Some(deadline) => deadline, + Some(d) => Some(d), None => return SelectRet::TimedOut, }, - Err(true) => return SelectRet::IsBlocking, + Err(true) => None, // Blocking: no timeout, wait indefinitely Err(false) => return SelectRet::Nonblocking, }; let res = socket::sock_select( @@ -1880,7 +2043,7 @@ mod _ssl { SslNeeds::Read => socket::SelectKind::Read, SslNeeds::Write => socket::SelectKind::Write, }, - Some(deadline), + timeout, ); match res { Ok(true) => SelectRet::TimedOut, @@ -2017,6 +2180,14 @@ mod _ssl { SslConnection::Bio(stream) => stream.get_shutdown(), } } + + // Check if incoming BIO has EOF (for BIO mode only) + fn is_bio_eof(&self) -> bool { + match self { + SslConnection::Socket(_) => false, + SslConnection::Bio(stream) => stream.get_ref().inbio.eof_written.load(), + } + } } #[pyattr] @@ -2172,17 +2343,21 @@ mod _ssl { #[pymethod] fn version(&self) -> Option<&'static str> { - let v = self.connection.read().ssl().version_str(); + // Use thread-local SSL pointer during handshake to avoid deadlock + let ssl_ptr = get_ssl_ptr_for_context_change(&self.connection); + // Return None if handshake is not complete (CPython behavior) + if unsafe { sys::SSL_is_init_finished(ssl_ptr) } == 0 { + return None; + } + let v = unsafe { ssl::SslRef::from_ptr(ssl_ptr).version_str() }; if v == "unknown" { None } else { Some(v) } } #[pymethod] fn cipher(&self) -> Option { - self.connection - .read() - .ssl() - .current_cipher() - .map(cipher_to_tuple) + // Use thread-local SSL pointer during handshake to avoid deadlock + let ssl_ptr = get_ssl_ptr_for_context_change(&self.connection); + unsafe { ssl::SslRef::from_ptr(ssl_ptr).current_cipher() }.map(cipher_to_tuple) } #[pymethod] @@ -2195,14 +2370,15 @@ mod _ssl { fn shared_ciphers(&self, vm: &VirtualMachine) -> Option { #[cfg(ossl110)] { - let stream = self.connection.read(); + // Use thread-local SSL pointer during handshake to avoid deadlock + let ssl_ptr = get_ssl_ptr_for_context_change(&self.connection); unsafe { - let server_ciphers = SSL_get_ciphers(stream.ssl().as_ptr()); + let server_ciphers = SSL_get_ciphers(ssl_ptr); if server_ciphers.is_null() { return None; } - let client_ciphers = SSL_get_client_ciphers(stream.ssl().as_ptr()); + let client_ciphers = SSL_get_client_ciphers(ssl_ptr); if client_ciphers.is_null() { return None; } @@ -2258,12 +2434,13 @@ mod _ssl { fn selected_alpn_protocol(&self) -> Option { #[cfg(ossl102)] { - let stream = self.connection.read(); + // Use thread-local SSL pointer during handshake to avoid deadlock + let ssl_ptr = get_ssl_ptr_for_context_change(&self.connection); unsafe { let mut out: *const libc::c_uchar = std::ptr::null(); let mut outlen: libc::c_uint = 0; - sys::SSL_get0_alpn_selected(stream.ssl().as_ptr(), &mut out, &mut outlen); + sys::SSL_get0_alpn_selected(ssl_ptr, &mut out, &mut outlen); if out.is_null() { None @@ -2343,42 +2520,53 @@ mod _ssl { } #[pymethod] - fn shutdown(&self, vm: &VirtualMachine) -> PyResult> { + fn shutdown(&self, vm: &VirtualMachine) -> PyResult>> { let stream = self.connection.read(); - - // BIO mode doesn't have an underlying socket - if stream.is_bio() { - return Err(vm.new_not_implemented_error( - "shutdown() is not supported for BIO-based SSL objects".to_owned(), - )); - } - let ssl_ptr = stream.ssl().as_ptr(); - // Perform SSL shutdown - let ret = unsafe { sys::SSL_shutdown(ssl_ptr) }; + // Perform SSL shutdown - may need to be called twice: + // 1st call: sends close-notify, returns 0 + // 2nd call: reads peer's close-notify, returns 1 + let mut ret = unsafe { sys::SSL_shutdown(ssl_ptr) }; + + // If ret == 0, try once more to complete the bidirectional shutdown + // This handles the case where peer's close-notify is already available + if ret == 0 { + ret = unsafe { sys::SSL_shutdown(ssl_ptr) }; + } if ret < 0 { // Error occurred let err = unsafe { sys::SSL_get_error(ssl_ptr, ret) }; - if err == sys::SSL_ERROR_WANT_READ || err == sys::SSL_ERROR_WANT_WRITE { - // Non-blocking would block - this is okay for shutdown - // Return the underlying socket + if err == sys::SSL_ERROR_WANT_READ { + return Err(create_ssl_want_read_error(vm).upcast()); + } else if err == sys::SSL_ERROR_WANT_WRITE { + return Err(create_ssl_want_write_error(vm).upcast()); } else { return Err(new_ssl_error( vm, format!("SSL shutdown failed: error code {}", err), )); } + } else if ret == 0 { + // Still waiting for peer's close-notify after retry + // In BIO mode, raise SSLWantReadError + if stream.is_bio() { + return Err(create_ssl_want_read_error(vm).upcast()); + } + } + + // BIO mode doesn't have an underlying socket to return + if stream.is_bio() { + return Ok(None); } - // Return the underlying socket - // Get the socket from the stream (SocketStream wraps PyRef) + // Return the underlying socket for socket mode let socket = stream .get_ref() .expect("unwrap() called on bio mode; should only be called in socket mode"); - Ok(socket.0.clone()) + Ok(Some(socket.0.clone())) } #[cfg(osslconf = "OPENSSL_NO_COMP")] @@ -2389,8 +2577,9 @@ mod _ssl { #[cfg(not(osslconf = "OPENSSL_NO_COMP"))] #[pymethod] fn compression(&self) -> Option<&'static str> { - let stream = self.connection.read(); - let comp_method = unsafe { sys::SSL_get_current_compression(stream.ssl().as_ptr()) }; + // Use thread-local SSL pointer during handshake to avoid deadlock + let ssl_ptr = get_ssl_ptr_for_context_change(&self.connection); + let comp_method = unsafe { sys::SSL_get_current_compression(ssl_ptr) }; if comp_method.is_null() { return None; } @@ -2416,7 +2605,7 @@ mod _ssl { let result = stream.do_handshake().map_err(|e| { let exc = convert_ssl_error(vm, e); // If it's a cert verification error, set verify info - if exc.class().is(PySslCertVerificationError::class(&vm.ctx)) { + if exc.class().is(PySSLCertVerificationError::class(&vm.ctx)) { set_verify_error_info(&exc, ssl_ptr, vm); } exc @@ -2473,7 +2662,7 @@ mod _ssl { } let exc = convert_ssl_error(vm, err); // If it's a cert verification error, set verify info - if exc.class().is(PySslCertVerificationError::class(&vm.ctx)) { + if exc.class().is(PySSLCertVerificationError::class(&vm.ctx)) { set_verify_error_info(&exc, ssl_ptr, vm); } // Clean up SNI ex_data before returning error @@ -2603,19 +2792,41 @@ mod _ssl { #[pygetset] fn session_reused(&self) -> bool { - let stream = self.connection.read(); - unsafe { sys::SSL_session_reused(stream.ssl().as_ptr()) != 0 } + // Use thread-local SSL pointer during handshake to avoid deadlock + let ssl_ptr = get_ssl_ptr_for_context_change(&self.connection); + unsafe { sys::SSL_session_reused(ssl_ptr) != 0 } } #[pymethod] fn read( &self, - n: usize, + n: isize, buffer: OptionalArg, vm: &VirtualMachine, ) -> PyResult { + // Handle negative n: + // - If buffer is None and n < 0: raise ValueError + // - If buffer is present and n <= 0: use buffer length + // This matches _ssl__SSLSocket_read_impl in CPython + let read_len: usize = match &buffer { + OptionalArg::Present(buf) => { + let buf_len = buf.borrow_buf_mut().len(); + if n <= 0 || (n as usize) > buf_len { + buf_len + } else { + n as usize + } + } + OptionalArg::Missing => { + if n < 0 { + return Err(vm.new_value_error("size should not be negative".to_owned())); + } + n as usize + } + }; + // Special case: reading 0 bytes should return empty bytes immediately - if n == 0 { + if read_len == 0 { return if buffer.is_present() { Ok(vm.ctx.new_int(0).into()) } else { @@ -2627,13 +2838,13 @@ mod _ssl { let mut inner_buffer = if let OptionalArg::Present(buffer) = &buffer { Either::A(buffer.borrow_buf_mut()) } else { - Either::B(vec![0u8; n]) + Either::B(vec![0u8; read_len]) }; let buf = match &mut inner_buffer { Either::A(b) => &mut **b, Either::B(b) => b.as_mut_slice(), }; - let buf = match buf.get_mut(..n) { + let buf = match buf.get_mut(..read_len) { Some(b) => b, None => buf, }; @@ -2642,7 +2853,18 @@ mod _ssl { let count = if stream.is_bio() { match stream.ssl_read(buf) { Ok(count) => count, - Err(e) => return Err(convert_ssl_error(vm, e)), + Err(e) => { + // Handle ZERO_RETURN (EOF) - raise SSLEOFError + if e.code() == ssl::ErrorCode::ZERO_RETURN { + return Err(create_ssl_eof_error(vm).upcast()); + } + // If WANT_READ and the incoming BIO has EOF written, + // this is an unexpected EOF (transport closed without TLS close_notify) + if e.code() == ssl::ErrorCode::WANT_READ && stream.is_bio_eof() { + return Err(create_ssl_eof_error(vm).upcast()); + } + return Err(convert_ssl_error(vm, e)); + } } } else { // Socket mode: handle timeout and blocking @@ -3051,22 +3273,10 @@ mod _ssl { let len = size.unwrap_or(-1); let len = if len < 0 || len > avail { avail } else { len }; - // Check if EOF has been written and no data available - // This matches CPython's behavior where read() returns b'' when EOF is set - if len == 0 && self.eof_written.load() { - return Ok(Vec::new()); - } - + // When no data available, return empty bytes (CPython behavior) + // CPython returns empty bytes directly without calling BIO_read() if len == 0 { - // No data available and no EOF - would block - // Call BIO_read() to get the proper error (SSL_ERROR_WANT_READ) - let mut test_buf = [0u8; 1]; - let nbytes = sys::BIO_read(self.bio, test_buf.as_mut_ptr() as *mut _, 1); - if nbytes < 0 { - return Err(convert_openssl_error(vm, ErrorStack::get())); - } - // Shouldn't reach here, but if we do, return what we got - return Ok(test_buf[..nbytes as usize].to_vec()); + return Ok(Vec::new()); } let mut buf = vec![0u8; len as usize]; @@ -3175,7 +3385,7 @@ mod _ssl { /// Helper function to create SSL error with proper OSError subtype handling fn new_ssl_error(vm: &VirtualMachine, msg: impl ToString) -> PyBaseExceptionRef { - vm.new_os_subtype_error(PySslError::class(&vm.ctx).to_owned(), None, msg.to_string()) + vm.new_os_subtype_error(PySSLError::class(&vm.ctx).to_owned(), None, msg.to_string()) .upcast() } @@ -3235,9 +3445,9 @@ mod _ssl { // Use SSLCertVerificationError for certificate verification failures let cls = if is_cert_verify_error { - PySslCertVerificationError::class(&vm.ctx).to_owned() + PySSLCertVerificationError::class(&vm.ctx).to_owned() } else { - PySslError::class(&vm.ctx).to_owned() + PySSLError::class(&vm.ctx).to_owned() }; // Build message @@ -3278,7 +3488,7 @@ mod _ssl { ) } None => { - let cls = PySslError::class(&vm.ctx).to_owned(); + let cls = PySSLError::class(&vm.ctx).to_owned(); vm.new_os_subtype_error(cls, None, "unknown SSL error") .upcast() } @@ -3317,27 +3527,18 @@ mod _ssl { ) -> PyBaseExceptionRef { let e = e.borrow(); let (cls, msg) = match e.code() { - ssl::ErrorCode::WANT_READ => ( - PySslWantReadError::class(&vm.ctx).to_owned(), - "The operation did not complete (read)", - ), - ssl::ErrorCode::WANT_WRITE => ( - PySslWantWriteError::class(&vm.ctx).to_owned(), - "The operation did not complete (write)", - ), + ssl::ErrorCode::WANT_READ => { + return create_ssl_want_read_error(vm).upcast(); + } + ssl::ErrorCode::WANT_WRITE => { + return create_ssl_want_write_error(vm).upcast(); + } ssl::ErrorCode::SYSCALL => match e.io_error() { Some(io_err) => return io_err.to_pyexception(vm), // When no I/O error and OpenSSL error queue is empty, // this is an EOF in violation of protocol -> SSLEOFError - // Need to set args[0] = SSL_ERROR_EOF for suppress_ragged_eofs check None => { - return vm - .new_os_subtype_error( - PySslEOFError::class(&vm.ctx).to_owned(), - Some(SSL_ERROR_EOF as i32), - "EOF occurred in violation of protocol", - ) - .upcast(); + return create_ssl_eof_error(vm).upcast(); } }, ssl::ErrorCode::SSL => { @@ -3350,24 +3551,18 @@ mod _ssl { let reason = sys::ERR_GET_REASON(err_code); let lib = sys::ERR_GET_LIB(err_code); if lib == ERR_LIB_SSL && reason == SSL_R_UNEXPECTED_EOF_WHILE_READING { - return vm - .new_os_subtype_error( - PySslEOFError::class(&vm.ctx).to_owned(), - Some(SSL_ERROR_EOF as i32), - "EOF occurred in violation of protocol", - ) - .upcast(); + return create_ssl_eof_error(vm).upcast(); } } return convert_openssl_error(vm, ssl_err.clone()); } ( - PySslError::class(&vm.ctx).to_owned(), + PySSLError::class(&vm.ctx).to_owned(), "A failure in the SSL library occurred", ) } _ => ( - PySslError::class(&vm.ctx).to_owned(), + PySSLError::class(&vm.ctx).to_owned(), "A failure in the SSL library occurred", ), }; diff --git a/crates/stdlib/src/ssl.rs b/crates/stdlib/src/ssl.rs index 992b32e00ea..bf25260ed3a 100644 --- a/crates/stdlib/src/ssl.rs +++ b/crates/stdlib/src/ssl.rs @@ -207,28 +207,6 @@ mod _ssl { #[pyattr] const OP_ALL: i32 = 0x00000BFB; // Combined "safe" options (reduced for i32, excluding OP_LEGACY_SERVER_CONNECT for OpenSSL 3.0.0+ compatibility) - // Error types - #[pyattr] - const SSL_ERROR_NONE: i32 = 0; - #[pyattr] - const SSL_ERROR_SSL: i32 = 1; - #[pyattr] - const SSL_ERROR_WANT_READ: i32 = 2; - #[pyattr] - const SSL_ERROR_WANT_WRITE: i32 = 3; - #[pyattr] - const SSL_ERROR_WANT_X509_LOOKUP: i32 = 4; - #[pyattr] - const SSL_ERROR_SYSCALL: i32 = 5; - #[pyattr] - const SSL_ERROR_ZERO_RETURN: i32 = 6; - #[pyattr] - const SSL_ERROR_WANT_CONNECT: i32 = 7; - #[pyattr] - const SSL_ERROR_EOF: i32 = 8; - #[pyattr] - const SSL_ERROR_INVALID_ERROR_CODE: i32 = 10; - // Alert types (matching _TLSAlertType enum) #[pyattr] const ALERT_DESCRIPTION_CLOSE_NOTIFY: i32 = 0; diff --git a/crates/stdlib/src/ssl/compat.rs b/crates/stdlib/src/ssl/compat.rs index ab3c81b7a4e..cd927a0e410 100644 --- a/crates/stdlib/src/ssl/compat.rs +++ b/crates/stdlib/src/ssl/compat.rs @@ -1397,8 +1397,21 @@ pub(super) fn ssl_read( return Ok(n); } - // No plaintext available and cannot read more TLS records + // No plaintext available and rustls doesn't want to read more TLS records if !needs_more_tls { + // Check if connection needs to write data first (e.g., TLS key update, renegotiation) + // This mirrors the handshake logic which checks both wants_read() and wants_write() + if conn.wants_write() && !is_bio { + // Flush pending TLS data before continuing + let tls_data = ssl_write_tls_records(conn)?; + if !tls_data.is_empty() { + socket.sock_send(tls_data, vm).map_err(SslError::Py)?; + } + // After flushing, rustls may want to read again - continue loop + continue; + } + + // BIO mode: check for EOF if is_bio && let Some(bio_obj) = socket.incoming_bio() { let is_eof = bio_obj .get_attr("eof", vm) diff --git a/crates/stdlib/src/ssl/error.rs b/crates/stdlib/src/ssl/error.rs index e31683ec72d..bef9ba513d7 100644 --- a/crates/stdlib/src/ssl/error.rs +++ b/crates/stdlib/src/ssl/error.rs @@ -10,9 +10,27 @@ pub(crate) mod ssl_error { types::Constructor, }; - // Error type constants (needed for create_ssl_want_read_error etc.) + // Error type constants - exposed as pyattr and available for internal use + #[pyattr] + pub(crate) const SSL_ERROR_NONE: i32 = 0; + #[pyattr] + pub(crate) const SSL_ERROR_SSL: i32 = 1; + #[pyattr] pub(crate) const SSL_ERROR_WANT_READ: i32 = 2; + #[pyattr] pub(crate) const SSL_ERROR_WANT_WRITE: i32 = 3; + #[pyattr] + pub(crate) const SSL_ERROR_WANT_X509_LOOKUP: i32 = 4; + #[pyattr] + pub(crate) const SSL_ERROR_SYSCALL: i32 = 5; + #[pyattr] + pub(crate) const SSL_ERROR_ZERO_RETURN: i32 = 6; + #[pyattr] + pub(crate) const SSL_ERROR_WANT_CONNECT: i32 = 7; + #[pyattr] + pub(crate) const SSL_ERROR_EOF: i32 = 8; + #[pyattr] + pub(crate) const SSL_ERROR_INVALID_ERROR_CODE: i32 = 10; #[pyattr] #[pyexception(name = "SSLError", base = PyOSError)] @@ -102,7 +120,7 @@ pub(crate) mod ssl_error { pub fn create_ssl_eof_error(vm: &VirtualMachine) -> PyRef { vm.new_os_subtype_error( PySSLEOFError::class(&vm.ctx).to_owned(), - None, + Some(SSL_ERROR_EOF), "EOF occurred in violation of protocol", ) } @@ -110,7 +128,7 @@ pub(crate) mod ssl_error { pub fn create_ssl_zero_return_error(vm: &VirtualMachine) -> PyRef { vm.new_os_subtype_error( PySSLZeroReturnError::class(&vm.ctx).to_owned(), - None, + Some(SSL_ERROR_ZERO_RETURN), "TLS/SSL connection has been closed (EOF)", ) }