From 3725e1aace388a16c084a30315ff124f37a5ee77 Mon Sep 17 00:00:00 2001 From: huali027 <44796653+huali027@users.noreply.github.com> Date: Thu, 26 Sep 2024 15:19:48 +0800 Subject: [PATCH] fix: ssl_certificate no longer depends on HttpdConfTree (#4220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The combiner and parser cannot handle special characters like 'é', so remove the dependency on it first - Jira: RHINENG-12609 Signed-off-by: Huanhuan Li --- insights/collect.py | 7 -- insights/specs/datasources/ssl_certificate.py | 72 +++++++++++----- .../tests/datasources/test_ssl_certificate.py | 83 ++++++++++--------- 3 files changed, 94 insertions(+), 68 deletions(-) diff --git a/insights/collect.py b/insights/collect.py index df3ae49498..3b7f158d13 100755 --- a/insights/collect.py +++ b/insights/collect.py @@ -167,13 +167,6 @@ - name: insights.combiners.ps enabled: true - # needed for httpd_certificate - - name: insights.combiners.httpd_conf.HttpdConfTree - enabled: true - - - name: insights.parsers.httpd_conf.HttpdConf - enabled: true - # needed for httpd_on_nfs - name: insights.parsers.mount.ProcMounts enabled: true diff --git a/insights/specs/datasources/ssl_certificate.py b/insights/specs/datasources/ssl_certificate.py index 308bea082e..431d34177b 100644 --- a/insights/specs/datasources/ssl_certificate.py +++ b/insights/specs/datasources/ssl_certificate.py @@ -1,16 +1,17 @@ """ Custom datasource to get ssl certificate file path. """ -from insights.combiners.httpd_conf import HttpdConfTree + from insights.combiners.nginx_conf import NginxConfTree +from insights.combiners.rsyslog_confs import RsyslogAllConf from insights.core.context import HostContext from insights.core.exceptions import SkipComponent from insights.core.plugins import datasource from insights.parsers.mssql_conf import MsSQLConf -from insights.combiners.rsyslog_confs import RsyslogAllConf +from insights.specs.datasources import httpd -@datasource(HttpdConfTree, HostContext) +@datasource(httpd.httpd_configuration_files, HostContext) def httpd_certificate_info_in_nss(broker): """ Get the certificate info configured in nss database @@ -25,22 +26,35 @@ def httpd_certificate_info_in_nss(broker): SkipComponent: Raised when NSSEngine isn't enabled or "NSSCertificateDatabase" and "NSSNickname" directives aren't found """ - conf = broker[HttpdConfTree] + confs = broker[httpd.httpd_configuration_files] path_pairs = [] - virtual_hosts = conf.find('VirtualHost') - for host in virtual_hosts: - nss_engine = nss_database = cert_name = None - nss_engine = host.select('NSSEngine') - nss_database = host.select('NSSCertificateDatabase') - cert_name = host.select('NSSNickname') - if nss_engine and nss_engine.value and nss_database and cert_name: - path_pairs.append((nss_database[0].value, cert_name[0].value)) + nss_engine = nss_database = cert_name = None + virtual_host_start = False + for conf in confs: + with open(conf) as a_f: + for line in a_f.readlines(): + line = line.strip() + if line.startswith(''): + if nss_engine == 'on' and nss_database and cert_name: + path_pairs.append((nss_database, cert_name)) + virtual_host_start = False + nss_engine = nss_database = cert_name = None if path_pairs: return path_pairs raise SkipComponent -@datasource(HttpdConfTree, HostContext) +@datasource(httpd.httpd_configuration_files, HostContext) def httpd_ssl_certificate_files(broker): """ Get the httpd SSL certificate file path configured by "SSLCertificateFile" @@ -54,17 +68,29 @@ def httpd_ssl_certificate_files(broker): Raises: SkipComponent: Raised if "SSLCertificateFile" directive isn't found """ - conf = broker[HttpdConfTree] - virtual_hosts = conf.find('VirtualHost') - ssl_certs = [] - for host in virtual_hosts: - ssl_cert = ssl_engine = None - ssl_engine = host.select('SSLEngine') - ssl_cert = host.select('SSLCertificateFile') - if ssl_engine and ssl_engine.value and ssl_cert: - ssl_certs.append(str(ssl_cert.value)) + confs = broker[httpd.httpd_configuration_files] + ssl_engine = ssl_cert = None + ssl_certs = set() + virtual_host_start = False + for conf in confs: + with open(conf) as a_f: + for line in a_f.readlines(): + line = line.strip() + if line.startswith(''): + if ssl_engine == 'on' and ssl_cert: + ssl_certs.add(ssl_cert) + virtual_host_start = False + ssl_engine = ssl_cert = None if ssl_certs: - return ssl_certs + return sorted(ssl_certs) raise SkipComponent diff --git a/insights/tests/datasources/test_ssl_certificate.py b/insights/tests/datasources/test_ssl_certificate.py index 7652a3e549..15bfe4549a 100644 --- a/insights/tests/datasources/test_ssl_certificate.py +++ b/insights/tests/datasources/test_ssl_certificate.py @@ -1,13 +1,20 @@ +# -*- coding: utf-8 -*- + import pytest +try: + from unittest.mock import patch, mock_open + builtin_open = "builtins.open" +except Exception: + from mock import patch, mock_open + builtin_open = "__builtin__.open" -from insights.combiners.httpd_conf import HttpdConfTree from insights.combiners.nginx_conf import NginxConfTree +from insights.combiners.rsyslog_confs import RsyslogAllConf from insights.core.exceptions import SkipComponent -from insights.parsers.httpd_conf import HttpdConf from insights.parsers.mssql_conf import MsSQLConf from insights.parsers.nginx_conf import NginxConfPEG from insights.parsers.rsyslog_conf import RsyslogConf -from insights.combiners.rsyslog_confs import RsyslogAllConf +from insights.specs.datasources import httpd from insights.specs.datasources.ssl_certificate import ( httpd_ssl_certificate_files, nginx_ssl_certificate_files, mssql_tls_cert_file, httpd_certificate_info_in_nss, @@ -26,7 +33,7 @@ ## SSL directives SSLEngine on - SSLCertificateFile "/etc/pki/katello/certs/katello-apache.crt" + SSLCertificateFile "/etc/pki/katello/certs/gént-katello-apache.crt" SSLCertificateKeyFile "/etc/pki/katello/private/katello-apache.key" SSLCertificateChainFile "/etc/pki/katello/certs/katello-server-ca.crt" SSLVerifyClient optional @@ -41,6 +48,7 @@ ## SSL directives ServerName a.b.c.com SSLEngine on + # SSLCertificateFile "/etc/pki/katello/certs/old_katello-apache.crt" SSLCertificateFile "/etc/pki/katello/certs/katello-apache.crt" SSLCertificateKeyFile "/etc/pki/katello/private/katello-apache.key" SSLCertificateChainFile "/etc/pki/katello/certs/katello-server-ca.crt" @@ -53,6 +61,7 @@ ## SSL directives ServerName d.c.e.com SSLEngine on + # bellow is SSLCertificateFile configuration SSLCertificateFile "/etc/pki/katello/certs/katello-apache_d.crt" SSLCertificateKeyFile "/etc/pki/katello/private/katello-apache_d.key" SSLCertificateChainFile "/etc/pki/katello/certs/katello-server-ca_d.crt" @@ -162,8 +171,10 @@ ServerName www.examplea.com:8443 NSSEngine on +# NSSCertificateDatabase old_path NSSCertificateDatabase /etc/httpd/aliasa -NSSNickname testcerta +# bellow is the NSSNickname configuration +NSSNickname testcertaê ServerName www.exampleb.com:8443 @@ -304,23 +315,19 @@ """ -def test_httpd_certificate(): - conf1 = HttpdConf(context_wrap(HTTPD_CONF, path='/etc/httpd/conf/httpd.conf')) - conf2 = HttpdConf(context_wrap(HTTPD_SSL_CONF, path='/etc/httpd/conf.d/ssl.conf')) - conf_tree = HttpdConfTree([conf1, conf2]) - +@patch("os.path.exists", return_value=True) +@patch(builtin_open, new_callable=mock_open, read_data=HTTPD_CONF) +def test_httpd_certificate(m_open, m_exist): + m_open.side_effect = [m_open.return_value, mock_open(read_data=HTTPD_SSL_CONF).return_value] broker = { - HttpdConfTree: conf_tree + httpd.httpd_configuration_files: {'/etc/httpd/conf/httpd.conf', '/etc/httpd/conf.d/ssl.conf'} } result = httpd_ssl_certificate_files(broker) - assert result == ['/etc/pki/katello/certs/katello-apache.crt'] - - conf1 = HttpdConf(context_wrap(HTTPD_CONF, path='/etc/httpd/conf/httpd.conf')) - conf2 = HttpdConf(context_wrap(HTTPD_SSL_CONF_2, path='/etc/httpd/conf.d/ssl.conf')) - conf_tree = HttpdConfTree([conf1, conf2]) + assert result == ['/etc/pki/katello/certs/gént-katello-apache.crt'] + m_open.side_effect = [m_open.return_value, mock_open(read_data=HTTPD_SSL_CONF_2).return_value] broker = { - HttpdConfTree: conf_tree + httpd.httpd_configuration_files: {'/etc/httpd/conf/httpd.conf', '/etc/httpd/conf.d/ssl.conf'} } result = httpd_ssl_certificate_files(broker) # "/etc/pki/katello/certs/katello-apache_e.crt" not in the result @@ -349,21 +356,21 @@ def test_nginx_certificate(): assert result == ['/a/b/www.example.com.crt', '/a/b/www.example.com.cecdsa.crt', '/a/b/www.example.org.crt'] -def test_httpd_ssl_cert_exception(): - conf1 = HttpdConf(context_wrap(HTTPD_CONF, path='/etc/httpd/conf/httpd.conf')) - conf2 = HttpdConf(context_wrap(HTTPD_CONF_WITHOUT_SSL, path='/etc/httpd/conf.d/no_ssl.conf')) - conf_tree = HttpdConfTree([conf1, conf2]) +@patch("os.path.exists", return_value=True) +@patch(builtin_open, new_callable=mock_open, read_data=HTTPD_CONF) +def test_httpd_ssl_cert_exception(m_open, m_exists): + m_open.side_effect = [m_open.return_value, mock_open(read_data=HTTPD_CONF_WITHOUT_SSL).return_value] broker1 = { - HttpdConfTree: conf_tree + httpd.httpd_configuration_files: {'/etc/httpd/conf/httpd.conf', '/etc/httpd/conf.d/no_ssl.conf'} } - conf1 = HttpdConf(context_wrap(HTTPD_CONF, path='/etc/httpd/conf/httpd.conf')) - conf2 = HttpdConf(context_wrap(HTTPD_SSL_CONF_NO_VALUE, path='/etc/httpd/conf.d/no_ssl.conf')) - conf_tree = HttpdConfTree([conf1, conf2]) + with pytest.raises(SkipComponent): + httpd_ssl_certificate_files(broker1) + + m_open.side_effect = [m_open.return_value, mock_open(read_data=HTTPD_SSL_CONF_NO_VALUE).return_value] broker2 = { - HttpdConfTree: conf_tree + httpd.httpd_configuration_files: {'/etc/httpd/conf/httpd.conf', '/etc/httpd/conf.d/no_ssl.conf'} } with pytest.raises(SkipComponent): - httpd_ssl_certificate_files(broker1) httpd_ssl_certificate_files(broker2) @@ -396,23 +403,23 @@ def test_mssql_tls_no_cert_exception(): mssql_tls_cert_file(broker1) -def test_httpd_certificate_info_in_nss(): - conf1 = HttpdConf(context_wrap(HTTPD_CONF, path='/etc/httpd/conf/httpd.conf')) - conf2 = HttpdConf(context_wrap(HTTPD_WITH_NSS, path='/etc/httpd/conf.d/nss.conf')) - conf_tree = HttpdConfTree([conf1, conf2]) +@patch("os.path.exists", return_value=True) +@patch(builtin_open, new_callable=mock_open, read_data=HTTPD_CONF) +def test_httpd_certificate_info_in_nss(m_open, m_exists): + m_open.side_effect = [m_open.return_value, mock_open(read_data=HTTPD_WITH_NSS).return_value] broker = { - HttpdConfTree: conf_tree + httpd.httpd_configuration_files: {'/etc/httpd/conf/httpd.conf', '/etc/httpd/conf.d/nss.conf'} } result = httpd_certificate_info_in_nss(broker) - assert result == [('/etc/httpd/aliasa', 'testcerta'), ('/etc/httpd/aliasb', 'testcertb')] + assert result == [('/etc/httpd/aliasa', 'testcertaê'), ('/etc/httpd/aliasb', 'testcertb')] -def test_httpd_certificate_info_in_nss_exception(): - conf1 = HttpdConf(context_wrap(HTTPD_CONF, path='/etc/httpd/conf/httpd.conf')) - conf2 = HttpdConf(context_wrap(HTTPD_WITH_NSS_OFF, path='/etc/httpd/conf.d/nss.conf')) - conf_tree = HttpdConfTree([conf1, conf2]) +@patch("os.path.exists", return_value=True) +@patch(builtin_open, new_callable=mock_open, read_data=HTTPD_CONF) +def test_httpd_certificate_info_in_nss_exception(m_open, m_exists): + m_open.side_effect = [m_open.return_value, mock_open(read_data=HTTPD_WITH_NSS_OFF).return_value] broker = { - HttpdConfTree: conf_tree + httpd.httpd_configuration_files: {'/etc/httpd/conf/httpd.conf', '/etc/httpd/conf.d/nss.conf'} } with pytest.raises(SkipComponent): httpd_certificate_info_in_nss(broker)