Browse Source

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

This is also tracked by https://github.com/rohe/pysaml2/issues/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.
Erick Tryzelaar 9 years ago
parent
commit
961d43a535

+ 15 - 3
desktop/core/ext-py/pysaml2-2.4.0/src/saml2/config.py

@@ -48,9 +48,20 @@ ONTS = {
 }
 
 COMMON_ARGS = [
-    "entityid", "xmlsec_binary", "debug", "key_file", "cert_file",
-    "encryption_type", "secret", "accepted_time_diff", "name", "ca_certs",
-    "description", "valid_for", "verify_ssl_cert",
+    "entityid",
+    "xmlsec_binary",
+    "debug",
+    "key_file",
+    "key_file_passphrase",
+    "cert_file",
+    "encryption_type",
+    "secret",
+    "accepted_time_diff",
+    "name",
+    "ca_certs",
+    "description",
+    "valid_for",
+    "verify_ssl_cert",
     "organization",
     "contact_person",
     "name_form",
@@ -185,6 +196,7 @@ class Config(object):
         self.xmlsec_path = []
         self.debug = False
         self.key_file = None
+        self.key_file_passphrase = None
         self.cert_file = None
         self.encryption_type = 'both'
         self.secret = None

+ 2 - 1
desktop/core/ext-py/pysaml2-2.4.0/src/saml2/entity.py

@@ -136,7 +136,8 @@ class Entity(HTTPBase):
 
         try:
             self.signkey = RSA.importKey(
-                open(self.config.getattr("key_file", ""), 'r').read())
+                open(self.config.getattr("key_file", ""), 'r').read(),
+                passphrase=self.config.key_file_passphrase)
         except (KeyError, TypeError):
             self.signkey = None
 

+ 88 - 22
desktop/core/ext-py/pysaml2-2.4.0/src/saml2/sigver.py

@@ -40,7 +40,7 @@ from saml2.time_util import instant
 from saml2.time_util import utc_now
 from saml2.time_util import str_to_time
 
-from tempfile import NamedTemporaryFile
+from tempfile import NamedTemporaryFile, mkdtemp
 from subprocess import Popen, PIPE
 
 from xmldsig import SIG_RSA_SHA1
@@ -563,6 +563,24 @@ def parse_xmlsec_output(output):
 def sha1_digest(msg):
     return hashlib.sha1(msg).digest()
 
+# --------------------------------------------------------------------------
+
+class NamedPipe(object):
+    def __init__(self):
+        self._tempdir = mkdtemp()
+        self.name = os.path.join(self._tempdir, 'fifo')
+
+        try:
+            os.mkfifo(self.name)
+        except:
+            os.rmdir(self._tempdir)
+
+    def close(self):
+        os.remove(self.name)
+        os.rmdir(self._tempdir)
+
+# --------------------------------------------------------------------------
+
 
 class Signer(object):
     """Abstract base class for signing algorithms."""
@@ -699,7 +717,7 @@ class CryptoBackend():
                           node_xpath):
         raise NotImplementedError()
 
-    def decrypt(self, enctext, key_file):
+    def decrypt(self, enctext, key_file, passphrase=None):
         raise NotImplementedError()
 
     def sign_statement(self, statement, node_name, key_file, node_id,
@@ -799,7 +817,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
 
         return output
 
-    def decrypt(self, enctext, key_file):
+    def decrypt(self, enctext, key_file, passphrase=None):
         """
 
         :param enctext: XML document containing an encrypted part
@@ -810,16 +828,19 @@ class CryptoBackendXmlSec1(CryptoBackend):
         logger.debug("Decrypt input len: %d" % len(enctext))
         _, fil = make_temp("%s" % enctext, decode=False)
 
-        com_list = [self.xmlsec, "--decrypt", "--privkey-pem",
-                    key_file, "--id-attr:%s" % ID_ATTR, ENC_KEY_CLASS]
+        com_list = [self.xmlsec, "--decrypt", "--id-attr:%s" % ID_ATTR,
+                    ENC_KEY_CLASS]
 
         (_stdout, _stderr, output) = self._run_xmlsec(com_list, [fil],
                                                       exception=DecryptError,
-                                                      validate_output=False)
+                                                      validate_output=False,
+                                                      key_file=key_file,
+                                                      passphrase=passphrase)
+
         return output
 
     def sign_statement(self, statement, node_name, key_file, node_id,
-                       id_attr):
+                       id_attr, passphrase=None):
         """
         Sign an XML statement.
 
@@ -832,18 +853,18 @@ class CryptoBackendXmlSec1(CryptoBackend):
         :return: The signed statement
         """
 
-        _, fil = make_temp("%s" % statement, suffix=".xml", decode=False, 
+        _, fil = make_temp("%s" % statement, suffix=".xml", decode=False,
                            delete=self._xmlsec_delete_tmpfiles)
 
         com_list = [self.xmlsec, "--sign",
-                    "--privkey-pem", key_file,
                     "--id-attr:%s" % id_attr, node_name]
         if node_id:
             com_list.extend(["--node-id", node_id])
 
         try:
             (stdout, stderr, signed_statement) = self._run_xmlsec(
-                com_list, [fil], validate_output=False)
+                com_list, [fil], validate_output=False, key_file=key_file,
+                passphrase=passphrase)
             # this doesn't work if --store-signatures are used
             if stdout == "":
                 if signed_statement:
@@ -898,7 +919,9 @@ class CryptoBackendXmlSec1(CryptoBackend):
         return parse_xmlsec_output(stderr)
 
     def _run_xmlsec(self, com_list, extra_args, validate_output=True,
-                    exception=XmlsecError):
+                    exception=XmlsecError,
+                    key_file=None,
+                    passphrase=None):
         """
         Common code to invoke xmlsec and parse the output.
         :param com_list: Key-value parameter list for xmlsec
@@ -910,13 +933,40 @@ class CryptoBackendXmlSec1(CryptoBackend):
         """
         ntf = NamedTemporaryFile(suffix=".xml",
                                  delete=self._xmlsec_delete_tmpfiles)
+
         com_list.extend(["--output", ntf.name])
+
+        # Unfortunately there's no safe way to pass a password to xmlsec1.
+        # Instead, we'll decrypt the certificate and write it into a named pipe,
+        # which we'll pass to xmlsec1.
+        named_pipe = None
+        if key_file is not None:
+            if passphrase is not None:
+                named_pipe = NamedPipe()
+
+                # Decrypt the certificate, but don't write it into the FIFO
+                # until after we've started xmlsec1.
+                with open(key_file) as f:
+                    key = importKey(f.read(), passphrase=passphrase)
+
+                key_file = named_pipe.name
+
+            com_list.extend(["--privkey-pem", key_file])
+
         com_list += extra_args
 
         logger.debug("xmlsec command: %s" % " ".join(com_list))
 
         pof = Popen(com_list, stderr=PIPE, stdout=PIPE)
 
+        if named_pipe is not None:
+            # Finally, write the key into our named pipe.
+            try:
+                with open(named_pipe.name, 'wb') as f:
+                    f.write(key.exportKey())
+            finally:
+                named_pipe.close()
+
         p_out = pof.stdout.read()
         p_err = pof.stderr.read()
 
@@ -958,7 +1008,7 @@ class CryptoBackendXMLSecurity(CryptoBackend):
         return "XMLSecurity 0.0"
 
     def sign_statement(self, statement, node_name, key_file, node_id,
-                       _id_attr):
+                       _id_attr, passphrase=None):
         """
         Sign an XML statement.
 
@@ -974,6 +1024,8 @@ class CryptoBackendXMLSecurity(CryptoBackend):
         import xmlsec
         import lxml.etree
 
+        assert passphrase is None, "Encrypted key files is not supported"
+
         xml = xmlsec.parse_xml(statement)
         signed = xmlsec.sign(xml, key_file)
         return lxml.etree.tostring(signed, xml_declaration=True)
@@ -1055,7 +1107,8 @@ def security_context(conf, debug=None):
         generate_cert_info=conf.generate_cert_info,
         tmp_cert_file=conf.tmp_cert_file,
         tmp_key_file=conf.tmp_key_file,
-        validate_certificate=conf.validate_certificate)
+        validate_certificate=conf.validate_certificate,
+        key_file_passphrase=conf.key_file_passphrase)
 
 
 def encrypt_cert_from_item(item):
@@ -1218,13 +1271,15 @@ class SecurityContext(object):
                  debug=False, template="", encrypt_key_type="des-192",
                  only_use_keys_in_metadata=False, cert_handler_extra_class=None,
                  generate_cert_info=None, tmp_cert_file=None,
-                 tmp_key_file=None, validate_certificate=None):
+                 tmp_key_file=None, validate_certificate=None,
+                 key_file_passphrase=None):
 
         self.crypto = crypto
         assert (isinstance(self.crypto, CryptoBackend))
 
         # Your private key
         self.key_file = key_file
+        self.key_file_passphrase = key_file_passphrase
         self.key_type = key_type
 
         # Your public key
@@ -1293,15 +1348,19 @@ class SecurityContext(object):
         """
         raise NotImplemented()
 
-    def decrypt(self, enctext, key_file=None):
+    def decrypt(self, enctext, key_file=None, passphrase=None):
         """ Decrypting an encrypted text by the use of a private key.
 
         :param enctext: The encrypted text as a string
         :return: The decrypted text
         """
-        if key_file is not None and len(key_file.strip()) > 0:
-            return self.crypto.decrypt(enctext, key_file)
-        return self.crypto.decrypt(enctext, self.key_file)
+        if key_file is None or len(key_file.strip()) == 0:
+            key_file = self.key_file
+
+        if passphrase is None:
+            passphrase = self.key_file_passphrase
+
+        return self.crypto.decrypt(enctext, key_file, passphrase)
 
     def verify_signature(self, signedtext, cert_file=None, cert_type="pem",
                          node_name=NODE_NAME, node_id=None, id_attr=""):
@@ -1619,7 +1678,8 @@ class SecurityContext(object):
         return self.sign_statement(statement, **kwargs)
 
     def sign_statement(self, statement, node_name, key=None,
-                       key_file=None, node_id=None, id_attr=""):
+                       key_file=None, node_id=None, id_attr="",
+                       passphrase=None):
         """Sign a SAML statement.
 
         :param statement: The statement to be signed
@@ -1640,8 +1700,12 @@ class SecurityContext(object):
         if not key and not key_file:
             key_file = self.key_file
 
+        if not passphrase:
+            passphrase = self.key_file_passphrase
+
         return self.crypto.sign_statement(statement, node_name, key_file,
-                                          node_id, id_attr)
+                                          node_id, id_attr,
+                                          passphrase=passphrase)
 
     def sign_assertion_using_xmlsec(self, statement, **kwargs):
         """ Deprecated function. See sign_assertion(). """
@@ -1674,7 +1738,8 @@ class SecurityContext(object):
         return self.sign_statement(statement, class_name(
             samlp.AttributeQuery()), **kwargs)
 
-    def multiple_signatures(self, statement, to_sign, key=None, key_file=None):
+    def multiple_signatures(self, statement, to_sign, key=None, key_file=None,
+                            passphrase=None):
         """
         Sign multiple parts of a statement
 
@@ -1697,7 +1762,8 @@ class SecurityContext(object):
 
             statement = self.sign_statement(statement, class_name(item),
                                             key=key, key_file=key_file,
-                                            node_id=sid, id_attr=id_attr)
+                                            node_id=sid, id_attr=id_attr,
+                                            passphrase=passphrase)
         return statement
 
 

+ 22 - 1
desktop/libs/libsaml/src/libsaml/conf.py

@@ -21,7 +21,7 @@ import subprocess
 
 from django.utils.translation import ugettext_lazy as _t, ugettext as _
 
-from desktop.lib.conf import Config, coerce_bool, coerce_csv
+from desktop.lib.conf import Config, coerce_bool, coerce_csv, coerce_password_from_script
 
 
 BASEDIR = os.path.dirname(os.path.abspath(__file__))
@@ -113,6 +113,17 @@ KEY_FILE = Config(
   type=str,
   help=_t("key_file is the name of a PEM formatted file that contains the private key of the Hue service. This is presently used both to encrypt/sign assertions and as client key in a HTTPS session."))
 
+KEY_FILE_PASSWORD = Config(
+  key="key_file_password",
+  help=_t("key_file_password password of the private key"),
+  default=None)
+
+KEY_FILE_PASSWORD_SCRIPT = Config(
+  key="key_file_password_script",
+  help=_t("Execute this script to produce the private key password. This will be used when `key_file_password` is not set."),
+  type=coerce_password_from_script,
+  default=None)
+
 CERT_FILE = Config(
   key="cert_file",
   default="",
@@ -155,6 +166,16 @@ NAME_ID_FORMAT = Config(
   type=str,
   help=_t("Request this NameID format from the server"))
 
+def get_key_file_password():
+  password = os.environ.get('HUE_SAML_KEY_FILE_PASSWORD')
+  if password is not None:
+    return password
+
+  password = KEY_FILE_PASSWORD.get()
+  if not password:
+    password = KEY_FILE_PASSWORD_SCRIPT.get()
+
+  return password
 
 def config_validator(user):
   res = []

+ 1 - 0
desktop/libs/libsaml/src/libsaml/saml_settings.py

@@ -93,6 +93,7 @@ def config_settings_loader(request):
 
     # certificate
     'key_file': libsaml.conf.KEY_FILE.get(),
+    'key_file_passphrase': libsaml.conf.get_key_file_password(),
     'cert_file': libsaml.conf.CERT_FILE.get()
   })