From d61c56ac52252cc1efa15401558cdfeb68839680 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 25 Jan 2024 14:41:12 +0100 Subject: [PATCH 1/4] :alien: Replace deprecated defusedxml.lxml module usage --- digid_eherkenning/_xml.py | 17 +++++++++++++++++ digid_eherkenning/saml2/eherkenning.py | 3 +-- digid_eherkenning/utils.py | 3 ++- 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 digid_eherkenning/_xml.py diff --git a/digid_eherkenning/_xml.py b/digid_eherkenning/_xml.py new file mode 100644 index 0000000..e6470c9 --- /dev/null +++ b/digid_eherkenning/_xml.py @@ -0,0 +1,17 @@ +""" +XML parsing with DTD/Entities blocking. + +Inspired by https://github.com/mvantellingen/python-zeep/pull/1179/ as their solution +for the deprecated defusedxml.lxml module and the defaults applied in defusedxml.lxml. +""" +from lxml.etree import XMLParser, parse as _parse + + +def parse(source): + """ + Parse an LXML etree from source without resolving entities. + + Resolving entities is a security risk, which is why we disable it. + """ + parser = XMLParser(resolve_entities=False) + return _parse(source, parser) diff --git a/digid_eherkenning/saml2/eherkenning.py b/digid_eherkenning/saml2/eherkenning.py index 5051e62..01ca00f 100644 --- a/digid_eherkenning/saml2/eherkenning.py +++ b/digid_eherkenning/saml2/eherkenning.py @@ -7,10 +7,9 @@ from django.urls import reverse from django.utils import timezone -from defusedxml.lxml import tostring from furl.furl import furl from lxml.builder import ElementMaker -from lxml.etree import Element +from lxml.etree import Element, tostring from OpenSSL import crypto from ..choices import AssuranceLevels diff --git a/digid_eherkenning/utils.py b/digid_eherkenning/utils.py index a86c191..abe02e0 100644 --- a/digid_eherkenning/utils.py +++ b/digid_eherkenning/utils.py @@ -2,9 +2,10 @@ from django.conf import settings -from defusedxml.lxml import parse from lxml import etree +from ._xml import parse + def get_client_ip(request): x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR") From dbc8fba6f8f67a7424add612fa05d5adac314634 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 12 Feb 2024 18:42:53 +0100 Subject: [PATCH 2/4] :heavy_minus_sign: Remove explicit defusedxml dependency In 2024 this is no longer essential, using lxml by itself should be sufficient. See also the discussion on python.org: https://discuss.python.org/t/status-of-defusedxml-and-recommendation-in-docs/34762/3 --- digid_eherkenning/_xml.py | 1 + setup.cfg | 1 - tests/mixins.py | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/digid_eherkenning/_xml.py b/digid_eherkenning/_xml.py index e6470c9..45bda52 100644 --- a/digid_eherkenning/_xml.py +++ b/digid_eherkenning/_xml.py @@ -4,6 +4,7 @@ Inspired by https://github.com/mvantellingen/python-zeep/pull/1179/ as their solution for the deprecated defusedxml.lxml module and the defaults applied in defusedxml.lxml. """ + from lxml.etree import XMLParser, parse as _parse diff --git a/setup.cfg b/setup.cfg index e0934bd..72136df 100644 --- a/setup.cfg +++ b/setup.cfg @@ -39,7 +39,6 @@ install_requires = django-simple-certmanager django-solo lxml - defusedxml>=0.7.0 furl maykin-python3-saml tests_require = diff --git a/tests/mixins.py b/tests/mixins.py index e8bd554..6103565 100644 --- a/tests/mixins.py +++ b/tests/mixins.py @@ -1,6 +1,7 @@ """ TODO: replace with pytest fixtures? """ + from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration From e5ff5281b8a6f424c1a6feb776ac5ccc5a16f869 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 15 Feb 2024 12:49:01 +0100 Subject: [PATCH 3/4] :pushpin: Require at least lxml 4.7.1 This repeats the lower bound from maykin-python3-saml but makes it explicit. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 72136df..7fbde0d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -38,7 +38,7 @@ install_requires = django-sessionprofile django-simple-certmanager django-solo - lxml + lxml >= 4.7.1 furl maykin-python3-saml tests_require = From 1e68b0fa0c316cdcee1f274710bbe64ef5cb77de Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 15 Feb 2024 15:50:29 +0100 Subject: [PATCH 4/4] :pushpin: Set lower bound for our python3-saml fork This should mostly be a no-op except that it effectively purges defusedxml.lxml usage. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 7fbde0d..4b815f0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -40,7 +40,7 @@ install_requires = django-solo lxml >= 4.7.1 furl - maykin-python3-saml + maykin-python3-saml >= 1.16.0 tests_require = django-test-migrations freezegun