Fenrir fixes (2026-06-23)#70
Conversation
…-3481) The DTLS branch called bind_socket.recvfrom(1) before creating the context. On UDP that removes the entire first datagram (the client's ClientHello) from the queue and discards everything past the first byte, so wolfSSL_accept() had nothing to consume and the handshake only recovered after the client's retransmit timer. The captured from_addr was also reused for every iteration of the -i loop. Replace it with a peek_peer_address() helper that uses MSG_PEEK to read the source address without consuming the datagram, and move the peek into the accept loop so the address is refreshed per connection.
wolfSSL_write can return WOLFSSL_ERROR_WANT_READ (e.g. during a renegotiation that must read a record before progressing; secure renegotiation is enabled by default). write() only handled WANT_WRITE, so WANT_READ fell through to a generic SSLError and non-blocking callers tore the session down. Add a WANT_READ branch raising SSLWantReadError, matching do_handshake().
wolfSSL_read can return WOLFSSL_ERROR_WANT_WRITE when the SSL layer must flush a handshake record (e.g. renegotiation) before returning data. read() only handled WANT_READ, raising a generic SSLError otherwise, which stops non-blocking callers from select()-ing on writability. Add a WANT_WRITE branch raising SSLWantWriteError.
recv_into() shares read()'s error-mapping pattern and inherited the same omission: wolfSSL_read returning WOLFSSL_ERROR_WANT_WRITE (during a renegotiation needing a write) was reported as a generic SSLError instead of SSLWantWriteError, breaking non-blocking callers that distinguish readiness directions. Add the WANT_WRITE branch.
For DTLS, write()/read()/recv_into() called do_handshake() on every call. do_handshake() runs wolfSSL_accept/connect, which on a non-blocking socket can raise SSLWantReadError and abort an I/O long after the handshake finished, and made DTLS write-side behaviour inconsistent with TCP. Track completion with a _handshake_complete flag set on a successful do_handshake(), and only drive the handshake from I/O methods while that flag is False.
There was a problem hiding this comment.
Pull request overview
This PR bundles several Fenrir-tracked behavioral fixes to the wolfSSL Python bindings to better match CPython ssl semantics (peer cert handling, I/O error mapping, DTLS handshake behavior) and to harden the shipped client/server examples (hostname verification + DTLS UDP peeking), with accompanying regression tests.
Changes:
- Adjust peer-certificate retrieval so
getpeercert()/get_peer_x509()returnNonewhen the peer presented no certificate. - Improve
SSLSocketI/O behavior: accept bytes-like inputs forwrite(), mapWANT_READ/WANT_WRITEto the corresponding exceptions, and avoid re-driving DTLS handshakes after completion. - Update examples (client hostname verification, DTLS server peek) and add targeted pytest coverage for each fix.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/__init__.py |
Core binding behavior changes: peer cert retrieval, bytes-like write(), DTLS handshake completion tracking, WANT_* exception mapping. |
examples/client.py |
Adds hostname-verification behavior (and opt-out flag) and passes server_hostname to wrap_socket(). |
examples/server.py |
Fixes DTLS UDP example to peek source address without consuming the ClientHello datagram. |
tests/test_write_bytes.py |
Regression tests ensuring write() sends bytes-like contents verbatim and rejects non-bytes-like inputs. |
tests/test_io_error_mapping.py |
Verifies WANT_READ/WANT_WRITE error-code mapping to SSLWantReadError/SSLWantWriteError. |
tests/test_getpeercert.py |
Confirms getpeercert()/get_peer_x509() return None when the peer provides no certificate. |
tests/test_dtls_server_example.py |
Ensures DTLS server peeking doesn’t consume the datagram needed for handshake. |
tests/test_dtls_handshake_once.py |
Ensures DTLS handshake is driven only until completion (not on every I/O call). |
tests/test_client_example.py |
Validates example client verification/hostname-check configuration behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The client example set CERT_REQUIRED and loaded CA roots but never set check_hostname or passed server_hostname to wrap_socket, so wolfSSL validated the chain to a trusted CA without binding the certificate to the requested host. A peer presenting any CA-trusted certificate for a different hostname would be accepted by anyone reusing this as a secure client template. Make verification configure hostname checking by default (via a new configure_verification helper) and add a -n flag to opt out explicitly for IP literals or test certificates.
write() converted data with t2b(), which str()-encodes anything that is
not already bytes. Valid bytes-like inputs such as bytearray and
memoryview were transmitted as their Python repr ("bytearray(b'...')",
"<memory at ...>") instead of their contents, corrupting the stream.
Convert via the buffer protocol (bytes(memoryview(data))) and raise
TypeError for objects that are not bytes-like, matching the stdlib ssl
module.
get_peer_x509() checked only whether the session was NULL and then built a WolfSSLX509, whose __init__ called wolfSSL_get_peer_certificate() and raised SSLError on NULL. On a valid connection where the peer presented no certificate (e.g. a server not requesting a client cert), this raised instead of returning None as the stdlib ssl getpeercert() contract requires. Fetch the certificate in get_peer_x509(), return None when it is NULL, and have WolfSSLX509 wrap the already-obtained pointer.
julek-wolfssl
force-pushed
the
fenrir/20260623
branch
from
43dc9f5 to
93954c9
Compare
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 3 total — 3 posted, 0 skipped
3 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] Client example default host is an IP literal but hostname verification is now on by default —
examples/client.py:46-48,134-158,183-191 - [Low] Non-ASCII character in test source literal —
tests/test_write_bytes.py:80-84 - [Low] WolfSSLX509 type discrimination relies on exact cffi cname string match —
wolfssl/__init__.py:99-111
Review generated by Skoll
| help="Disable client cert check" | ||
| ) | ||
|
|
||
| parser.add_argument( |
There was a problem hiding this comment.
🟠 [Medium] Client example default host is an IP literal but hostname verification is now on by default
F-5621 enables hostname verification whenever cert checking is on (the default). But the client's default host is the IP literal -h 127.0.0.1 (line 46), and configure_verification returns args.h as server_hostname and sets check_hostname = True for that default. __init__ then calls wolfSSL_check_domain_name(ssl, "127.0.0.1"). wolfSSL_check_domain_name performs DNS/CN-style domain matching; the bundled certs/server-cert.pem carries 127.0.0.1 only as an IP Address SAN (SAN is DNS:example.com, IP Address:127.0.0.1), not a DNS-type entry. If wolfSSL's domain-name check does not match IP-type SANs, the out-of-the-box python client.py run will now fail with a hostname/domain mismatch and require the new -n flag — a behavior change for the example's default invocation. The function's own docstring even states IP literals need -n, which is exactly the default host. Additionally, sending an IP literal via SNI (use_sni("127.0.0.1")) is not RFC-6066 conformant.
Fix: Confirm that a default python client.py (host 127.0.0.1) still completes the handshake against the bundled certs with hostname verification enabled. If wolfSSL_check_domain_name does not match the IP-type SAN, either auto-disable the hostname check for IP-literal hosts, document that -n is required for the default host, or change the example default host to a DNS name in the cert SAN.
|
|
||
|
|
||
| def test_write_str_is_utf8_encoded(monkeypatch): | ||
| # Backward compatibility: str is UTF-8 encoded (historical t2b() |
There was a problem hiding this comment.
🔵 [Low] Non-ASCII character in test source literal
The test embeds a raw non-ASCII character é (U+00E9) in the source to verify UTF-8 encoding. The file declares # -*- coding: utf-8 -*- so it is functionally valid, but the wolfSSL project convention is 7-bit ASCII for committed code. The same assertion can be expressed with an ASCII-only escape while still exercising a multi-byte UTF-8 encoding.
Fix: Replace the literal é with the escape \u00e9 to keep the source 7-bit ASCII while testing the same multi-byte path.
| @@ -97,7 +97,15 @@ class WolfSSLX509(object): | |||
| """ | |||
|
|
|||
| def __init__(self, session): | |||
There was a problem hiding this comment.
🔵 [Low] WolfSSLX509 type discrimination relies on exact cffi cname string match
The constructor distinguishes a WOLFSSL* session from a WOLFSSL_X509* by comparing _ffi.typeof(session).cname == "WOLFSSL *". This is verified correct against the current cdef (wolfSSL_get_peer_certificate(WOLFSSL*) returns WOLFSSL_X509*), but it is brittle: it depends on cffi's exact rendering of the type name, will raise TypeError if a non-cdata is ever passed, and silently stores whatever is in the else branch without validating it is actually a WOLFSSL_X509*. A small comment is already present; a slightly more defensive check (e.g. comparing against the X509 type, or guarding the typeof call) would be more robust to future cdef changes.
Fix: Optional hardening: key the branch off the expected X509 type or guard the typeof lookup so the constructor degrades gracefully if an unexpected type is passed. Functionally correct as-is.
This branch collects a set of Fenrir-tracked fixes for the wolfSSL Python bindings.
Changes
getpeercertwhen peer has no certificate (F-5623) — match the stdlibsslcontract.SSLSocket.write(F-5622) — avoid mangling bytes-like inputs.WANT_WRITEfromSSLSocket.recv_into()toSSLWantWriteError(F-3907).WANT_WRITEfromSSLSocket.read()toSSLWantWriteError(F-3906).WANT_READfromSSLSocket.write()toSSLWantReadError(F-3905).Each fix ships with accompanying tests.