fix: ssl_certificate no longer depends on HttpdConfTree (#4220)

- The combiner and parser cannot handle special characters like 'é',
  so remove the dependency on it first
- Jira: RHINENG-12609

Signed-off-by: Huanhuan Li <huali@redhat.com>

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

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('<VirtualHost'):
+                    virtual_host_start = True
+                    continue
+                if virtual_host_start:
+                    if line.startswith('NSSEngine'):
+                        nss_engine = line.split()[-1].lower().strip('"\'')
+                    elif line.startswith('NSSCertificateDatabase'):
+                        nss_database = line.split()[-1].lower().strip('"\'')
+                    elif line.startswith('NSSNickname'):
+                        cert_name = line.split()[-1].lower().strip('"\'')
+                    elif line.startswith('</VirtualHost>'):
+                        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('<VirtualHost'):
+                    virtual_host_start = True
+                    continue
+                if virtual_host_start:
+                    if line.startswith('SSLEngine'):
+                        ssl_engine = line.split()[-1].lower().strip('"\'')
+                    elif line.startswith('SSLCertificateFile'):
+                        ssl_cert = line.strip().split()[-1].strip('"\'')
+                    elif line.startswith('</VirtualHost>'):
+                        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
 
 

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 @@
 <VirtualHost *:443>
   ## 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 @@
 <VirtualHost _default_:8443>
 ServerName www.examplea.com:8443
 NSSEngine on
+# NSSCertificateDatabase old_path
 NSSCertificateDatabase /etc/httpd/aliasa
-NSSNickname testcerta
+# bellow is the NSSNickname configuration
+NSSNickname testcertaê
 </VirtualHost>
 <VirtualHost :8443>
 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)