From ff4fb0fbbdedc70807a2e4b475d22bf3b2f04683 Mon Sep 17 00:00:00 2001 From: Ernesto Puerta Date: Fri, 23 Aug 2019 13:45:45 +0200 Subject: [PATCH] mgr/dashboard: fix SAML input argument handling Currently dashboard provides a Ceph command to specify location or contents of SAML2 IdP XML (`idp_metadata` argument). This loose interface is implemented by trying to: - First, opens HTTPS connection to whatever that argument contains (it might be a proper remote URL, a local file or XML contents). - Then, tries to open the local file - Finally, assumes the input argument is an XML and proceeds to parse it. However, as the XML can have an undefined length, when fed as a filename it results in FreeBSD raising a OSError exception (`Max filename length exceeded`, 1K). This essentially means that this handling results in unexpected behaviour, as it pushes the validation & error handling to the underlying methods. In this fix, some preliminary validation is performed. Especifically: - Is the input argument a potential filename? - Is the input argument complying with URL syntax? Only if the above checks fail, the input argument is fed into the XML parser. Additionally, previous syntax is deprecated, so now, `idp_metadata` enforces 2 syntaxes: - Raw XML contents - URL specification (http, https, and file schemas accepted). For local file, URL 'file://' should be used instead. Fixes: https://tracker.ceph.com/issues/41358 Signed-off-by: Ernesto Puerta --- doc/mgr/dashboard.rst | 2 +- src/pybind/mgr/dashboard/services/sso.py | 69 +++++++++++------------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/doc/mgr/dashboard.rst b/doc/mgr/dashboard.rst index ab080c69af6..b1508279b03 100644 --- a/doc/mgr/dashboard.rst +++ b/doc/mgr/dashboard.rst @@ -451,7 +451,7 @@ To configure SSO on Ceph Dashboard, you should use the following command:: Parameters: * ****: Base URL where Ceph Dashboard is accessible (e.g., `https://cephdashboard.local`) -* ****: URL, file path or content of the IdP metadata XML (e.g., `https://myidp/metadata`) +* ****: URL to remote (`http://`, `https://`) or local (`file://`) path or content of the IdP metadata XML (e.g., `https://myidp/metadata`, `file:///home/myuser/metadata.xml`). * **** *(optional)*: Attribute that should be used to get the username from the authentication response. Defaults to `uid`. * **** *(optional)*: Use this when more than one entity id exists on the IdP metadata. * ** / ** *(optional)*: File path or content of the certificate that should be used by Ceph Dashboard (Service Provider) for signing and encryption. diff --git a/src/pybind/mgr/dashboard/services/sso.py b/src/pybind/mgr/dashboard/services/sso.py index 03c73c4f6a9..7cfe4e3f1da 100644 --- a/src/pybind/mgr/dashboard/services/sso.py +++ b/src/pybind/mgr/dashboard/services/sso.py @@ -2,18 +2,22 @@ # pylint: disable=too-many-return-statements,too-many-branches from __future__ import absolute_import +import os import errno import json import threading +import warnings import six if six.PY2: FileNotFoundError = IOError +from six.moves.urllib import parse + try: - from onelogin.saml2.settings import OneLogin_Saml2_Settings - from onelogin.saml2.errors import OneLogin_Saml2_Error - from onelogin.saml2.idp_metadata_parser import OneLogin_Saml2_IdPMetadataParser + from onelogin.saml2.settings import OneLogin_Saml2_Settings as Saml2Settings + from onelogin.saml2.errors import OneLogin_Saml2_Error as Saml2Error + from onelogin.saml2.idp_metadata_parser import OneLogin_Saml2_IdPMetadataParser as Saml2Parser python_saml_imported = True except ImportError: @@ -147,8 +151,8 @@ def handle_sso_command(cmd): if cmd['prefix'] == 'dashboard sso enable saml2': try: - OneLogin_Saml2_Settings(mgr.SSO_DB.saml2.onelogin_settings) - except OneLogin_Saml2_Error: + Saml2Settings(mgr.SSO_DB.saml2.onelogin_settings) + except Saml2Error: return -errno.EPERM, '', 'Single Sign-On is not configured: ' \ 'use `ceph dashboard sso setup saml2`' mgr.SSO_DB.protocol = 'saml2' @@ -174,43 +178,34 @@ def handle_sso_command(cmd): idp_metadata = cmd['idp_metadata'] idp_username_attribute = _get_optional_attr(cmd, 'idp_username_attribute', 'uid') idp_entity_id = _get_optional_attr(cmd, 'idp_entity_id', None) - sp_x_509_cert = _get_optional_attr(cmd, 'sp_x_509_cert', '') - sp_private_key = _get_optional_attr(cmd, 'sp_private_key', '') - if sp_x_509_cert and not sp_private_key: + sp_x_509_cert_path = _get_optional_attr(cmd, 'sp_x_509_cert', '') + sp_private_key_path = _get_optional_attr(cmd, 'sp_private_key', '') + if sp_x_509_cert_path and not sp_private_key_path: return -errno.EINVAL, '', 'Missing parameter `sp_private_key`.' - if not sp_x_509_cert and sp_private_key: + if not sp_x_509_cert_path and sp_private_key_path: return -errno.EINVAL, '', 'Missing parameter `sp_x_509_cert`.' - has_sp_cert = sp_x_509_cert != "" and sp_private_key != "" + has_sp_cert = sp_x_509_cert_path != "" and sp_private_key_path != "" try: - f = open(sp_x_509_cert, 'r') - sp_x_509_cert = f.read() - f.close() + with open(sp_x_509_cert_path, 'r') as f: + sp_x_509_cert = f.read() except FileNotFoundError: - pass + sp_x_509_cert = '' try: - f = open(sp_private_key, 'r') - sp_private_key = f.read() - f.close() + with open(sp_private_key_path, 'r') as f: + sp_private_key = f.read() except FileNotFoundError: - pass - try: - idp_settings = OneLogin_Saml2_IdPMetadataParser.parse_remote(idp_metadata, - validate_cert=False, - entity_id=idp_entity_id) - # pylint: disable=broad-except - except Exception: - try: - f = open(idp_metadata, 'r') - idp_metadata = f.read() - f.close() - except FileNotFoundError: - pass - try: - idp_settings = OneLogin_Saml2_IdPMetadataParser.parse(idp_metadata, - entity_id=idp_entity_id) - # pylint: disable=broad-except - except Exception: - return -errno.EINVAL, '', 'Invalid parameter `idp_metadata`.' + sp_private_key = '' + + if os.path.isfile(idp_metadata): + warnings.warn_explicit( + "Please prepend 'file://' to indicate a local SAML2 IdP file", DeprecationWarning) + with open(idp_metadata, 'r') as f: + idp_settings = Saml2Parser.parse(f.read(), entity_id=idp_entity_id) + elif parse.urlparse(idp_metadata)[0] in ('http', 'https', 'file'): + idp_settings = Saml2Parser.parse_remote( + url=idp_metadata, validate_cert=False, entity_id=idp_entity_id) + else: + idp_settings = Saml2Parser.parse(idp_metadata, entity_id=idp_entity_id) url_prefix = prepare_url_prefix(mgr.get_module_option('url_prefix', default='')) settings = { @@ -251,7 +246,7 @@ def handle_sso_command(cmd): "wantAttributeStatement": False } } - settings = OneLogin_Saml2_IdPMetadataParser.merge_settings(settings, idp_settings) + settings = Saml2Parser.merge_settings(settings, idp_settings) mgr.SSO_DB.saml2.onelogin_settings = settings mgr.SSO_DB.protocol = 'saml2' mgr.SSO_DB.save() -- 2.39.5