Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update monkey patch for python-saml #1421

Closed
AlanCoding opened this issue Mar 2, 2018 · 5 comments
Closed

Update monkey patch for python-saml #1421

AlanCoding opened this issue Mar 2, 2018 · 5 comments

Comments

@AlanCoding
Copy link
Member

AlanCoding commented Mar 2, 2018

Here's the story, the __init__.py of the sso app has some heroic logic (written by @cchurch) in order to avoid a segmentation fault in CentOS triggered in a library that it uses.

awx/awx/sso/__init__.py

Lines 4 to 23 in f907995

# Python
import threading
# Monkeypatch xmlsec.initialize() to only run once (https://github.com/ansible/ansible-tower/issues/3241).
xmlsec_init_lock = threading.Lock()
xmlsec_initialized = False
import dm.xmlsec.binding # noqa
original_xmlsec_initialize = dm.xmlsec.binding.initialize
def xmlsec_initialize(*args, **kwargs):
global xmlsec_init_lock, xmlsec_initialized, original_xmlsec_initialize
with xmlsec_init_lock:
if not xmlsec_initialized:
original_xmlsec_initialize(*args, **kwargs)
xmlsec_initialized = True
dm.xmlsec.binding.initialize = xmlsec_initialize

Related issue for the library SAML-Toolkits/python-saml#30 (comment)

Within the current batch of dependencies, it was found that a segmentation fault occurred (borking the entire app) on version 2.2.3 of python-saml, but not version 2.2.2. This is what changed between the versions.

SAML-Toolkits/python-saml@v2.2.2...v2.2.3

Note that the monkeypatch changed things with the dm.xmlsec.binding, and that the upgrade of python-saml upgraded dm.xmlsec.binding.

Clearly these things are connected.

It looks like we need to alter the monkeypatch logic in awx/sso/__init__.py to be compatible with python-saml>=2.2.3 (specifically its updated dependency, dm.xmlsec.binding 1.3.3).

I'm carding this out separately so it doesn't hold up the timeframe of the rest of dependency updates.

@AlanCoding
Copy link
Member Author

https://pypi.python.org/pypi/dm.xmlsec.binding/1.3.3

1.3.3
Applied patch provided by Robert Frije to make the nsPrefix template parameter work as expected.

That's the closest I can get to change changed in that lib

@AlanCoding
Copy link
Member Author

The segfault seems to be pretty easy to create inside the tools_awx container

(alantest) [root@awx awx_devel]# pip install dm.xmlsec.binding
Collecting dm.xmlsec.binding
  Downloading dm.xmlsec.binding-1.3.3.tar.gz (120kB)
    100% |████████████████████████████████| 122kB 865kB/s 
Requirement already satisfied: setuptools in /alantest/lib/python2.7/site-packages (from dm.xmlsec.binding)
Collecting lxml>=3.0 (from dm.xmlsec.binding)
  Using cached lxml-4.1.1-cp27-cp27mu-manylinux1_x86_64.whl
Building wheels for collected packages: dm.xmlsec.binding
  Running setup.py bdist_wheel for dm.xmlsec.binding ... done
  Stored in directory: /root/.cache/pip/wheels/f2/35/5f/8c63b3968a6b46949c2479eb1d6071b3cb2462a569ba2e409c
Successfully built dm.xmlsec.binding
Installing collected packages: lxml, dm.xmlsec.binding
Successfully installed dm.xmlsec.binding-1.3.3 lxml-4.1.1
(alantest) [root@awx awx_devel]# python
Python 2.7.5 (default, Aug  4 2017, 00:39:18) 
[GCC 4.8.5 20150623 (Red Hat 4.8.5-16)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import dm.xmlsec.binding
Segmentation fault

@rooftopcellist
Copy link
Member

The segfault can also be reproduced by running awx-manage, which also explains why this botches the whole app. Also, the segfault is sticky, once it happens once, you have to clear the container completely and start it again.

Adding the -DXMLSEC_NO_SIZE_T flag to the_flags variable in /usr/bin/xmlsec1-config fixes this in CentOS. This will allow us to update to python-saml 2.4.0.

@AlanCoding
Copy link
Member Author

Is the monkey patch still needed?

chrismeyersfsu pushed a commit to chrismeyersfsu/awx that referenced this issue Apr 26, 2018
…-form

Prevent user from selecting an invalid JT when adding/editing a wfjt node
@AlanCoding
Copy link
Member Author

I've been digging through some history, and it looks like the git requirements are not needed anymore, because of how this problem was resolved:

e519503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants