SAML-Toolkits / python3-saml

MIT License
704 stars 309 forks source link

lxml 4.7.x results in "A valid SubjectConfirmation was not found on this Response" with ADFS #292

Open aquatix opened 2 years ago

aquatix commented 2 years ago

We recently upgraded our lxml from 4.6.4 to 4.7.1 and suddenly there were some SSO issues:

OneLogin works fine, but when a client logs in from an ADFS instance, they get the message that "A valid SubjectConfirmation was not found on this Response".

Moving back to 4.6.4 resolves this issue.

The configuration:

        return {
            'strict': True,
            'debug': True,
            'sp': {
                'entityId': f'{servername}/saml/metadata/{self.logincode}',
                'assertionConsumerService': {
                    'url': f'{servername}/saml/acs/{self.logincode}',
                    'binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
                },
                'singleLogoutService': {
                    'url': f'{servername}/saml/sls/{self.logincode}',
                    'binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
                },
                'NameIDFormat': 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified',
                'x509cert': '',
                'privateKey': '',
            },
            'idp': {
                'entityId': self.idprovider_entity_id,
                'singleSignOnService': {
                    'url': self.idprovider_url,
                    'binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
                },
                'singleLogoutService': {
                    'url': self.idprovider_url,  # TODO: add SLO url property
                    'binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
                },
                'x509cert': self.x509certificate,
            },
            'security':
            {
                'signatureAlgorithm': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256',
                'digestAlgorithm': 'http://www.w3.org/2001/04/xmlenc#sha256',
                'requestedAuthnContext': False,
            },
        }
memocong commented 2 years ago

Encountered the same issue and it only happens when the SAMLResponse is encrypted. The issue can be mitigated by patching the code:

from onelogin.saml2.xmlparser import tostring, fromstring
self.decrypted_document = fromstring(tostring(self.decrypted_document))

Reproduced the issue with the code in demo-django and dockerfile added (reference this repo)

  1. folder structure

    demo-django
    ├── Dockerfile
    ├── demo
    │   ├── __init__.py
    │   ├── settings.py
    │   ├── urls.py
    │   ├── views.py
    │   └── wsgi.py
    ├── docker-compose.yml
    ├── manage.py
    ├── requirements.txt
    ├── saml
    │   ├── advanced_settings.json
    │   ├── certs
    │   │   ├── README
    │   │   ├── sp.crt
    │   │   └── sp.key
    │   └── settings.json
    └── templates
    ├── attrs.html
    ├── base.html
    └── index.html
  2. Dockerfile

FROM python:3.8

RUN apt-get update && apt-get install -y \
    libffi-dev libxmlsec1-dev libxmlsec1-openssl xmlsec1

ADD . /code
WORKDIR /code

ADD requirements.txt /requirements.txt
RUN pip install -r /requirements.txt
  1. requirements.txt

    Django==3.2
    python3-saml
  2. docker-compose.yml

    
    version: '3'

services: web: restart: always build: . command: sh -c "python manage.py runserver 0.0.0.0:9000" volumes:

  1. installed packages
    Package      Version
    ------------ -------
    asgiref      3.4.1
    Django       3.2
    isodate      0.6.1
    lxml         4.7.1
    pip          21.2.4
    python3-saml 1.12.0
    pytz         2021.3
    setuptools   57.5.0
    six          1.16.0
    sqlparse     0.4.2
    wheel        0.37.0
    xmlsec       1.3.12
nijel commented 2 years ago

This issue is also exposed in the python-social-auth testsuite, see https://github.com/python-social-auth/social-core/pull/653

nijel commented 2 years ago

I've tried workaround mentioned by @memocong in https://github.com/onelogin/python3-saml/issues/292#issuecomment-1002604291, and it does somewhat improve the situation, but still is not 100% reliable (without the workaround I get nearly 100% fail rate, with the workaround it is down to something like 20%).

Just for the reference, the workaround patch I've tried:

--- .venv/lib/python3.9/site-packages/onelogin/saml2/response.py.orig   2022-01-14 11:36:50.176430928 +0100
+++ .venv/lib/python3.9/site-packages/onelogin/saml2/response.py    2022-01-14 11:36:53.344432690 +0100
@@ -47,6 +47,8 @@
             decrypted_document = deepcopy(self.document)
             self.encrypted = True
             self.decrypted_document = self.__decrypt_assertion(decrypted_document)
+            from onelogin.saml2.xmlparser import tostring, fromstring
+            self.decrypted_document = fromstring(tostring(self.decrypted_document))

     def is_valid(self, request_data, request_id=None, raise_exceptions=False):
         """

To me it seems like there is some memory corruption going on inside lxml inside __decrypt_assertion.

Also, sometimes this fails with tons of namespace errors:

``` namespace error : Namespace prefix saml2 on Attribute is not defined 1.4.1.5923.1.1.1.1" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined 1.4.1.5923.1.1.1.6" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined e="urn:oid:2.5.4.4" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined 1.4.1.5923.1.1.1.9" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined ="urn:oid:2.5.4.42" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined 1.4.1.5923.1.1.1.7" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined e="urn:oid:2.5.4.3" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix xmlns for xsi on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix xsi for type on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined Value xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string" ^ namespace error : Namespace prefix saml2 on Attribute is not defined .4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" ^ namespace error : Namespace prefix saml2 on AttributeValue is not defined meFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
mredaelli commented 2 years ago

Has it been determined if this is an lxml regression (as I assume it is), or if this needs to be addressed here?

If the former, does anyone have a link to an issue on lxml's bug tracker?

MattiasLevlin commented 2 years ago

Encountered the same "SubjectConfirmation" issue in our SSO solution, which utilizes python3-saml. Downgrading lxml from 4.7.0 to 4.6.4 seems to have solved the issue.

https://github.com/CSCfi/fairdata-sso/releases/tag/1.2.1

AvnerCohen commented 2 years ago

My guess is that this was in issue in the 4.7.0, this version is yanked and is already fixed in 4.7.1 The change log has: https://lxml.de/4.7/changes-4.7.1.html That points to this thread -https://mail.python.org/archives/list/lxml@python.org/thread/6ZFBHFOVHOS5GFDOAMPCT6HM5HZPWQ4Q/

And has been fixed in 4.7.1 if I get this right.

nijel commented 2 years ago

No, it is broken in 4.7.1 and 4.8.0 as well (4.6.5 works fine). The 4.7.0 had an issue that it did not include all files in the release, that's why it's yanked.

AvnerCohen commented 2 years ago

Oh, I see thanks for the clarification! I'll be be updating my comment to avoid confusing people then..

mredaelli commented 2 years ago

No, it is broken in 4.7.1 and 4.8.0 as well (4.6.5 works fine). The 4.7.0 had an issue that it did not include all files in the release, that's why it's yanked.

Then let me ask again: do we know if they know they have a bug? Did someone open an issue somewhere?

AvnerCohen commented 2 years ago

@mredaelli list of open lxml bugs are here - https://bugs.launchpad.net/lxml I think that to be able to check if the bug is open, we need to better understand what is the root cause here, on the lxml level. I don't think I see in this thread anything like that.

nijel commented 2 years ago

I've just verified it happens in 4.9.0 as well. But it seems to happen only with the binary wheels, if I build lxml from the source, it works correctly. Can somebody else confirm such behavior? (I'm testing using python-social-auth testsuite, and it does not trigger the bug in 100%, so there is always some estimation). You can install from the source using pip install --force-reinstall --no-binary :all: lxml==4.9.0

Looking at lxml bugs, this one might be the one we're hitting: https://bugs.launchpad.net/lxml/+bug/1960668

SilviaAmAm commented 2 years ago

I'm wondering if there is any resolution in sight for this issue? lxml has released version 4.9.1 which resolves a vulnerability issue, but this issue blocks the upgrade of projects using python3-saml :slightly_frowning_face:

AndrewGrossman commented 2 years ago

I'm wondering if there is any resolution in sight for this issue? lxml has released version 4.9.1 which resolves a vulnerability issue, but this issue blocks the upgrade of projects using python3-saml 🙁

+1

bofeng commented 2 years ago

I am having the same issue here, however, without changing any source code, this solved my issue:

$ pip uninstall lxml
$ pip install --no-binary lxml lxml==4.7.0
nosnilmot commented 2 years ago

I am having the same issue here, however, without changing any source code, this solved my issue:

$ pip uninstall lxml
$ pip install --no-binary lxml lxml==4.7.0

yes, that works, but you would be better to install lxml==4.9.1

https://bugs.launchpad.net/lxml/+bug/1960668 is the canonical upstream lxml bug report. It still isn't clear if this is solvable with code changes in one or more of lxml, xmlsec and libxml2, or if the above is the correct long-term solution to ensure xmlsec and lxml are using the same version of libxml2.

jeffsawatzky commented 1 year ago

For those of you installing python3-saml using a requirements.txt file, try doing something like this:

--no-binary lxml
# other deps
python3-saml
# more deps