Merge lp:~joeborg/charm-helpers/charm-helpers into lp:charm-helpers

[Joseph Borg]

[lp:~joeborg/charm-helpers/charm-helpers]

719. By Joseph Borg on 2017-03-14

Adding checks on Openstack specific files when in hardening mode

[Edward Hope-Morley (hopem)]

Hi Joseph, thank you for submitting a patch to charm-helpers. In general your patch looks good and unit tests are passing. I do however have one comment that i'd like you to consider;

The os module is not intended to be used for the hardening of non-system applications. I think it would be wiser, particularly given the potential scope, to have an openstack module that performs hardening of Openstack-specific resources. Alternatively, we could add support to the os module to perform perm ops on arbitrary files but that seems less clean. I'll leave it to you to decide.

review: Needs Information

[Joseph Borg (joeborg)]

I think that's a wise idea. I'll move it and push again.
Thanks

[lp:~joeborg/charm-helpers/charm-helpers]

720. By Joseph Borg on 2017-03-15

Moving Openstack hardening audits into own module

721. By Joseph Borg on 2017-03-17

Hardening Apache ciphers

[Ante Karamatić (ivoks)]

review: Needs Fixing

[Ante Karamatić (ivoks)]

review: Approve

[Edward Hope-Morley (hopem)]

Hi Joseph, can you please add a slightly more descriptive commit message. Also you seem to have unintentionally removed traceenable from the context. With regards to my previous comment on adding a new module for openstack hardening, i meant actually adding support for an altogether new layer to the stack i.e. right now we have os/apache/ssh/mysql and I'm suggesting that we add an openstack layer. This is a somewhat non-trivial change to make but if in the long run we are planning to add more openstack-specific hardening then we should evaluate doing this. If that is not the route you want to go down then I suggest you look into finding a way to make these changes purely through config such that we can avoid having subsystem-specific code in the host module.

review: Needs Fixing

[Joseph Borg (joeborg)]

Hi Edward,

It was removed intentionally as it appears to be a completely redundant (and misleading) statement, please correct me if I'm wrong.

Forgive me for being a bit dense, but I don't understand the point about a new layer. I've created a new module (openstack.py) and moved the code into there. Are you talking about breaking all of the modules inside hardening up? I've not got much experience with this library (only about a month), so please forgive any ignorance my end.

[Edward Hope-Morley (hopem)]

Hi Joseph, yep sure I see why you removed it now that's fine. Admittedly I am overloading the term module so apologies if it is confusing. What i mean is that in the harding library we have four hardening modules i.e. "os", "mysql", "apache" and "ssh" each of which has an accompanying set of code e.g. "os" is implemented in contrib.hardening.host, "ssh" in contrib.hardning.ssh etc. What I am suggesting is that we either bite the bullet now (if we think there will be enough requirements to warrant the extra work) and implement a new hardening module called "openstack" and add the bits you need to that (i.e. setting openstack config file permissions). Another possible alternative is that we add support to the "os" module for setting permissions on arbitrary files which could be provided through configuration. My reason for saying this is principally that contrib.hardening.host is not supposed to have project specific code in it. If this patch is time-sensitive then it probably makes more sense to add support for setting permissions on arbitrary files. I also wonder whether it woud be simpler to just fix this in the charms themselves. If you fix this in charmhelpers you are going to have to sync it into the charms anyway in order for it to be used so you could just fix it straight in the charms for which a problem has been identified.

[Ante Karamatić (ivoks)]

Hi Ed

Joe and I had a chat about this and what we will do is amend this MP. Since we will extend OpenStack hardening even further in future, I think you are right and that should be a separate module. Please don't close this MR, as we will confine it only to apache hardening fixes.

Thanks

[Joseph Borg (joeborg)]

Apache ciphers moved to https://code.launchpad.net/~joeborg/charm-helpers/apache-hardening/+merge/320328

Unmerged revisions

721. By Joseph Borg on 2017-03-17

Hardening Apache ciphers

720. By Joseph Borg on 2017-03-15

Moving Openstack hardening audits into own module

719. By Joseph Borg on 2017-03-14

Adding checks on Openstack specific files when in hardening mode

718. By Joseph Borg on 2017-03-14

Enforcing Apache welcome page is deleted when hardening is enabled

=== modified file 'charmhelpers/contrib/hardening/apache/checks/config.py'
--- charmhelpers/contrib/hardening/apache/checks/config.py	2016-07-06 14:41:05 +0000
+++ charmhelpers/contrib/hardening/apache/checks/config.py	2017-03-17 10:36:56 +0000
@@ -26,6 +26,7 @@
     DirectoryPermissionAudit,
     NoReadWriteForOther,
     TemplatedFile,
+    DeletedFile
 )
 from charmhelpers.contrib.hardening.audits.apache import DisabledModuleAudit
 from charmhelpers.contrib.hardening.apache import TEMPLATES_DIR
@@ -74,6 +75,8 @@
         DisabledModuleAudit(settings['hardening']['modules_to_disable']),
 
         NoReadWriteForOther(settings['common']['apache_dir']),
+
+        DeletedFile(['/var/www/html/index.html'])
     ]
 
     return audits
@@ -94,5 +97,5 @@
         ctxt['apache_version'] = re.search(r'.+version: Apache/(.+?)\s.+',
                                            out).group(1)
         ctxt['apache_icondir'] = '/usr/share/apache2/icons/'
-        ctxt['traceenable'] = settings['hardening']['traceenable']
+
         return ctxt

=== modified file 'charmhelpers/contrib/hardening/apache/templates/hardening.conf'
--- charmhelpers/contrib/hardening/apache/templates/hardening.conf	2016-03-14 12:39:21 +0000
+++ charmhelpers/contrib/hardening/apache/templates/hardening.conf	2017-03-17 10:36:56 +0000
@@ -16,3 +16,6 @@
 </Location>
 
 TraceEnable {{ traceenable }}
+
+SSLHonorCipherOrder {{ honor_cipher_order }}
+SSLCipherSuite {{ cipher_suite }}

=== modified file 'charmhelpers/contrib/hardening/defaults/apache.yaml'
--- charmhelpers/contrib/hardening/defaults/apache.yaml	2016-03-14 13:57:17 +0000
+++ charmhelpers/contrib/hardening/defaults/apache.yaml	2017-03-17 10:36:56 +0000
@@ -9,5 +9,7 @@
 
 hardening:
     traceenable: 'off'
-    allowed_http_methods: "GET POST"
-    modules_to_disable: [ cgi, cgid ]
\ No newline at end of file
+    allowed_http_methods: 'GET POST'
+    modules_to_disable: [ cgi, cgid ]
+    honor_cipher_order: 'on'
+    cipher_suite: 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'

=== modified file 'charmhelpers/contrib/hardening/defaults/apache.yaml.schema'
--- charmhelpers/contrib/hardening/defaults/apache.yaml.schema	2016-03-14 13:57:17 +0000
+++ charmhelpers/contrib/hardening/defaults/apache.yaml.schema	2017-03-17 10:36:56 +0000
@@ -7,3 +7,5 @@
 hardening:
     allowed_http_methods:
     modules_to_disable:
+    honor_cipher_order:
+    cipher_suite:

=== modified file 'charmhelpers/contrib/hardening/host/checks/__init__.py'
--- charmhelpers/contrib/hardening/host/checks/__init__.py	2016-07-06 14:41:05 +0000
+++ charmhelpers/contrib/hardening/host/checks/__init__.py	2017-03-17 10:36:56 +0000
@@ -21,6 +21,7 @@
     limits,
     login,
     minimize_access,
+    openstack,
     pam,
     profile,
     securetty,
@@ -35,6 +36,7 @@
     checks.extend(limits.get_audits())
     checks.extend(login.get_audits())
     checks.extend(minimize_access.get_audits())
+    checks.extend(openstack.get_audits())
     checks.extend(pam.get_audits())
     checks.extend(profile.get_audits())
     checks.extend(securetty.get_audits())

=== added file 'charmhelpers/contrib/hardening/host/checks/openstack.py'
--- charmhelpers/contrib/hardening/host/checks/openstack.py	1970-01-01 00:00:00 +0000
+++ charmhelpers/contrib/hardening/host/checks/openstack.py	2017-03-17 10:36:56 +0000
@@ -0,0 +1,30 @@
+# Copyright 2016 Canonical Limited.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from charmhelpers.contrib.hardening.audits.file import DirectoryPermissionAudit
+
+
+def get_audits():
+    """
+    Get OS hardening security limits audits.
+
+    :returns: List of audits
+    """
+    audits = [
+        DirectoryPermissionAudit('/etc/openstack-dashboard', user='horizon', group='horizon', mode=0o700),
+        DirectoryPermissionAudit('/etc/cinder', user='cinder', group='cinder', mode=0o700),
+        DirectoryPermissionAudit('/etc/glance', user='glance', group='glance', mode=0o700),
+        DirectoryPermissionAudit('/etc/heat', user='heat', group='heat', mode=0o700),
+        DirectoryPermissionAudit('/etc/ceilometer', user='ceilometer', group='ceilometer', mode=0o700)
+    ]
+    return audits

=== modified file 'tests/contrib/hardening/apache/checks/test_config.py'
--- tests/contrib/hardening/apache/checks/test_config.py	2016-07-06 14:41:05 +0000
+++ tests/contrib/hardening/apache/checks/test_config.py	2017-03-17 10:36:56 +0000
@@ -51,14 +51,16 @@
     @patch.object(config.subprocess, 'call', lambda *args, **kwargs: 0)
     def test_get_audits_apache_is_installed(self):
         audits = config.get_audits()
-        self.assertEqual(6, len(audits))
+        self.assertEqual(7, len(audits))
 
     @patch.object(config.utils, 'get_settings', lambda x: {
         'common': {'apache_dir': TEST_TMPDIR},
         'hardening': {
             'allowed_http_methods': {'GOGETEM'},
             'modules_to_disable': {'modfoo'},
-            'traceenable': 'off'
+            'traceenable': 'off',
+            'honor_cipher_order': 'on',
+            'cipher_suite': 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'
         }
     })
     @patch.object(config, 'subprocess')
@@ -77,4 +79,6 @@
                                       '/usr/share/apache2/icons/',
                                       'apache_version': '2.4.7',
                                       'modules_to_disable': set(['modfoo']),
-                                      'traceenable': 'off'})
+                                      'traceenable': 'off',
+                                      'honor_cipher_order': 'on',
+                                      'cipher_suite': 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'})

=== modified file 'tests/contrib/hardening/host/checks/test_limits.py'
--- tests/contrib/hardening/host/checks/test_limits.py	2016-07-06 14:41:05 +0000
+++ tests/contrib/hardening/host/checks/test_limits.py	2017-03-17 10:36:56 +0000
@@ -16,6 +16,7 @@
 
 from mock import patch
 
+from charmhelpers.contrib.hardening.audits import BaseAudit
 from charmhelpers.contrib.hardening.host.checks import limits
 
 
@@ -26,13 +27,8 @@
     def test_core_dump_disabled(self):
         audits = limits.get_audits()
         self.assertEqual(2, len(audits))
-        audit = audits[0]
-        self.assertTrue(isinstance(audit, limits.DirectoryPermissionAudit))
-        self.assertEqual('/etc/security/limits.d', audit.paths[0])
-        audit = audits[1]
-        self.assertTrue(isinstance(audit, limits.TemplatedFile))
-        self.assertEqual('/etc/security/limits.d/10.hardcore.conf',
-                         audit.paths[0])
+        for audit in audits:
+            self.assertTrue(isinstance(audit, BaseAudit))
 
     @patch.object(limits.utils, 'get_settings', lambda x: {
         'security': {'kernel_enable_core_dump': True}
@@ -40,4 +36,5 @@
     def test_core_dump_enabled(self):
         audits = limits.get_audits()
         self.assertEqual(1, len(audits))
-        self.assertTrue(isinstance(audits[0], limits.DirectoryPermissionAudit))
+        for audit in audits:
+            self.assertTrue(isinstance(audit, BaseAudit))

=== added file 'tests/contrib/hardening/host/checks/test_openstack.py'
--- tests/contrib/hardening/host/checks/test_openstack.py	1970-01-01 00:00:00 +0000
+++ tests/contrib/hardening/host/checks/test_openstack.py	2017-03-17 10:36:56 +0000
@@ -0,0 +1,24 @@
+# Copyright 2016 Canonical Limited.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from unittest import TestCase
+from charmhelpers.contrib.hardening.host.checks import openstack
+from charmhelpers.contrib.hardening.audits.file import DirectoryPermissionAudit
+
+
+class LimitsTestCase(TestCase):
+    def test_core_dump_disabled(self):
+        audits = openstack.get_audits()
+        for audit in audits:
+            self.assertTrue(isinstance(audit, DirectoryPermissionAudit))

=== modified file 'tests/contrib/hardening/test_templating.py'
--- tests/contrib/hardening/test_templating.py	2016-07-06 14:41:05 +0000
+++ tests/contrib/hardening/test_templating.py	2017-03-17 10:36:56 +0000
@@ -221,7 +221,9 @@
         'hardening': {
             'allowed_http_methods': {'GOGETEM'},
             'modules_to_disable': {'modfoo'},
-            'traceenable': 'off'
+            'traceenable': 'off',
+            'honor_cipher_order': 'on',
+            'cipher_suite': 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'
         }
     })
     @patch('charmhelpers.contrib.hardening.audits.file.os.path.exists',