From 05b43b6f4d66b6912f4e7755f9b2c8bb7ca100a7 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Wed, 18 Jun 2025 12:45:08 +0200 Subject: [PATCH 01/10] Allow for multiple signing certificates --- src/pyff/samlmd.py | 2 +- src/pyff/utils.py | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/pyff/samlmd.py b/src/pyff/samlmd.py index a0047a6c..106de107 100644 --- a/src/pyff/samlmd.py +++ b/src/pyff/samlmd.py @@ -305,7 +305,7 @@ def parse(self, resource: Resource, content: str) -> EidasMDParserInfo: fingerprints = list(certs.keys()) fp = None if len(fingerprints) > 0: - fp = fingerprints[0] + fp = fingerprints ep = ml.find("{{{}}}Endpoint".format(NS['ser'])) if ep is not None and fp is not None: diff --git a/src/pyff/utils.py b/src/pyff/utils.py index 3df96e65..f4154c42 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -264,15 +264,23 @@ def redis(): def check_signature(t: ElementTree, key: Optional[str], only_one_signature: bool = False) -> ElementTree: + refs = None if key is not None: - log.debug(f"verifying signature using {key}") - refs = xmlsec.verified(t, key, drop_signature=True) - if only_one_signature and len(refs) != 1: - raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) - t = refs[0] # prevent wrapping attacks + for k in key: + log.debug(f"verifying signature using {k}") + try: + refs = xmlsec.verified(t, k, drop_signature=True) + except xmlsec.exceptions.XMLSigException: + continue + if refs: + if only_one_signature and len(refs) != 1: + raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) + t = refs[0] # prevent wrapping attacks + return t - return t + raise MetadataException("No valid signature(s) found") + return t def validate_document(t): schema().assertValid(t) From 03e96bf3ea83dfd54875561743407ff73ccc0500 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 10:04:14 +0200 Subject: [PATCH 02/10] Allow for multiple signed parts with diffrent keys Even though we don't allow it later on (for now?). --- src/pyff/utils.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/pyff/utils.py b/src/pyff/utils.py index f4154c42..64f3aa74 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -264,21 +264,21 @@ def redis(): def check_signature(t: ElementTree, key: Optional[str], only_one_signature: bool = False) -> ElementTree: - refs = None if key is not None: + refs = [] for k in key: log.debug(f"verifying signature using {k}") try: - refs = xmlsec.verified(t, k, drop_signature=True) + refs = refs + xmlsec.verified(t, k, drop_signature=True) except xmlsec.exceptions.XMLSigException: continue - if refs: - if only_one_signature and len(refs) != 1: - raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) - t = refs[0] # prevent wrapping attacks - return t - raise MetadataException("No valid signature(s) found") + if refs: + if only_one_signature and len(refs) != 1: + raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) + t = refs[0] # prevent wrapping attacks + else: + raise MetadataException("No valid signature(s) found") return t From a5d7975cf908413a5a588d7855f9f141d9cfdfd8 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 10:54:41 +0200 Subject: [PATCH 03/10] Improved naming for when handling multiple certs --- src/pyff/resource.py | 2 +- src/pyff/samlmd.py | 9 +++------ src/pyff/utils.py | 10 +++++----- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/pyff/resource.py b/src/pyff/resource.py index 4d0d1177..a9c07cd9 100644 --- a/src/pyff/resource.py +++ b/src/pyff/resource.py @@ -160,7 +160,7 @@ def i_handle(self, t: Resource, url=None, response=None, exception=None, last_fe class ResourceOpts(BaseModel): alias: str | None = Field(None, alias='as') # TODO: Rename to 'name'? # a certificate (file) or a SHA1 fingerprint to use for signature verification - verify: str | None = None + verify: list[str] = [] # TODO: move classes to make the correct typing work: Iterable[Union[Lambda, PipelineCallback]] = Field([]) via: list[Callable] = Field([]) # A list of callbacks that can be used to pre-process parsed metadata before validation. Use as a clue-bat. diff --git a/src/pyff/samlmd.py b/src/pyff/samlmd.py index 106de107..d6a4e483 100644 --- a/src/pyff/samlmd.py +++ b/src/pyff/samlmd.py @@ -303,23 +303,20 @@ def parse(self, resource: Resource, content: str) -> EidasMDParserInfo: if location: certs = CertDict(ml) fingerprints = list(certs.keys()) - fp = None - if len(fingerprints) > 0: - fp = fingerprints ep = ml.find("{{{}}}Endpoint".format(NS['ser'])) - if ep is not None and fp is not None: + if ep is not None and len(fingerprints) != 0: args = dict( country_code=mdl.get('Territory'), hide_from_discovery=str2bool(ep.get('HideFromDiscovery', 'false')), ) log.debug( "MDSL[{}]: {} verified by {} for country {}".format( - info.scheme_territory, location, fp, args.get('country_code') + info.scheme_territory, location, fingerprints, args.get('country_code') ) ) child_opts = resource.opts.model_copy(update={'alias': None}, deep=True) - child_opts.verify = fp + child_opts.verify = fingerprints r = resource.add_child(location, child_opts) # this is specific post-processing for MDSL files diff --git a/src/pyff/utils.py b/src/pyff/utils.py index 64f3aa74..b6cee3d1 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -263,13 +263,13 @@ def redis(): return thread_data.redis -def check_signature(t: ElementTree, key: Optional[str], only_one_signature: bool = False) -> ElementTree: - if key is not None: +def check_signature(t: ElementTree, keys: list = [], only_one_signature: bool = False) -> ElementTree: + if len(keys) != 0: refs = [] - for k in key: - log.debug(f"verifying signature using {k}") + for key in keys: + log.debug(f"verifying signature using {key}") try: - refs = refs + xmlsec.verified(t, k, drop_signature=True) + refs = refs + xmlsec.verified(t, key, drop_signature=True) except xmlsec.exceptions.XMLSigException: continue From 8ed08545a4bf961111038eb67d302f9c2c777fa4 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 11:56:47 +0200 Subject: [PATCH 04/10] Better handle if no valid signs are found --- src/pyff/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pyff/utils.py b/src/pyff/utils.py index b6cee3d1..21da084c 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -273,7 +273,9 @@ def check_signature(t: ElementTree, keys: list = [], only_one_signature: bool = except xmlsec.exceptions.XMLSigException: continue - if refs: + if not refs: + raise MetadataException("No valid signature(s) found") + else: if only_one_signature and len(refs) != 1: raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) t = refs[0] # prevent wrapping attacks From 9f94ce3796ee9acf95ad9e70d9876e65c9ea37a5 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 11:57:36 +0200 Subject: [PATCH 05/10] Help future dev --- src/pyff/utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pyff/utils.py b/src/pyff/utils.py index 21da084c..371b9f37 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -278,9 +278,7 @@ def check_signature(t: ElementTree, keys: list = [], only_one_signature: bool = else: if only_one_signature and len(refs) != 1: raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) - t = refs[0] # prevent wrapping attacks - else: - raise MetadataException("No valid signature(s) found") + t = refs[0] # prevent wrapping attacks, also doesn't pyff.samlmd.parse_saml_metadata handle when multiple trees are returned return t From 79fa135199829548c2edfb3bf10f4f7fa487f649 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 12:04:44 +0200 Subject: [PATCH 06/10] Improve language --- src/pyff/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pyff/utils.py b/src/pyff/utils.py index 371b9f37..3e8be0f8 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -278,7 +278,10 @@ def check_signature(t: ElementTree, keys: list = [], only_one_signature: bool = else: if only_one_signature and len(refs) != 1: raise MetadataException("XML metadata contains %d signatures - exactly 1 is required" % len(refs)) - t = refs[0] # prevent wrapping attacks, also doesn't pyff.samlmd.parse_saml_metadata handle when multiple trees are returned + # Make sure to only return one tree: + # - prevent wrapping attacks + # - pyff.samlmd.parse_saml_metadata doesn't handle when multiple trees are returned + t = refs[0] return t From 19305edc8ddf771e2b0c3448e6e3cb0051ec5951 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 13:21:35 +0200 Subject: [PATCH 07/10] Compatibilty with python3.9 --- src/pyff/resource.py | 4 ++-- src/pyff/utils.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pyff/resource.py b/src/pyff/resource.py index a9c07cd9..4d38a161 100644 --- a/src/pyff/resource.py +++ b/src/pyff/resource.py @@ -13,7 +13,7 @@ from datetime import datetime from enum import Enum from threading import Condition, Lock -from typing import TYPE_CHECKING, Any, Callable +from typing import TYPE_CHECKING, Any, Callable, Optional from urllib.parse import quote as urlescape import requests @@ -160,7 +160,7 @@ def i_handle(self, t: Resource, url=None, response=None, exception=None, last_fe class ResourceOpts(BaseModel): alias: str | None = Field(None, alias='as') # TODO: Rename to 'name'? # a certificate (file) or a SHA1 fingerprint to use for signature verification - verify: list[str] = [] + verify: Optional[list[str]] = None # TODO: move classes to make the correct typing work: Iterable[Union[Lambda, PipelineCallback]] = Field([]) via: list[Callable] = Field([]) # A list of callbacks that can be used to pre-process parsed metadata before validation. Use as a clue-bat. diff --git a/src/pyff/utils.py b/src/pyff/utils.py index 3e8be0f8..a43eef08 100644 --- a/src/pyff/utils.py +++ b/src/pyff/utils.py @@ -263,8 +263,8 @@ def redis(): return thread_data.redis -def check_signature(t: ElementTree, keys: list = [], only_one_signature: bool = False) -> ElementTree: - if len(keys) != 0: +def check_signature(t: ElementTree, keys: Optional[list[str]] = None, only_one_signature: bool = False) -> ElementTree: + if keys: refs = [] for key in keys: log.debug(f"verifying signature using {key}") From 548cdf37c69260776cfeaa45eefd8233643c2277 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 14:03:33 +0200 Subject: [PATCH 08/10] Verify should be a list nowadays --- src/pyff/builtins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pyff/builtins.py b/src/pyff/builtins.py index 9c560956..eb56bbc7 100644 --- a/src/pyff/builtins.py +++ b/src/pyff/builtins.py @@ -686,7 +686,7 @@ def load(req: Plumbing.Request, *opts): if elt == "as": child_opts.alias = value elif elt == "verify": - child_opts.verify = value + child_opts.verify = [value] elif elt == "via": child_opts.via.append(PipelineCallback(value, req, store=req.md.store)) elif elt == "cleanup": @@ -698,7 +698,7 @@ def load(req: Plumbing.Request, *opts): "Usage: load resource [as url] [[verify] verification] [via pipeline]* [cleanup pipeline]*" ) else: - child_opts.verify = elt + child_opts.verify = [elt] # override anything in child_opts with what is in opts child_opts = child_opts.model_copy(update=_opts) From 748657a9f678c12e748358ad96e5378bd099e7fe Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 14:27:47 +0200 Subject: [PATCH 09/10] All for multiple certs in configuration --- src/pyff/builtins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pyff/builtins.py b/src/pyff/builtins.py index eb56bbc7..dca81346 100644 --- a/src/pyff/builtins.py +++ b/src/pyff/builtins.py @@ -686,7 +686,7 @@ def load(req: Plumbing.Request, *opts): if elt == "as": child_opts.alias = value elif elt == "verify": - child_opts.verify = [value] + child_opts.verify = value.split(",") elif elt == "via": child_opts.via.append(PipelineCallback(value, req, store=req.md.store)) elif elt == "cleanup": @@ -698,7 +698,7 @@ def load(req: Plumbing.Request, *opts): "Usage: load resource [as url] [[verify] verification] [via pipeline]* [cleanup pipeline]*" ) else: - child_opts.verify = [elt] + child_opts.verify = elt.split(",") # override anything in child_opts with what is in opts child_opts = child_opts.model_copy(update=_opts) From 0bf138f2996ad3940104c96442794179f10c1762 Mon Sep 17 00:00:00 2001 From: Johan Wassberg Date: Thu, 19 Jun 2025 14:32:51 +0200 Subject: [PATCH 10/10] It's plural now --- src/pyff/resource.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pyff/resource.py b/src/pyff/resource.py index 4d38a161..4ab07a01 100644 --- a/src/pyff/resource.py +++ b/src/pyff/resource.py @@ -159,7 +159,7 @@ def i_handle(self, t: Resource, url=None, response=None, exception=None, last_fe class ResourceOpts(BaseModel): alias: str | None = Field(None, alias='as') # TODO: Rename to 'name'? - # a certificate (file) or a SHA1 fingerprint to use for signature verification + # a list of certificate (file(s)) or a SHA1 fingerprint(s) to use for signature verification verify: Optional[list[str]] = None # TODO: move classes to make the correct typing work: Iterable[Union[Lambda, PipelineCallback]] = Field([]) via: list[Callable] = Field([])