From b5d69f7bde0cb82dfa1abdf008e98c5b3c786efc Mon Sep 17 00:00:00 2001 From: WiktorB2004 Date: Sun, 25 Oct 2026 00:24:32 +0200 Subject: [PATCH 1/3] Close the session when login fails in Token.open() Token.open() opens a session with C_OpenSession and then authenticates with C_Login. If authentication failed (wrong PIN, locked PIN, user already logged in, or protected authentication requested from a module that does not support it), the exception propagated without closing the session, and since the caller never received a Session object, the handle stayed open until C_Finalize. Close it before re-raising. The C_CloseSession return value is ignored on purpose, so that it cannot mask the original error. --- pkcs11/_pkcs11.pyx | 29 ++++++++++++++++++----------- tests/test_sessions.py | 21 ++++++++++++++++++++- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/pkcs11/_pkcs11.pyx b/pkcs11/_pkcs11.pyx index fbc0cd3..9ecc61a 100644 --- a/pkcs11/_pkcs11.pyx +++ b/pkcs11/_pkcs11.pyx @@ -528,20 +528,27 @@ cdef class Token(HasFuncList, types.Token): retval = self.funclist.C_OpenSession(slot_id, flags, NULL, NULL, &handle) assertRV(retval) - if so_pin is PROTECTED_AUTH or user_pin is PROTECTED_AUTH: - if self.flags & TokenFlag.PROTECTED_AUTHENTICATION_PATH: + try: + if so_pin is PROTECTED_AUTH or user_pin is PROTECTED_AUTH: + if self.flags & TokenFlag.PROTECTED_AUTHENTICATION_PATH: + with nogil: + retval = self.funclist.C_Login(handle, c_user_type, NULL, 0) + assertRV(retval) + else: + raise ArgumentsBad("Protected authentication is not supported by loaded module") + elif pin is not None: + pin_data = pin + pin_length = len(pin) + with nogil: - retval = self.funclist.C_Login(handle, c_user_type, NULL, 0) + retval = self.funclist.C_Login(handle, c_user_type, pin_data, pin_length) assertRV(retval) - else: - raise ArgumentsBad("Protected authentication is not supported by loaded module") - elif pin is not None: - pin_data = pin - pin_length = len(pin) - + except BaseException: + # Do not leak the session opened above if authentication fails. + # Ignore the CloseSession return value: the original error matters. with nogil: - retval = self.funclist.C_Login(handle, c_user_type, pin_data, pin_length) - assertRV(retval) + self.funclist.C_CloseSession(handle) + raise return Session.make( self, handle, diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 41aa2c9..9197360 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -11,7 +11,7 @@ PKCS11Error, ) from pkcs11.attributes import AttributeMapper, handle_bool, handle_str -from pkcs11.exceptions import PinIncorrect, PinLenRange +from pkcs11.exceptions import ArgumentsBad, PinIncorrect, PinLenRange from . import TOKEN_PIN, TOKEN_SO_PIN, Not, Only, TestCase, requires @@ -34,6 +34,25 @@ def test_open_session_and_login_so(self): with self.token.open(rw=True, so_pin=TOKEN_SO_PIN) as session: self.assertIsInstance(session, pkcs11.Session) + @Only.softhsm2 # We don't have credentials to do this for other platforms + def test_failed_login_does_not_leak_session(self): + # A read-only session left open by a failed login would make the + # security officer login below fail with SessionReadOnlyExists. + with self.assertRaises(PinIncorrect): + self.token.open(user_pin="not the user pin") + + with self.token.open(rw=True, so_pin=TOKEN_SO_PIN) as session: + self.assertIsInstance(session, pkcs11.Session) + + @Only.softhsm2 # We don't have credentials to do this for other platforms + def test_unsupported_protected_auth_does_not_leak_session(self): + # SoftHSMv2 has no protected authentication path. + with self.assertRaises(ArgumentsBad): + self.token.open(user_pin=pkcs11.PROTECTED_AUTH) + + with self.token.open(rw=True, so_pin=TOKEN_SO_PIN) as session: + self.assertIsInstance(session, pkcs11.Session) + @requires(pkcs11.Mechanism.AES_KEY_GEN) def test_generate_key(self): with self.token.open(user_pin=TOKEN_PIN) as session: From 45499bb7e6c07b8e7d1d30c041e09840b3cf16e4 Mon Sep 17 00:00:00 2001 From: WiktorB2004 Date: Sun, 25 Oct 2026 14:30:48 +0100 Subject: [PATCH 2/3] Check protected authentication before opening the session Raise ArgumentsBad for an unsupported protected authentication path before C_OpenSession, and decide on the C_Login result whether to close the session and raise or to return it, instead of a catch-all except. --- pkcs11/_pkcs11.pyx | 37 ++++++++++++++++++------------------- tests/test_sessions.py | 4 ++-- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/pkcs11/_pkcs11.pyx b/pkcs11/_pkcs11.pyx index 9ecc61a..b635b1e 100644 --- a/pkcs11/_pkcs11.pyx +++ b/pkcs11/_pkcs11.pyx @@ -524,31 +524,30 @@ cdef class Token(HasFuncList, types.Token): pin = None c_user_type = CKU_USER_NOBODY + if (so_pin is PROTECTED_AUTH or user_pin is PROTECTED_AUTH) and \ + not self.flags & TokenFlag.PROTECTED_AUTHENTICATION_PATH: + raise ArgumentsBad("Protected authentication is not supported by loaded module") + with nogil: retval = self.funclist.C_OpenSession(slot_id, flags, NULL, NULL, &handle) assertRV(retval) - try: - if so_pin is PROTECTED_AUTH or user_pin is PROTECTED_AUTH: - if self.flags & TokenFlag.PROTECTED_AUTHENTICATION_PATH: - with nogil: - retval = self.funclist.C_Login(handle, c_user_type, NULL, 0) - assertRV(retval) - else: - raise ArgumentsBad("Protected authentication is not supported by loaded module") - elif pin is not None: - pin_data = pin - pin_length = len(pin) - - with nogil: - retval = self.funclist.C_Login(handle, c_user_type, pin_data, pin_length) - assertRV(retval) - except BaseException: - # Do not leak the session opened above if authentication fails. - # Ignore the CloseSession return value: the original error matters. + if so_pin is PROTECTED_AUTH or user_pin is PROTECTED_AUTH: + with nogil: + retval = self.funclist.C_Login(handle, c_user_type, NULL, 0) + elif pin is not None: + pin_data = pin + pin_length = len(pin) + + with nogil: + retval = self.funclist.C_Login(handle, c_user_type, pin_data, pin_length) + + if retval != CKR_OK: + # Do not leak the session if the login failed; report the login error, + # not the result of closing the session. with nogil: self.funclist.C_CloseSession(handle) - raise + assertRV(retval) return Session.make( self, handle, diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 9197360..2eb197c 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -34,7 +34,7 @@ def test_open_session_and_login_so(self): with self.token.open(rw=True, so_pin=TOKEN_SO_PIN) as session: self.assertIsInstance(session, pkcs11.Session) - @Only.softhsm2 # We don't have credentials to do this for other platforms + @Only.softhsm2 def test_failed_login_does_not_leak_session(self): # A read-only session left open by a failed login would make the # security officer login below fail with SessionReadOnlyExists. @@ -44,7 +44,7 @@ def test_failed_login_does_not_leak_session(self): with self.token.open(rw=True, so_pin=TOKEN_SO_PIN) as session: self.assertIsInstance(session, pkcs11.Session) - @Only.softhsm2 # We don't have credentials to do this for other platforms + @Only.softhsm2 def test_unsupported_protected_auth_does_not_leak_session(self): # SoftHSMv2 has no protected authentication path. with self.assertRaises(ArgumentsBad): From 3d65d82526e958d662bfe5122913df48093d4306 Mon Sep 17 00:00:00 2001 From: WiktorB2004 Date: Sun, 25 Oct 2026 14:37:30 +0100 Subject: [PATCH 3/3] Tighten the comments of the login failure fix --- pkcs11/_pkcs11.pyx | 3 +-- tests/test_sessions.py | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/pkcs11/_pkcs11.pyx b/pkcs11/_pkcs11.pyx index b635b1e..75307d6 100644 --- a/pkcs11/_pkcs11.pyx +++ b/pkcs11/_pkcs11.pyx @@ -543,8 +543,7 @@ cdef class Token(HasFuncList, types.Token): retval = self.funclist.C_Login(handle, c_user_type, pin_data, pin_length) if retval != CKR_OK: - # Do not leak the session if the login failed; report the login error, - # not the result of closing the session. + # Raise the login error, not a C_CloseSession error. with nogil: self.funclist.C_CloseSession(handle) assertRV(retval) diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 2eb197c..f66db85 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -36,8 +36,6 @@ def test_open_session_and_login_so(self): @Only.softhsm2 def test_failed_login_does_not_leak_session(self): - # A read-only session left open by a failed login would make the - # security officer login below fail with SessionReadOnlyExists. with self.assertRaises(PinIncorrect): self.token.open(user_pin="not the user pin") @@ -46,7 +44,6 @@ def test_failed_login_does_not_leak_session(self): @Only.softhsm2 def test_unsupported_protected_auth_does_not_leak_session(self): - # SoftHSMv2 has no protected authentication path. with self.assertRaises(ArgumentsBad): self.token.open(user_pin=pkcs11.PROTECTED_AUTH)