Skip to content

Fenrir fixes #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JeremiahM37 wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fenrir fixes #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ffd86de
Add tests for recent fixes
JeremiahM37 Apr 2, 2026
dc49a53
Default DTLS version in client example
JeremiahM37 Apr 2, 2026
a406dce
Add DTLS handshake to recv_into
JeremiahM37 Apr 2, 2026
84bd637
Copy DER buffer in get_der
JeremiahM37 Apr 2, 2026
e436664
Skip close on DTLS loop socket
JeremiahM37 Apr 2, 2026
89667f9
Add null checks to version/pending
JeremiahM37 Apr 2, 2026
760cb46
Shutdown and free SSL in unwrap
JeremiahM37 Apr 2, 2026
c81c839
Free peer address on set_peer failure
JeremiahM37 Apr 2, 2026
e1ede23
Fix wolfSSL_Init return type and check
JeremiahM37 Apr 2, 2026
2c4ba3c
Fix low-severity issues
JeremiahM37 Apr 2, 2026
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
6 changes: 5 additions & 1 deletion examples/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ def main():

# DTLS connection over UDP
if args.u:
if args.v > 2:
args.v = 1
bind_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
context = wolfssl.SSLContext(get_DTLSmethod(args.v))
# SSL/TLS connection over TCP
Expand All @@ -151,6 +153,7 @@ def main():
if args.l:
context.set_ciphers(args.l)

secure_socket = None
try:
secure_socket = context.wrap_socket(bind_socket)

Expand All @@ -171,7 +174,8 @@ def main():
print()

finally:
secure_socket.close()
if secure_socket:
secure_socket.close()


if __name__ == '__main__':
Expand Down
3 changes: 2 additions & 1 deletion examples/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ def main():
finally:
if secure_socket:
secure_socket.shutdown(socket.SHUT_RDWR)
secure_socket.close()
if not args.u:
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 8, 2026

Choose a reason for hiding this comment

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

Description: The new code conditionally skips secure_socket.close() for DTLS (args.u). This is correct because for DTLS, secure_socket wraps bind_socket (the listening UDP socket), and closing it would prevent subsequent loop iterations from accepting new connections. However, the reasoning isn't documented and could confuse future readers.

Code:

finally:
    if secure_socket:
        secure_socket.shutdown(socket.SHUT_RDWR)
        if not args.u:
            secure_socket.close()

Recommendation: Add a comment explaining why close is skipped for DTLS:

Suggested change
if not args.u:
secure_socket.shutdown(socket.SHUT_RDWR)
# Don't close for DTLS - secure_socket wraps the shared
# bind_socket which is needed for subsequent connections
if not args.u:
secure_socket.close()

secure_socket.close()

if not args.i:
break
Expand Down
14 changes: 14 additions & 0 deletions tests/test_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,17 @@ def test_load_verify_locations_with_cafile(ssl_context):

def test_load_verify_locations_with_cadata(ssl_context):
ssl_context.load_verify_locations(cadata=_CADATA)


def test_check_hostname_requires_cert_required(ssl_provider, ssl_context):
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 8, 2026

Choose a reason for hiding this comment

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

Description: This diff introduces several significant behavior changes to close(), shutdown(), and unwrap() in SSLSocket, but no tests are added for these changes:

  1. close() now calls wolfSSL_shutdown() before releasing the native object (lines 913-916)
  2. shutdown() now implements two-phase TLS shutdown — calling wolfSSL_shutdown() a second time if the first returns 0 (lines 725-728)
  3. unwrap() now performs a full SSL shutdown + release instead of just detaching the fd (lines 738-740)

The two new tests (test_check_hostname_requires_cert_required and test_wrap_socket_server_side_mismatch) are good additions but cover different code paths.

Recommendation: Add tests that verify:

  • close() on an SSLSocket that hasn't completed handshake doesn't raise
  • close() followed by operations properly raises ValueError ("closed" check)
  • shutdown() properly releases the native object (subsequent calls don't crash)

with pytest.raises(ValueError):
ssl_context.check_hostname = True

ssl_context.verify_mode = ssl_provider.CERT_REQUIRED
ssl_context.check_hostname = True
assert ssl_context.check_hostname is True


def test_wrap_socket_server_side_mismatch(ssl_context, tcp_socket):
with pytest.raises(ValueError):
ssl_context.wrap_socket(tcp_socket, server_side=True)
97 changes: 58 additions & 39 deletions wolfssl/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ class WolfSSL(object):

@classmethod
def enable_debug(self):
_lib.wolfSSL_Debugging_ON()
if _lib.wolfSSL_Debugging_ON() != _SSL_SUCCESS:
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 8, 2026

Choose a reason for hiding this comment

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

Description: The diff changes enable_debug() to raise RuntimeError when wolfSSL_Debugging_ON() doesn't return _SSL_SUCCESS (i.e., when wolfSSL is not compiled with --enable-debug). Both example scripts call wolfssl.WolfSSL.enable_debug() unconditionally without any try/except:

# examples/client.py:143
wolfssl.WolfSSL.enable_debug()

# examples/server.py:139
wolfssl.WolfSSL.enable_debug()

Previously, enable_debug() called wolfSSL_Debugging_ON() and silently ignored the return value, so the examples worked whether or not debug was compiled in. Now they will crash with RuntimeError: wolfSSL debugging not available for any non-debug wolfSSL build — which is the common default.

The comments in the examples even acknowledge this: "enable debug, if native wolfSSL has been compiled with '--enable-debug'" — the "if" implies it should be tolerant.

Code:

# wolfssl/__init__.py (new)
@classmethod
def enable_debug(self):
    if _lib.wolfSSL_Debugging_ON() != _SSL_SUCCESS:
        raise RuntimeError(
            "wolfSSL debugging not available")

Recommendation: Wrap enable_debug() calls in the examples in a try/except:

Suggested change
if _lib.wolfSSL_Debugging_ON() != _SSL_SUCCESS:
# examples/client.py and examples/server.py
try:
wolfssl.WolfSSL.enable_debug()
except RuntimeError:
pass

Alternatively, the library could provide a try_enable_debug() that returns a bool, keeping enable_debug() strict.

raise RuntimeError(
"wolfSSL debugging not available")

@classmethod
def disable_debug(self):
Expand Down Expand Up @@ -143,9 +145,7 @@ def get_der(self):
if derPtr == _ffi.NULL:
return None

derBytes = _ffi.buffer(derPtr, outSz[0])

return derBytes
return _ffi.buffer(derPtr, outSz[0])[:]

class SSLContext(object):
"""
Expand All @@ -154,7 +154,9 @@ class SSLContext(object):
"""

def __init__(self, protocol, server_side=None):
_lib.wolfSSL_Init()
if _lib.wolfSSL_Init() != _SSL_SUCCESS:
raise RuntimeError(
"wolfSSL library initialization failed")
method = _WolfSSLMethod(protocol, server_side)

self.protocol = protocol
Expand Down Expand Up @@ -356,9 +358,10 @@ def load_verify_locations(self, cafile=None, capath=None, cadata=None):
raise SSLError("Unable to load verify locations. E(%d)" % ret)

if cadata is not None:
cadata_bytes = t2b(cadata)
ret = _lib.wolfSSL_CTX_load_verify_buffer(
self.native_object, t2b(cadata),
len(cadata), _SSL_FILETYPE_PEM)
self.native_object, cadata_bytes,
len(cadata_bytes), _SSL_FILETYPE_PEM)

if ret != _SSL_SUCCESS:
raise SSLError("Unable to load verify locations. E(%d)" % ret)
Expand Down Expand Up @@ -476,8 +479,11 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
ret = _lib.wolfSSL_check_domain_name(self.native_object,
sni)
if ret != _SSL_SUCCESS:
raise SSLError("Unable to set domain name check for "
"hostname verification")
self._release_native_object()
raise SSLError(
"Unable to set domain name "
"check for hostname "
"verification")

if connected:
try:
Expand All @@ -497,6 +503,7 @@ def _release_native_object(self):
self.native_object = _ffi.NULL

def pending(self):
self._check_closed("pending")
return _lib.wolfSSL_pending(self.native_object)

@property
Expand Down Expand Up @@ -605,13 +612,6 @@ def sendall(self, data, flags=0):

while sent < length:
ret = self.write(data[sent:])
if (ret <= 0):
#expect to receive 0 when peer is reset or closed
err = _lib.wolfSSL_get_error(self.native_object, 0)
if err == _SSL_ERROR_WANT_WRITE:
raise SSLWantWriteError()
else:
raise SSLError("wolfSSL_write error (%d)" % err)

sent += ret

Expand Down Expand Up @@ -676,11 +676,13 @@ def recv_into(self, buffer, nbytes=None, flags=0):
self._check_closed("read")
if self._context.protocol < PROTOCOL_DTLSv1:
self._check_connected()
else:
self.do_handshake()

if buffer is None:
raise ValueError("buffer cannot be None")

if nbytes is None:
if nbytes is None or nbytes == 0:
nbytes = len(buffer)
else:
nbytes = min(len(buffer), nbytes)
Expand Down Expand Up @@ -721,7 +723,9 @@ def recvmsg_into(self, *args, **kwargs):

def shutdown(self, how):
if self.native_object != _ffi.NULL:
_lib.wolfSSL_shutdown(self.native_object)
ret = _lib.wolfSSL_shutdown(self.native_object)
if ret == 0:
_lib.wolfSSL_shutdown(self.native_object)
self._release_native_object()
if self._context.protocol < PROTOCOL_DTLSv1:
self._sock.shutdown(how)
Expand All @@ -732,7 +736,8 @@ def unwrap(self):
Returns the wrapped OS socket.
"""
if self.native_object != _ffi.NULL:
_lib.wolfSSL_set_fd(self.native_object, -1)
_lib.wolfSSL_shutdown(self.native_object)
self._release_native_object()

sock = socket(family=self._sock.family,
sock_type=self._sock.type,
Expand All @@ -748,11 +753,16 @@ def add_peer(self, addr):
peerAddr = _lib.wolfSSL_dtls_create_peer(addr[1],t2b(addr[0]))
if peerAddr == _ffi.NULL:
raise SSLError("Failed to create peer")
ret = _lib.wolfSSL_dtls_set_peer(self.native_object, peerAddr,
_SOCKADDR_SZ)
if ret != _SSL_SUCCESS:
raise SSLError("Unable to set dtls peer. E(%d)" % ret)
_lib.wolfSSL_dtls_free_peer(peerAddr)
try:
ret = _lib.wolfSSL_dtls_set_peer(
self.native_object, peerAddr,
_SOCKADDR_SZ)
if ret != _SSL_SUCCESS:
raise SSLError(
"Unable to set dtls peer."
" E(%d)" % ret)
finally:
_lib.wolfSSL_dtls_free_peer(peerAddr)

def do_handshake(self, block=False): # pylint: disable=unused-argument
"""
Expand Down Expand Up @@ -814,18 +824,16 @@ def _real_connect(self, addr, connect_ex):
raise ValueError("attempt to connect already-connected SSLSocket!")

err = 0
ret = _SSL_SUCCESS


if self._context.protocol >= PROTOCOL_DTLSv1:
self.add_peer(addr)
self.add_peer(addr)
else:
if connect_ex:
err = self._sock.connect_ex(addr)
else:
err = 0
self._sock.connect(addr)

if err == 0 and ret == _SSL_SUCCESS:
if err == 0:
self._connected = True
if self.do_handshake_on_connect:
self.do_handshake()
Expand Down Expand Up @@ -894,12 +902,18 @@ def version(self):
"""
Returns the version of the protocol used in the connection.
"""
return _ffi.string(_lib.wolfSSL_get_version(self.native_object)).decode("ascii")
self._check_closed("version")
return _ffi.string(
_lib.wolfSSL_get_version(
self.native_object)).decode("ascii")

# The following functions expose functionality of the underlying
# Socket object. These are also exposed through Python's ssl module
# API and are provided here for compatibility.
def close(self):
if self.native_object != _ffi.NULL:
_lib.wolfSSL_shutdown(self.native_object)
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 8, 2026

Choose a reason for hiding this comment

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

Description: The new close() implementation calls wolfSSL_shutdown() before releasing the native object. While this is correct for clean TLS teardown, wolfSSL_shutdown() on a socket that never completed the handshake (e.g., wrap_socket succeeded but connect/do_handshake was never called) may produce unexpected errors or warnings in the wolfSSL log. The return value of wolfSSL_shutdown() is silently ignored in close(), which is acceptable for cleanup, but there's no guard for the case where the SSL session was never fully established.

Code:

def close(self):
    if self.native_object != _ffi.NULL:
        _lib.wolfSSL_shutdown(self.native_object)
        self._release_native_object()
    self._sock.close()

Recommendation: Consider guarding the shutdown call to only run when a connection was established:

Suggested change
_lib.wolfSSL_shutdown(self.native_object)
def close(self):
if self.native_object != _ffi.NULL:
if self._connected:
_lib.wolfSSL_shutdown(self.native_object)
self._release_native_object()
self._sock.close()

self._release_native_object()
self._sock.close()

def fileno(self):
Expand Down Expand Up @@ -1029,12 +1043,17 @@ def callback(self):
def _get_passwd(self, passwd, sz, rw, userdata):
try:
result = self._passwd_wrapper(sz, rw, userdata)
if not isinstance(result, bytes):
raise ValueError("Problem, expected String, not bytes")
if len(result) > sz:
raise ValueError("Problem with password returned being long")
for i in range(len(result)):
passwd[i] = result[i:i + 1]
return len(result)
except Exception as e:
raise ValueError("Problem getting password from callback")
except Exception:
raise ValueError(
"Problem getting password from callback")
if not isinstance(result, bytes):
raise ValueError(
"Password callback must return bytes,"
" not str")
if len(result) > sz:
raise ValueError(
"Problem with password returned"
" being long")
for i in range(len(result)):
passwd[i] = result[i:i + 1]
return len(result)
4 changes: 2 additions & 2 deletions wolfssl/_build_ffi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ def generate_libwolfssl():
/*
* Debugging
*/
void wolfSSL_Debugging_ON();
int wolfSSL_Debugging_ON(void);
void wolfSSL_Debugging_OFF();

/*
Expand Down Expand Up @@ -474,7 +474,7 @@ def generate_libwolfssl():
/*
* SSL/TLS Session functions
*/
void wolfSSL_Init();
int wolfSSL_Init(void);
WOLFSSL* wolfSSL_new(WOLFSSL_CTX*);
void wolfSSL_free(WOLFSSL*);

Expand Down
Loading