8000 PySSLCertificate by youknowone · Pull Request #6219 · RustPython/RustPython · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Lib/test/test_urllib2_localnet.py
Original file line number Diff line number Diff line change
Expand Up @@ -568,17 +568,13 @@ def test_200_with_parameters(self):
self.assertEqual(data, expected_response)
self.assertEqual(handler.requests, ["/bizarre", b"get=with_feeling"])

# TODO: RUSTPYTHON
@unittest.expectedFailure
@unittest.skipIf(os.name == "nt", "TODO: RUSTPYTHON, ValueError: illegal environment variable name")
def test_https(self):
handler = self.start_https_server()
context = ssl.create_default_context(cafile=CERT_localhost)
data = self.urlopen("https://localhost:%s/bizarre" % handler.port, context=context)
self.assertEqual(data, b"we care a bit")

# TODO: RUSTPYTHON
@unittest.expectedFailure
@unittest.skipIf(os.name == "nt", "TODO: RUSTPYTHON, ValueError: illegal environment variable name")
def test_https_with_cafile(self):
handler = self.start_https_server(certfile=CERT_localhost)
Expand Down
247 changes: 100 additions & 147 deletions stdlib/src/ssl.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// spell-checker:disable

mod cert;

use crate::vm::{PyRef, VirtualMachine, builtins::PyModule};
use openssl_probe::ProbeResult;

Expand All @@ -26,15 +28,12 @@ cfg_if::cfg_if! {
}

#[allow(non_upper_case_globals)]
#[pymodule(with(ossl101, ossl111, windows))]
#[pymodule(with(cert::ssl_cert, ossl101, ossl111, windows))]
mod _ssl {
use super::{bio, probe};
use crate::{
common::{
ascii,
lock::{
PyMappedRwLockReadGuard, PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard,
},
common::lock::{
PyMappedRwLockReadGuard, PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard,
},
socket::{self, PySocket},
vm::{
Expand All @@ -43,7 +42,7 @@ mod _ssl {
PyBaseExceptionRef, PyBytesRef, PyListRef, PyOSError, PyStrRef, PyTypeRef, PyWeak,
},
class_or_notimplemented,
convert::{ToPyException, ToPyObject},
convert::ToPyException,
exceptions,
function::{
ArgBytesLike, ArgCallable, ArgMemoryBuffer, ArgStrOrBytesLike, Either, FsPath,
Expand All @@ -60,7 +59,7 @@ mod _ssl {
error::ErrorStack,
nid::Nid,
ssl::{self, SslContextBuilder, SslOptions, SslVerifyMode},
x509::{self, X509, X509Ref},
x509::X509,
};
use openssl_sys as sys;
use rustpython_vm::ospath::OsPath;
Expand All @@ -73,6 +72,14 @@ mod _ssl {
time::Instant,
};

// Import certificate types from parent module
use super::cert::{self, cert_to_certificate, cert_to_py};

// Re-export PySSLCertificate to make it available in the _ssl module
// It will be automatically exposed to Python via #[pyclass]
#[allow(unused_imports)]
use super::cert::PySSLCertificate;

// Constants
#[pyattr]
use sys::{
Expand Down Expand Up @@ -178,6 +185,18 @@ mod _ssl {
#[pyattr]
const HAS_PSK: bool = true;

// Encoding constants for Certificate.public_bytes()
#[pyattr]
pub(crate) const ENCODING_PEM: i32 = sys::X509_FILETYPE_PEM;
#[pyattr]
pub(crate) const ENCODING_DER: i32 = sys::X509_FILETYPE_ASN1;
#[pyattr]
const ENCODING_PEM_AUX: i32 = sys::X509_FILETYPE_PEM + 0x100;

// OpenSSL error codes for unexpected EOF detection
const ERR_LIB_SSL: i32 = 20;
const SSL_R_UNEXPECTED_EOF_WHILE_READING: i32 = 294;
Comment on lines +188 to +198
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Clarify ENCODING_PEM_AUX usage and magic number.

The constant ENCODING_PEM_AUX uses a magic number (0x100) added to X509_FILETYPE_PEM, but it's unclear if this constant is actually used anywhere in the codebase. Additionally, the hardcoded OpenSSL error codes (ERR_LIB_SSL = 20, SSL_R_UNEXPECTED_EOF_WHILE_READING = 294) could become outdated across OpenSSL versions.

Run the following to check if ENCODING_PEM_AUX is used:

Consider adding a comment explaining the 0x100 offset for ENCODING_PEM_AUX, or removing it if unused. For the error codes, consider referencing the OpenSSL version these codes correspond to in a comment.


🏁 Script executed:

#!/bin/bash
# Search for ENCODING_PEM_AUX usage
rg -n 'ENCODING_PEM_AUX' --type rust

Length of output: 87



---

🏁 Script executed:

```shell
#!/bin/bash
# Check for broader usage and context of these constants
# Look for ERR_LIB_SSL and SSL_R_UNEXPECTED_EOF_WHILE_READING usage
rg -n 'ERR_LIB_SSL|SSL_R_UNEXPECTED_EOF_WHILE_READING' --type rust -C 3

Length of output: 1583


🏁 Script executed:

#!/bin/bash
# Search for X509_FILETYPE patterns to understand the constants better
rg -n 'X509_FILETYPE|ENCODING_PEM|ENCODING_DER' --type rust -C 2

Length of output: 1707


🏁 Script executed:

#!/bin/bash
# Check if there are tests or examples that use these constants
fd -e rs -e toml -e md | xargs grep -l 'ENCODING_PEM_AUX\|ssl.*encoding' 2>/dev/null || true

Length of output: 17



---

🏁 Script executed:

```shell
#!/bin/bash
# Check OpenSSL documentation or sys module for X509_FILETYPE_PEM_AUX reference
rg -n 'X509_FILETYPE_PEM_AUX|0x100' --type rust stdlib/src/ssl.rs

Length of output: 69


🏁 Script executed:

#!/bin/bash
# Look for any comments or documentation about the 0x100 offset
sed -n '188,200p' stdlib/src/ssl.rs

Length of output: 491


Add documentation for ENCODING_PEM_AUX and the 0x100 offset magic number.

ENCODING_PEM_AUX is exposed to Python via #[pyattr] but unused in Rust code; clarify whether this is intentional public API or dead code. Additionally, add a comment explaining the 0x100 offset, which appears to follow OpenSSL's own X509_FILETYPE_PEM_AUX pattern but lacks documentation.

The hardcoded error codes ERR_LIB_SSL and SSL_R_UNEXPECTED_EOF_WHILE_READING are actively used in the OpenSSL 3.0+ error detection logic at line 2083 and do not present a versioning concern in this context.

🤖 Prompt for AI Agents
In stdlib/src/ssl.rs around lines 188 to 198, ENCODING_PEM_AUX is exposed to
Python but unused in Rust and the 0x100 magic offset lacks explanation; update
the source by adding a short doc comment above ENCODING_PEM_AUX stating that
this attribute is intentionally exported for compatibility with OpenSSL's
X509_FILETYPE_PEM_AUX (kept as part of the public Python API), and explain that
the 0x100 value is the OpenSSL-defined offset used to distinguish PEM_AUX from
PEM; also add a brief comment near ERR_LIB_SSL and
SSL_R_UNEXPECTED_EOF_WHILE_READING noting these are OpenSSL numeric error codes
relied on for EOF detection so they are intentionally hardcoded and should be
kept if used elsewhere.


// the openssl version from the API headers

#[pyattr(name = "OPENSSL_VERSION")]
Expand Down Expand Up @@ -349,32 +368,6 @@ mod _ssl {
fn _nid2obj(nid: Nid) -> Option<Asn1Object> {
unsafe { ptr2obj(sys::OBJ_nid2obj(nid.as_raw())) }
}
fn obj2txt(obj: &Asn1ObjectRef, no_name: bool) -> Option<String> {
let no_name = i32::from(no_name);
let ptr = obj.as_ptr();
let b = unsafe {
let buflen = sys::OBJ_obj2txt(std::ptr::null_mut(), 0, ptr, no_name);
assert!(buflen >= 0);
if buflen == 0 {
return None;
}
let buflen = buflen as usize;
let mut buf = Vec::<u8>::with_capacity(buflen + 1);
let ret = sys::OBJ_obj2txt(
buf.as_mut_ptr() as *mut libc::c_char,
buf.capacity() as _,
ptr,
no_name,
);
assert!(ret >= 0);
// SAFETY: OBJ_obj2txt initialized the buffer successfully
buf.set_len(buflen);
buf
};
let s = String::from_utf8(b)
.unwrap_or_else(|e| String::from_utf8_lossy(e.as_bytes()).into_owned());
Some(s)
}

type PyNid = (libc::c_int, String, String, Option<String>);
fn obj2py(obj: &Asn1ObjectRef, vm: &VirtualMachine) -> PyResult<PyNid> {
Expand All @@ -387,7 +380,12 @@ mod _ssl {
.long_name()
.map_err(|_| vm.new_value_error("NID has no long name".to_owned()))?
.to_owned();
Ok((nid.as_raw(), short_name, long_name, obj2txt(obj, true)))
Ok((
nid.as_raw(),
short_name,
long_name,
cert::obj2txt(obj, true),
))
}

#[derive(FromArgs)]
Expand Down Expand Up @@ -1219,46 +1217,54 @@ mod _ssl {
}

#[pymethod]
fn get_unverified_chain(&self, vm: &VirtualMachine) -> Option<PyObjectRef> {
fn get_unverified_chain(&self, vm: &VirtualMachine) -> PyResult<Option<PyListRef>> {
let stream = self.stream.read();
let chain = stream.ssl().peer_cert_chain()?;
let Some(chain) = stream.ssl().peer_cert_chain() else {
return Ok(None);
};

// Return Certificate objects
let certs: Vec<PyObjectRef> = chain
.iter()
.filter_map(|cert| cert.to_der().ok().map(|der| vm.ctx.new_bytes(der).into()))
.collect();

Some(vm.ctx.new_list(certs).into())
.map(|cert| unsafe {
sys::X509_up_ref(cert.as_ptr());
let owned = X509::from_ptr(cert.as_ptr());
cert_to_certificate(vm, owned)
})
.collect::<PyResult<_>>()?;
Ok(Some(vm.ctx.new_list(certs)))
}

#[pymethod]
fn get_verified_chain(&self, vm: &VirtualMachine) -> Option<PyListRef> {
fn get_verified_chain(&self, vm: &VirtualMachine) -> PyResult<Option<PyListRef>> {
let stream = self.stream.read();
unsafe {
let chain = sys::SSL_get0_verified_chain(stream.ssl().as_ptr());
if chain.is_null() {
return None;
return Ok(None);
}

let num_certs = sys::OPENSSL_sk_num(chain as *const _);
let mut certs = Vec::new();

let mut certs = Vec::with_capacity(num_certs as usize);
// Return Certificate objects
for i in 0..num_certs {
let cert_ptr = sys::OPENSSL_sk_value(chain as *const _, i) as *mut sys::X509;
if cert_ptr.is_null() {
continue;
}
let cert = X509Ref::from_ptr(cert_ptr);
if let Ok(der) = cert.to_der() {
certs.push(vm.ctx.new_bytes(der).into());
}
// Clone the X509 certificate to create an owned copy
sys::X509_up_ref(cert_ptr);
let owned_cert = X509::from_ptr(cert_ptr);
let cert_obj = cert_to_certificate(vm, owned_cert)?;
certs.push(cert_obj);
}

if certs.is_empty() {
Ok(if certs.is_empty() {
None
} else {
Some(vm.ctx.new_list(certs))
}
})
}
}

Expand Down Expand Up @@ -1978,7 +1984,10 @@ mod _ssl {
}

#[track_caller]
fn convert_openssl_error(vm: &VirtualMachine, err: ErrorStack) -> PyBaseExceptionRef {
pub(crate) fn convert_openssl_error(
vm: &Virtual 2F8B Machine,
err: ErrorStack,
) -> PyBaseExceptionRef {
let cls = PySslError::class(&vm.ctx).to_owned();
match err.errors().last() {
Some(e) => {
Expand Down Expand Up @@ -2047,18 +2056,49 @@ mod _ssl {
),
ssl::ErrorCode::SYSCALL => match e.io_error() {
Some(io_err) => return io_err.to_pyexception(vm),
None => (
PySslSyscallError::class(&vm.ctx).to_owned(),
"EOF occurred in violation of protocol",
),
// 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_exception(
PySslEOFError::class(&vm.ctx).to_owned(),
vec![
vm.ctx.new_int(SSL_ERROR_EOF).into(),
vm.ctx
.new_str("EOF occurred in violation of protocol")
.into(),
],
);
}
},
ssl::ErrorCode::SSL => match e.ssl_error() {
Some(e) => return convert_openssl_error(vm, e.clone()),
None => (
ssl::ErrorCode::SSL => {
// Check for OpenSSL 3.0 SSL_R_UNEXPECTED_EOF_WHILE_READING
if let Some(ssl_err) = e.ssl_error() {
// In OpenSSL 3.0+, unexpected EOF is reported as SSL_ERROR_SSL
// with this specific reason code instead of SSL_ERROR_SYSCALL
unsafe {
let err_code = sys::ERR_peek_last_error();
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_exception(
vm.class("_ssl", "SSLEOFError"),
vec![
vm.ctx.new_int(SSL_ERROR_EOF).into(),
vm.ctx
.new_str("EOF occurred in violation of protocol")
.into(),
],
);
}
}
return convert_openssl_error(vm, ssl_err.clone());
}
(
PySslError::class(&vm.ctx).to_owned(),
"A failure in the SSL library occurred",
),
},
)
}
Comment on lines +2074 to +2101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use PySslEOFError::class for consistency.

The SSL_ERROR_SSL handler uses vm.class("_ssl", "SSLEOFError") at line 2085, which performs a runtime module lookup. This is inconsistent with the SSL_ERROR_SYSCALL handler (line 2064) which uses PySslEOFError::class(&vm.ctx).to_owned(). Runtime lookups are more fragile and could fail if the module isn't fully initialized.

Apply this diff for consistency:

                         if lib == ERR_LIB_SSL && reason == SSL_R_UNEXPECTED_EOF_WHILE_READING {
                             return vm.new_exception(
-                                vm.class("_ssl", "SSLEOFError"),
+                                PySslEOFError::class(&vm.ctx).to_owned(),
                                 vec![
                                     vm.ctx.new_int(SSL_ERROR_EOF).into(),
                                     vm.ctx
🤖 Prompt for AI Agents
In stdlib/src/ssl.rs around lines 2074 to 2101, replace the runtime module
lookup vm.class("_ssl", "SSLEOFError") with the compile-time reference
PySslEOFError::class(&vm.ctx).to_owned() so the EOF error construction uses the
same PySslEOFError::class(&vm.ctx).to_owned() pattern as the SSL_ERROR_SYSCALL
branch; update the returned vm.new_exception call to pass that class reference
instead of the vm.class(...) lookup.

_ => (
PySslError::class(&vm.ctx).to_owned(),
"A failure in the SSL library occurred",
Expand Down Expand Up @@ -2106,93 +2146,6 @@ mod _ssl {
(cipher.name(), cipher.version(), cipher.bits().secret)
}

fn cert_to_py(vm: &VirtualMachine, cert: &X509Ref, binary: bool) -> PyResult {
let r = if binary {
let b = cert.to_der().map_err(|e| convert_openssl_error(vm, e))?;
vm.ctx.new_bytes(b).into()
} else {
let dict = vm.ctx.new_dict();

let name_to_py = |name: &x509::X509NameRef| -> PyResult {
let list = name
.entries()
.map(|entry| {
let txt = obj2txt(entry.object(), false).to_pyobject(vm);
let data = vm.ctx.new_str(entry.data().as_utf8()?.to_owned());
Ok(vm.new_tuple(((txt, data),)).into())
})
.collect::<Result<_, _>>()
.map_err(|e| convert_openssl_error(vm, e))?;
Ok(vm.ctx.new_tuple(list).into())
};

dict.set_item("subject", name_to_py(cert.subject_name())?, vm)?;
dict.set_item("issuer", name_to_py(cert.issuer_name())?, vm)?;
// X.509 version: OpenSSL uses 0-based (0=v1, 1=v2, 2=v3) but Python uses 1-based (1=v1, 2=v2, 3=v3)
dict.set_item("version", vm.new_pyobj(cert.version() + 1), vm)?;

let serial_num = cert
.serial_number()
.to_bn()
.and_then(|bn| bn.to_hex_str())
.map_err(|e| convert_openssl_error(vm, e))?;
dict.set_item(
"serialNumber",
vm.ctx.new_str(serial_num.to_owned()).into(),
vm,
)?;

dict.set_item(
"notBefore",
vm.ctx.new_str(cert.not_before().to_string()).into(),
vm,
)?;
dict.set_item(
"notAfter",
vm.ctx.new_str(cert.not_after().to_string()).into(),
vm,
)?;

#[allow(clippy::manual_map)]
if let Some(names) = cert.subject_alt_names() {
let san = names
.iter()
.filter_map(|gen_name| {
if let Some(email) = gen_name.email() {
Some(vm.new_tuple((ascii!("email"), email)).into())
} else if let Some(dnsname) = gen_name.dnsname() {
Some(vm.new_tuple((ascii!("DNS"), dnsname)).into())
} else if let Some(ip) = gen_name.ipaddress() {
Some(
vm.new_tuple((
ascii!("IP Address"),
String::from_utf8_lossy(ip).into_owned(),
))
.into(),
)
} else {
// TODO: convert every type of general name:
// https://github.com/python/cpython/blob/3.6/Modules/_ssl.c#L1092-L1231
None
}
})
.collect();
dict.set_item("subjectAltName", vm.ctx.new_tuple(san).into(), vm)?;
};

dict.into()
};
Ok(r)
}

#[pyfunction]
fn _test_decode_cert(path: FsPath, vm: &VirtualMachine) -> PyResult {
let path = path.to_path_buf(vm)?;
let pem = std::fs::read(path).map_err(|e| e.to_pyexception(vm))?;
let x509 = X509::from_pem(&pem).map_err(|e| convert_openssl_error(vm, e))?;
cert_to_py(vm, &x509, false)
}

impl Read for SocketStream {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
let mut socket: &PySocket = &self.0;
Expand Down
Loading
Loading
0