Skip to content

Commit a11ed15

Browse files
committed
fix: update pyFF to pybergshamra
- Require pybergshamra 0.9.2 and refresh the lockfile. - Remove the temporary sibling checkout override. - Serialize PKCS#11 signing per token and release sessions on errors. - Add regression tests for session cleanup and concurrent signing. - Update dependency documentation and record server benchmarks.
1 parent 2c45ba4 commit a11ed15

8 files changed

Lines changed: 154 additions & 35 deletions

File tree

‎README.rst‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,13 @@ Features
4141
Dependencies
4242
============
4343

44-
* pyXMLSecurity
45-
* PyKCS11 (optional)
44+
* pybergshamra >= 0.9.2 (XML signatures and PKCS#11 support)
45+
* pyuppsala >= 0.11.0 (XML processing)
4646
* pygments
4747
* gunicorn (for the standalone pyffd server)
48-
* ... cf requirements.txt
48+
* ... cf pyproject.toml
49+
50+
``uv sync`` installs pybergshamra from PyPI; no sibling checkout is required.
4951

5052
More information
5153
================

‎docs/index.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ SAML metadata.
1313
size including edugain.org.
1414

1515
**pyFF** supports producing and validating digital signatures on SAML metadata using
16-
the pyXMLSecurity package which in turn supports PKCS#11 and other mechanisms for
16+
the pybergshamra package (0.9.2 or newer), which supports PKCS#11 and other mechanisms for
1717
talking to HSMs and other cryptographic hardware.
1818

1919
**pyFF** is also a complete implementation of the SAML metadata query protocol as

‎docs/usage/examples.rst‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,17 @@ a slot selector with ``token_label`` or ``token_serial``. The token label select
242242
the HSM token; the final URI component (``sctest2`` above) selects the key object
243243
within that token.
244244

245+
pybergshamra 0.9.2 rejects a new login when the token is already authenticated.
246+
pyFF serializes signing requests per token within each worker process and
247+
releases the session and signing objects after each operation, including on
248+
errors. Providers stay cached to keep the vendor module loaded. Other
249+
authenticated sessions for the same token in that process must be closed
250+
before pyFF can log in.
251+
245252
Running this example requires some preparation. Run the 'p11setup.sh' script in the examples directory.
246253
This results in a SoftHSM token being setup with the PIN 'secret1' and SO_PIN 'secret2'. Now run pyFF (assuming
247254
you are using a unix-like environment).
248255

249256
.. code-block:: bash
250257
251258
# env PYKCS11PIN=secret1 SOFTHSM_CONF=softhsm.conf pyff --loglevel=DEBUG p11.fd
252-

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ dependencies = [
2828
"pyramid>=2.0.2",
2929
"pytz>=2025.2",
3030
"pyuppsala>=0.11.0",
31-
"pybergshamra>=0.9.1",
31+
"pybergshamra>=0.9.2",
3232
"pyyaml>=3.10",
3333
"redis>=5.3.0",
3434
"redis-collections>=0.13.0",

‎src/pyff/builtins.py‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
# remain short-lived and are opened separately for each signing operation.
7575
_pkcs11_provider_cache = {}
7676
_pkcs11_provider_cache_lock = threading.Lock()
77+
_pkcs11_signing_locks = {}
7778

7879

7980
@pipe
@@ -1226,6 +1227,30 @@ def _select_pkcs11_provider(
12261227
return provider
12271228

12281229

1230+
def _sign_pkcs11(provider, module_path, pin, label, xml, *, reference_id=None, cert_pem=None):
1231+
"""Serialize token logins and release authenticated objects even on errors."""
1232+
# Different selectors can resolve to the same token. Lock its actual slot,
1233+
# rather than the configured label/serial/slot selector or key label.
1234+
token = (os.path.realpath(module_path), provider.slot_id)
1235+
with _pkcs11_provider_cache_lock:
1236+
lock = _pkcs11_signing_locks.setdefault(token, threading.Lock())
1237+
1238+
with lock:
1239+
session = signer = ctx = None
1240+
try:
1241+
session = provider.open_session(pin)
1242+
signer = pybergshamra.Pkcs11Signer(session, label, pybergshamra.Algorithm.RSA_SHA256)
1243+
ctx = pybergshamra.DsigContext(pybergshamra.KeysManager())
1244+
ctx.set_hsm_signer(signer)
1245+
return pybergshamra.sign_enveloped(ctx, xml, reference_id=reference_id, cert_pem=cert_pem)
1246+
finally:
1247+
# Exception tracebacks retain Python frames. Clear every local
1248+
# owner before unlocking so a failed request cannot retain login.
1249+
ctx = None
1250+
signer = None
1251+
session = None
1252+
1253+
12291254
@pipe
12301255
def sign(req: Plumbing.Request, *_opts):
12311256
"""
@@ -1255,7 +1280,7 @@ def sign(req: Plumbing.Request, *_opts):
12551280
12561281
The pin parameter can be used to point to an environment variable containing the pin: "env:<ENV variable>".
12571282
By default pin is "env:PYKCS11PIN" which tells sign to use the pin found in the PYKCS11PIN environment
1258-
variable. This is also the default for PyKCS11 which is used to communicate with the PKCS#11 module.
1283+
variable. Authentication and signing are serialized per token within the worker process.
12591284
12601285
**Examples**
12611286
@@ -1310,9 +1335,6 @@ def sign(req: Plumbing.Request, *_opts):
13101335
with open(cert_file) as fd:
13111336
cert_pem = fd.read()
13121337

1313-
mgr = pybergshamra.KeysManager()
1314-
ctx = pybergshamra.DsigContext(mgr)
1315-
13161338
is_pkcs11_key = key_file.startswith("pkcs11:")
13171339
if is_pkcs11_key:
13181340
# HSM signing via PKCS#11. The URI form is:
@@ -1334,27 +1356,22 @@ def sign(req: Plumbing.Request, *_opts):
13341356
token_label=req.args.get('token_label'),
13351357
token_serial=req.args.get('token_serial'),
13361358
)
1337-
session = provider.open_session(pin)
1338-
signer = pybergshamra.Pkcs11Signer(session, label, pybergshamra.Algorithm.RSA_SHA256)
1339-
ctx.set_hsm_signer(signer)
1359+
# Keep HSM signing on an owned XML string. Authentication and all
1360+
# operation objects must be released before the next token login.
1361+
xml = etree.tostring(relt, encoding='unicode')
1362+
signed = _sign_pkcs11(provider, module_path, pin, label, xml, reference_id=idattr, cert_pem=cert_pem)
1363+
req.t = root(etree.fromstring(signed))
1364+
return req.t
13401365
else:
13411366
# On-disk private key path.
1367+
mgr = pybergshamra.KeysManager()
1368+
ctx = pybergshamra.DsigContext(mgr)
13421369
key = pybergshamra.load_key_file(key_file)
13431370
mgr.add_key(key)
13441371

1345-
if not is_pkcs11_key and is_document_root(relt):
1346-
# Sign the working tree in place through the shared native DOM
1347-
# (pyuppsala document handle). This avoids serializing the whole
1348-
# aggregate, the signer's internal re-parses, and the reparse of the
1349-
# signed result -- downstream pipes keep the very same tree, with the
1350-
# <ds:Signature> inserted as the root's first child.
1351-
#
1352-
# PKCS#11 signing deliberately uses the owned-string path below. In
1353-
# production, mutating a live pyuppsala aggregate through the native
1354-
# document capsule while a vendor PKCS#11 module signs has caused
1355-
# worker SIGSEGVs and partially serialized aggregates. Keeping the HSM
1356-
# operation on an owned document avoids that unsafe integration
1357-
# boundary while preserving the native fast path for software keys.
1372+
if is_document_root(relt):
1373+
# The document API uses an owned XML handoff and imports the signed
1374+
# result into the existing document, preserving downstream tree identity.
13581375
#
13591376
# Only valid when ``relt`` is the document root. After ``first`` (an
13601377
# MDQ single-entity request) the working document is an
@@ -1366,7 +1383,7 @@ def sign(req: Plumbing.Request, *_opts):
13661383
pybergshamra.sign_enveloped_document(ctx, doc, reference_id=idattr, cert_pem=cert_pem)
13671384
req.t = relt
13681385
else:
1369-
# PKCS11 signing, or a subtree of a larger document (small, so the
1386+
# A subtree of a larger document (small, so the
13701387
# string round trip is cheap): serialize the working document, build
13711388
# the enveloped signature, sign, and reparse so downstream pipes see a
13721389
# standalone pyuppsala element.

‎src/pyff/test/test_pipeline.py‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
import os
33
import shutil
44
import tempfile
5+
import threading
56
import time
7+
import weakref
68
from concurrent.futures import ThreadPoolExecutor
79
from io import StringIO
810
from unittest.mock import MagicMock, patch
@@ -377,9 +379,102 @@ class TestPkcs11SigningConfig:
377379

378380
@pytest.fixture(autouse=True)
379381
def _isolated_provider_cache(self):
380-
with patch.object(builtins, '_pkcs11_provider_cache', {}):
382+
with (
383+
patch.object(builtins, '_pkcs11_provider_cache', {}),
384+
patch.object(builtins, '_pkcs11_signing_locks', {}),
385+
):
381386
yield
382387

388+
@pytest.mark.parametrize('failure_stage', ['signer', 'sign'])
389+
def test_signing_failure_releases_session_with_retained_traceback(self, failure_stage):
390+
"""A retained error must not keep a token logged in on the next request."""
391+
sessions = []
392+
393+
class Session:
394+
pass
395+
396+
class Provider:
397+
slot_id = 7
398+
399+
def open_session(self, _pin):
400+
assert all(reference() is None for reference in sessions)
401+
session = Session()
402+
sessions.append(weakref.ref(session))
403+
return session
404+
405+
class Signer:
406+
def __init__(self, session, _label, _algorithm):
407+
self.session = session
408+
409+
class Context:
410+
def __init__(self, _manager):
411+
pass
412+
413+
def set_hsm_signer(self, signer):
414+
self.signer = signer
415+
416+
errors = []
417+
# Native call argument errors model a PyO3 exception without adding a
418+
# Python mock traceback frame that independently retains its arguments.
419+
signer_type = object if failure_stage == 'signer' else Signer
420+
with (
421+
patch.object(builtins.pybergshamra, 'Pkcs11Signer', signer_type),
422+
patch.object(builtins.pybergshamra, 'DsigContext', Context),
423+
):
424+
for _ in range(2):
425+
try:
426+
builtins._sign_pkcs11(Provider(), '/module.so', 'pin', 'key', '<root/>')
427+
except TypeError as error:
428+
errors.append(error)
429+
else:
430+
pytest.fail('Expected a native argument error')
431+
assert all(reference() is None for reference in sessions)
432+
assert len(errors) == len(sessions) == 2
433+
434+
def test_serializes_login_across_providers_for_same_token(self):
435+
"""Aliases for one token cannot overlap authenticated signing lifetimes."""
436+
sessions = []
437+
start = threading.Barrier(2)
438+
439+
class Session:
440+
pass
441+
442+
class Provider:
443+
slot_id = 7
444+
445+
def open_session(self, _pin):
446+
assert all(reference() is None for reference in sessions), 'token already logged in'
447+
session = Session()
448+
sessions.append(weakref.ref(session))
449+
time.sleep(0.05)
450+
return session
451+
452+
class Signer:
453+
def __init__(self, session, _label, _algorithm):
454+
self.session = session
455+
456+
class Context:
457+
def __init__(self, _manager):
458+
pass
459+
460+
def set_hsm_signer(self, signer):
461+
self.signer = signer
462+
463+
def sign(provider):
464+
start.wait(timeout=5)
465+
return builtins._sign_pkcs11(provider, '/module.so', 'pin', 'key', '<root/>')
466+
467+
with (
468+
patch.object(builtins.pybergshamra, 'Pkcs11Signer', Signer),
469+
patch.object(builtins.pybergshamra, 'DsigContext', Context),
470+
patch.object(builtins.pybergshamra, 'sign_enveloped', lambda _ctx, xml, **_kwargs: xml),
471+
ThreadPoolExecutor(max_workers=2) as executor,
472+
):
473+
results = list(executor.map(sign, [Provider(), Provider()]))
474+
assert results == ['<root/>', '<root/>']
475+
assert len(sessions) == 2
476+
assert all(reference() is None for reference in sessions)
477+
383478
def test_parse_absolute_module_path(self):
384479
"""Preserve an absolute module path and read the PIN environment selector."""
385480
module, label, pin, slot_id = builtins._parse_pkcs11_key_uri(

‎src/pyff/utils.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,9 +317,8 @@ def check_signature(t: ElementTree, key: Optional[str], only_one_signature: bool
317317

318318
relt = root(t)
319319
if is_document_root(relt):
320-
# Verify directly against the shared native DOM (pyuppsala document
321-
# handle): no serialization of the working tree and no re-parse
322-
# inside the verifier. Only valid when ``relt`` *is* the document --
320+
# The document API verifies through an owned XML handoff.
321+
# Only valid when ``relt`` *is* the document --
323322
# for a subtree the native document would be the enclosing aggregate.
324323
result = pybergshamra.verify_document(ctx, etree.native_document(relt))
325324
else:

‎uv.lock‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)