Skip to content

pysaml2 does not support certificate passwords #278

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.

Sign up for GitHub

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

Jump to bottom
Open
erickt opened this issue Oct 30, 2015 · 13 comments
Open

pysaml2 does not support certificate passwords #278

erickt opened this issue Oct 30, 2015 · 13 comments
Labels
closing feature wontfix

Comments

@erickt
Copy link
Contributor

erickt commented Oct 30, 2015

It appears that pysaml2 does not support certificates with passwords, which results in OpenSSL prompting for a password when the certificate is parsed. I don't think it'd be terribly hard to support, we would just have to update every instance of RSA.importKey to pass in an optionally configured passphrase.
@rohe
Copy link
Contributor

rohe commented Nov 1, 2015

30 okt 2015 kl. 10:52 skrev Erick Tryzelaar [email protected]:

It appears that pysaml2 does not support certificates with passwords, which results in OpenSSL prompting for a password when the certificate is parsed. I don’t think it'd be terribly hard to support, we would just have to update every instance of RSA.importKey to pass in an optionally configured passphrase.

I’d appreciate if you would send in a pull request otherwise I will do it but not within the next week.

— Roland
'Look, that's why there's rules, understand? So that you think before you break ’em.’ - Terry Pratchett

@erickt
Copy link
Contributor Author

erickt commented Nov 2, 2015

Hello @rohe: I'm slowly working on a PR. Current challenge is that xmlsec1 currently only allows passwords to be passed in on the commandline, or through an interactive prompt. The former is insecure, since one could either see the argument with ps or in the .bash_history, the latter requires using a PTY, which makes it not portable to some OSs. The right solution is probably to switch over to using python-xmlsec and exposing the xmlsec API that passes the password along to the underlying library. For hue, we're probably first going to implement the PTY approach for the short term, and then over time implement the python-xmlsec. Would you be interested in the PTY? If not, I can just patch my fork, and then upstream the python-xmlsec version once it's done.

@rohe
Copy link
Contributor

rohe commented Nov 3, 2015

Hi Erick,

2 nov 2015 kl. 13:24 skrev Erick Tryzelaar [email protected]:

Hello @rohe: I'm slowly working on a PR. Current challenge is that xmlsec1 currently only allows passwords to be passed in on the commandline, or through an interactive prompt. The former is insecure, since one could either see the argument with ps or in the .bash_history, the latter requires using a PTY, which makes it not portable to some OSs. The right solution is probably to switch over to using python-xmlsec and exposing the xmlsec API that passes the password along to the underlying library. For hue, we're probably first going to implement the PTY approach for the short term, and then over time implement the python-xmlsec. Would you be interested in the PTY?

I would.

If not, I can just patch my fork, and then upstream the python-xmlsec version once it's done.

I’d like to have the python-xmlsec version too.
When I started implementing pysaml2, python-xmlsec was in such a bad state that I didn’t really
consider it to be a viable option. If it’s better today I’d really like to have it as an option beside xmlsec.

— Roland
'Look, that's why there's rules, understand? So that you think before you break ’em.’ - Terry Pratchett

@erickt
Copy link
Contributor Author

erickt commented Nov 3, 2015

Unfortunately I don't actually know the state of python-xmlsec. It seems to have some fairly regular downloads (1289 in the past month), but I'm not personally familiar with the project.

@rectalogic
Copy link
Contributor

dm.xmlsec.binding might be an alternative to python-xmlsec. Onelogin python-saml uses it https://github.com/onelogin/python-saml/blob/master/src/onelogin/saml2/utils.py#L712

@rohe
Copy link
Contributor

rohe commented Jan 15, 2016

14 jan 2016 kl. 19:01 skrev Andrew Wason [email protected]:

dm.xmlsec.binding might be an alternative to python-xmlsec. Onelogin python-saml uses it https://github.com/onelogin/python-saml/blob/master/src/onelogin/saml2/utils.py#L712

Do you know anything about their plans to support Python 3.X ?
They only list support for 2.4, 2.6 and 2.7 now.

— Roland

erickt pushed a commit to cloudera/hue that referenced this issue Jan 22, 2016
HUE-3102 [libsaml] Add support for private key passwords
961d43a 
This is also tracked by IdentityPython/pysaml2#278.

This patch adds support for SAML certificates that are protected
with a password. The way it does so is with a bit of trickiness,
due to the fact that `xmlsec1`, which is an external program that
pysaml2 uses to sign the XML requests, which does not have great
support for password protected certificates. It either supports
passing in the password on the command line (which is not safe
since someone else on the machine could see the password), or
through an interactive prompt.

The proper way to fix this would be to update pysaml2 to use
another xmlsec library, but implementing that may take some
time. In the short/medium term, this patch implements this
instead by decrypting the certificate in memory, and passing
this decrypted certificate to xmlsec1 through a named pipe.
This protects us from the decrypted certificate ever hitting
the disk.

Unfortunately, this solution is only portable to
POSIX-compatible platforms. That is fine for Hue, but it
probably means we cannot push this patch to the upstream
pysaml2 repository. This patch will tied us over until
the upstream project switches to a better xmlsec library.
@erickt
Copy link
Contributor Author

erickt commented Jan 22, 2016

Just to let you know, in Hue we've implemented a short term solution in our fork of pysaml2. It decrypts the certificate in memory, then passes it to xmlsec1 through a named pipe. I didn't submit it here for review because this technique only works on POSIX systems, and I'm not sure if pysaml2 supports Windows.

@rohe
Copy link
Contributor

rohe commented Feb 11, 2016

22 jan 2016 kl. 19:23 skrev Erick Tryzelaar [email protected]:

Just to let you know, in Hue we've implemented a short term solution in our fork of pysaml2. It decrypts the certificate in memory, then passes it to xmlsec1 through a named pipe. I didn't submit it here for review because this technique only works on POSIX systems, and I'm not sure if pysaml2 supports Windows.

I and my group are mainly concerned with *nix based systems.
I would prefer if we didn’t, well ware of the consequences, add something that would make it impossible to use pysaml2 on Windows.

If you could add certificate password support such that it would work on a POSIX system but not be available on other systems I would gladly accept such a pull request.

”Everybody should be quiet near a little stream and listen."
From ’Open House for Butterflies’ by Ruth Krauss

@erickt
Copy link
Contributor Author

erickt commented Feb 11, 2016

@rohe: Yeah that would be the best solution. We thought though that we wouldn't have the time to validate saml using a new xmlsec library for our next release. Long term we're hoping switching pysaml2 to a python binding of xmlsec like dm.xmlsec.binding would be the better option.

@rohe
Copy link
Contributor

rohe commented Feb 12, 2016

11 feb 2016 kl. 19:22 skrev Erick Tryzelaar [email protected]:

@rohe: Yeah that would be the best solution. We thought though that we wouldn't have the time to validate saml using a new xmlsec library for our next release. Long term we're hoping switching pysaml2 to a python binding of xmlsec like dm.xmlsec.binding would be the better option.

Agree !
The reason for me not to use the python bindings for xmlsec when I started this project almost a decade ago was
that the bindings was in such a poor state and I didn’t want to take on the responsibility for keeping them
maintained. That may have changed (the first reason not the second :-)).

— Roland

”Everybody should be quiet near a little stream and listen."
From ’Open House for Butterflies’ by Ruth Krauss

@pitbulk
Copy link

pitbulk commented Feb 14, 2016

Hi @rohe,

I'm the developer of OneLogin's SAML toolkits.
I used your pysaml2 library when I was working at Yaco with Lorenzo Gil and saw you at Terena conferences.

At python-saml I used dm.xmlsec.binding so it only works on Python 2.X

After experience some installing problems and the request of python3 support I wanted to release a new version of the same toolkit based in a python3 compatible solution.

@bgaifullin, that currently help supporting python-xmlsec did a great job and helped me to migrate part of the code, and python3-saml appeared, based on python-xmlsec instead of dm.xmlsec.binding, working on python2 and python3.

@mehcode, the main maintainer of python-xmlsec has its own SAML library

So nowadays SAML developers have several alternatives :)

Best regards and thanks for your contributions.

cloudera-hudson pushed a commit to cloudera/hue that referenced this issue Apr 8, 2016
@romainr
HUE-3102 [libsaml] Add support for private key passwords
e842535 
This is also tracked by IdentityPython/pysaml2#278.

This patch adds support for SAML certificates that are protected
with a password. The way it does so is with a bit of trickiness,
due to the fact that `xmlsec1`, which is an external program that
pysaml2 uses to sign the XML requests, which does not have great
support for password protected certificates. It either supports
passing in the password on the command line (which is not safe
since someone else on the machine could see the password), or
through an interactive prompt.

The proper way to fix this would be to update pysaml2 to use
another xmlsec library, but implementing that may take some
time. In the short/medium term, this patch implements this
instead by decrypting the certificate in memory, and passing
this decrypted certificate to xmlsec1 through a named pipe.
This protects us from the decrypted certificate ever hitting
the disk.

Unfortunately, this solution is only portable to
POSIX-compatible platforms. That is fine for Hue, but it
probably means we cannot push this patch to the upstream
pysaml2 repository. This patch will tied us over until
the upstream project switches to a better xmlsec library.
@c00kiemon5ter c00kiemon5ter mentioned this issue Aug 19, 2017
Open
@peppelinux peppelinux mentioned this issue Jul 21, 2019
Open
ranade1 added a commit to cloudera/hue that referenced this issue Apr 9, 2020
@ranade1
HUE-3102 [libsaml] Add support for private key passwords
966b270 
This is also tracked by IdentityPython/pysaml2#278.

This patch adds support for SAML certificates that are protected
with a password. The way it does so is with a bit of trickiness,
due to the fact that `xmlsec1`, which is an external program that
pysaml2 uses to sign the XML requests, which does not have great
support for password protected certificates. It either supports
passing in the password on the command line (which is not safe
since someone else on the machine could see the password), or
through an interactive prompt.

The proper way to fix this would be to update pysaml2 to use
another xmlsec library, but implementing that may take some
time. In the short/medium term, this patch implements this
instead by decrypting the certificate in memory, and passing
this decrypted certificate to xmlsec1 through a named pipe.
This protects us from the decrypted certificate ever hitting
the disk.

Unfortunately, this solution is only portable to
POSIX-compatible platforms. That is fine for Hue, but it
probably means we cannot push this patch to the upstream
pysaml2 repository. This patch will tied us over until
the upstream project switches to a better xmlsec library.
ranade1 added a commit to cloudera/hue that referenced this issue Apr 9, 2020
@ranade1
HUE-3102 [libsaml] Add support for private key passwords
177c115 
This is also tracked by IdentityPython/pysaml2#278.

This patch adds support for SAML certificates that are protected
with a password. The way it does so is with a bit of trickiness,
due to the fact that `xmlsec1`, which is an external program that
pysaml2 uses to sign the XML requests, which does not have great
support for password protected certificates. It either supports
passing in the password on the command line (which is not safe
since someone else on the machine could see the password), or
through an interactive prompt.

The proper way to fix this would be to update pysaml2 to use
another xmlsec library, but implementing that may take some
time. In the short/medium term, this patch implements this
instead by decrypting the certificate in memory, and passing
this decrypted certificate to xmlsec1 through a named pipe.
This protects us from the decrypted certificate ever hitting
the disk.

Unfortunately, this solution is only portable to
POSIX-compatible platforms. That is fine for Hue, but it
probably means we cannot push this patch to the upstream
pysaml2 repository. This patch will tied us over until
the upstream project switches to a better xmlsec library.
ranade1 added a commit to cloudera/hue that referenced this issue Apr 10, 2020
@ranade1 @wing2fly
Hue Aquascan CVEs (#1095)
b99879b 
* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
 - Adding Django-1.11.29

* HUE-8905 [core] Apply HUE-8772 to Django-1.11.22 for fixing 'user is missing in mako context'

(cherry picked from commit cf717de)

* HUE-8905 [core] Apply HUE-8836 to Django-1.11.22 for HTTP_X_FORWARDED_HOST contains multiple hosts

(cherry picked from commit 26e5f7a)

* Revert "HUE-8905 [core] Apply HUE-8772 to Django-1.11.22 for fixing 'user is missing in mako context'"

This reverts commit cf717de.

(cherry picked from commit 9014bfa)

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Removing Django-1.11.22

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding cryptography-2.9
- Removing cryptography-2.1.4

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding MarkupSafe-1.1.1
- Removing MarkupSafe-0.9.3

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding PyYAML-5.3.1
- Removing PyYAML-3.12

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding requests-2.23.0
- Removing requests-2.18.4

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding urllib3-1.22
- Removing urllib3-1.25.8

* HUE-5095 [backend] Python requests library should put port information in log message

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding requests-kerberos-0.12.0
- Removing requests-kerberos-0.6.1

* HUE-7127 [backend] Fix for Python requests Kerberos/GSSAPI authentication library fails to authenticate kerberos requests to the same destination

current python request Kerberos library fails generating the GSSAPI authentication token with kerberos for the host which run multiple kerberised

Testing Done:
- Manual testing
  - using multiple load generator scripts
- Tested on different cluster with Python 2.6 and Python 2.7

* HUE-8202 [jb] Fix mutual authentication failed with Isilon (#675)

https://review.cloudera.org/r/12820/

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding tablib-0.14.0
- Removing tablib-0.10.0

* HUE-2523 [core] Remove xlwt and xlrd from tablib

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding MarkupPy-1.14

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding openpyxl-2.6.4
- Removing openpyxl-2.3.0-b2
- Removing openpyxl-2.5.3

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding odfpy-1.4.1

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding pysaml2-4.9.0
- Removing pysaml2-4.4.0

* HUE-3102 [libsaml] Add support for private key passwords

This is also tracked by IdentityPython/pysaml2#278.

This patch adds support for SAML certificates that are protected
with a password. The way it does so is with a bit of trickiness,
due to the fact that `xmlsec1`, which is an external program that
pysaml2 uses to sign the XML requests, which does not have great
support for password protected certificates. It either supports
passing in the password on the command line (which is not safe
since someone else on the machine could see the password), or
through an interactive prompt.

The proper way to fix this would be to update pysaml2 to use
another xmlsec library, but implementing that may take some
time. In the short/medium term, this patch implements this
instead by decrypting the certificate in memory, and passing
this decrypted certificate to xmlsec1 through a named pipe.
This protects us from the decrypted certificate ever hitting
the disk.

Unfortunately, this solution is only portable to
POSIX-compatible platforms. That is fine for Hue, but it
probably means we cannot push this patch to the upstream
pysaml2 repository. This patch will tied us over until
the upstream project switches to a better xmlsec library.

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding chardet-3.0.4

* HUE-9225 [core] Upgrade certain third party python libraries that has identified vulnerabilities
- Adding pycryptodomex-3.9.7
- Removing pycrypto-2.6.1
- Removing pycryptodomex-3.4.7

Co-authored-by: Ying Chen <[email protected]>
@peppelinux
Copy link
Member

The final solution for this kind of problem would be converting (using openssl) a key (or certificate) encoded with a password to one without.

openssl rsa -in [original.key] -out [new.key]

@peppelinux peppelinux mentioned this issue Sep 6, 2020
Closed
6 tasks
@peppelinux peppelinux reopened this Sep 13, 2020
@peppelinux
Copy link
Member

@c00kiemon5ter can we close this or do you want to follow It to implement this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
closing feature wontfix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@rectalogic @erickt @rohe @pitbulk @peppelinux