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

Proposed by Joseph Borg
Status: Needs review
Proposed branch: lp:~joeborg/charm-helpers/charm-helpers
Merge into: lp:charm-helpers
Diff against target: 228 lines (+84/-15)
10 files modified
charmhelpers/contrib/hardening/apache/checks/config.py (+4/-1)
charmhelpers/contrib/hardening/apache/templates/hardening.conf (+3/-0)
charmhelpers/contrib/hardening/defaults/apache.yaml (+4/-2)
charmhelpers/contrib/hardening/defaults/apache.yaml.schema (+2/-0)
charmhelpers/contrib/hardening/host/checks/__init__.py (+2/-0)
charmhelpers/contrib/hardening/host/checks/openstack.py (+30/-0)
tests/contrib/hardening/apache/checks/test_config.py (+7/-3)
tests/contrib/hardening/host/checks/test_limits.py (+5/-8)
tests/contrib/hardening/host/checks/test_openstack.py (+24/-0)
tests/contrib/hardening/test_templating.py (+3/-1)
To merge this branch: bzr merge lp:~joeborg/charm-helpers/charm-helpers
Reviewer Review Type Date Requested Status
Edward Hope-Morley Needs Fixing
Ante Karamatić (community) Approve
Review via email: mp+319830@code.launchpad.net
To post a comment you must log in.
719. By Joseph Borg

Adding checks on Openstack specific files when in hardening mode

Revision history for this message
Edward Hope-Morley (hopem) wrote :

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
Revision history for this message
Joseph Borg (joeborg) wrote :

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

720. By Joseph Borg

Moving Openstack hardening audits into own module

721. By Joseph Borg

Hardening Apache ciphers

Revision history for this message
Ante Karamatić (ivoks) :
review: Needs Fixing
Revision history for this message
Ante Karamatić (ivoks) :
review: Approve
Revision history for this message
Edward Hope-Morley (hopem) wrote :

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
Revision history for this message
Joseph Borg (joeborg) wrote :

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.

Revision history for this message
Edward Hope-Morley (hopem) wrote :

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.

Revision history for this message
Ante Karamatić (ivoks) wrote :

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

Revision history for this message
Joseph Borg (joeborg) wrote :

Unmerged revisions

721. By Joseph Borg

Hardening Apache ciphers

720. By Joseph Borg

Moving Openstack hardening audits into own module

719. By Joseph Borg

Adding checks on Openstack specific files when in hardening mode

718. By Joseph Borg

Enforcing Apache welcome page is deleted when hardening is enabled

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/hardening/apache/checks/config.py'
2--- charmhelpers/contrib/hardening/apache/checks/config.py 2016-07-06 14:41:05 +0000
3+++ charmhelpers/contrib/hardening/apache/checks/config.py 2017-03-17 10:36:56 +0000
4@@ -26,6 +26,7 @@
5 DirectoryPermissionAudit,
6 NoReadWriteForOther,
7 TemplatedFile,
8+ DeletedFile
9 )
10 from charmhelpers.contrib.hardening.audits.apache import DisabledModuleAudit
11 from charmhelpers.contrib.hardening.apache import TEMPLATES_DIR
12@@ -74,6 +75,8 @@
13 DisabledModuleAudit(settings['hardening']['modules_to_disable']),
14
15 NoReadWriteForOther(settings['common']['apache_dir']),
16+
17+ DeletedFile(['/var/www/html/index.html'])
18 ]
19
20 return audits
21@@ -94,5 +97,5 @@
22 ctxt['apache_version'] = re.search(r'.+version: Apache/(.+?)\s.+',
23 out).group(1)
24 ctxt['apache_icondir'] = '/usr/share/apache2/icons/'
25- ctxt['traceenable'] = settings['hardening']['traceenable']
26+
27 return ctxt
28
29=== modified file 'charmhelpers/contrib/hardening/apache/templates/hardening.conf'
30--- charmhelpers/contrib/hardening/apache/templates/hardening.conf 2016-03-14 12:39:21 +0000
31+++ charmhelpers/contrib/hardening/apache/templates/hardening.conf 2017-03-17 10:36:56 +0000
32@@ -16,3 +16,6 @@
33 </Location>
34
35 TraceEnable {{ traceenable }}
36+
37+SSLHonorCipherOrder {{ honor_cipher_order }}
38+SSLCipherSuite {{ cipher_suite }}
39
40=== modified file 'charmhelpers/contrib/hardening/defaults/apache.yaml'
41--- charmhelpers/contrib/hardening/defaults/apache.yaml 2016-03-14 13:57:17 +0000
42+++ charmhelpers/contrib/hardening/defaults/apache.yaml 2017-03-17 10:36:56 +0000
43@@ -9,5 +9,7 @@
44
45 hardening:
46 traceenable: 'off'
47- allowed_http_methods: "GET POST"
48- modules_to_disable: [ cgi, cgid ]
49\ No newline at end of file
50+ allowed_http_methods: 'GET POST'
51+ modules_to_disable: [ cgi, cgid ]
52+ honor_cipher_order: 'on'
53+ cipher_suite: 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'
54
55=== modified file 'charmhelpers/contrib/hardening/defaults/apache.yaml.schema'
56--- charmhelpers/contrib/hardening/defaults/apache.yaml.schema 2016-03-14 13:57:17 +0000
57+++ charmhelpers/contrib/hardening/defaults/apache.yaml.schema 2017-03-17 10:36:56 +0000
58@@ -7,3 +7,5 @@
59 hardening:
60 allowed_http_methods:
61 modules_to_disable:
62+ honor_cipher_order:
63+ cipher_suite:
64
65=== modified file 'charmhelpers/contrib/hardening/host/checks/__init__.py'
66--- charmhelpers/contrib/hardening/host/checks/__init__.py 2016-07-06 14:41:05 +0000
67+++ charmhelpers/contrib/hardening/host/checks/__init__.py 2017-03-17 10:36:56 +0000
68@@ -21,6 +21,7 @@
69 limits,
70 login,
71 minimize_access,
72+ openstack,
73 pam,
74 profile,
75 securetty,
76@@ -35,6 +36,7 @@
77 checks.extend(limits.get_audits())
78 checks.extend(login.get_audits())
79 checks.extend(minimize_access.get_audits())
80+ checks.extend(openstack.get_audits())
81 checks.extend(pam.get_audits())
82 checks.extend(profile.get_audits())
83 checks.extend(securetty.get_audits())
84
85=== added file 'charmhelpers/contrib/hardening/host/checks/openstack.py'
86--- charmhelpers/contrib/hardening/host/checks/openstack.py 1970-01-01 00:00:00 +0000
87+++ charmhelpers/contrib/hardening/host/checks/openstack.py 2017-03-17 10:36:56 +0000
88@@ -0,0 +1,30 @@
89+# Copyright 2016 Canonical Limited.
90+#
91+# Licensed under the Apache License, Version 2.0 (the "License");
92+# you may not use this file except in compliance with the License.
93+# You may obtain a copy of the License at
94+#
95+# http://www.apache.org/licenses/LICENSE-2.0
96+#
97+# Unless required by applicable law or agreed to in writing, software
98+# distributed under the License is distributed on an "AS IS" BASIS,
99+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
100+# See the License for the specific language governing permissions and
101+# limitations under the License.
102+from charmhelpers.contrib.hardening.audits.file import DirectoryPermissionAudit
103+
104+
105+def get_audits():
106+ """
107+ Get OS hardening security limits audits.
108+
109+ :returns: List of audits
110+ """
111+ audits = [
112+ DirectoryPermissionAudit('/etc/openstack-dashboard', user='horizon', group='horizon', mode=0o700),
113+ DirectoryPermissionAudit('/etc/cinder', user='cinder', group='cinder', mode=0o700),
114+ DirectoryPermissionAudit('/etc/glance', user='glance', group='glance', mode=0o700),
115+ DirectoryPermissionAudit('/etc/heat', user='heat', group='heat', mode=0o700),
116+ DirectoryPermissionAudit('/etc/ceilometer', user='ceilometer', group='ceilometer', mode=0o700)
117+ ]
118+ return audits
119
120=== modified file 'tests/contrib/hardening/apache/checks/test_config.py'
121--- tests/contrib/hardening/apache/checks/test_config.py 2016-07-06 14:41:05 +0000
122+++ tests/contrib/hardening/apache/checks/test_config.py 2017-03-17 10:36:56 +0000
123@@ -51,14 +51,16 @@
124 @patch.object(config.subprocess, 'call', lambda *args, **kwargs: 0)
125 def test_get_audits_apache_is_installed(self):
126 audits = config.get_audits()
127- self.assertEqual(6, len(audits))
128+ self.assertEqual(7, len(audits))
129
130 @patch.object(config.utils, 'get_settings', lambda x: {
131 'common': {'apache_dir': TEST_TMPDIR},
132 'hardening': {
133 'allowed_http_methods': {'GOGETEM'},
134 'modules_to_disable': {'modfoo'},
135- 'traceenable': 'off'
136+ 'traceenable': 'off',
137+ 'honor_cipher_order': 'on',
138+ 'cipher_suite': 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'
139 }
140 })
141 @patch.object(config, 'subprocess')
142@@ -77,4 +79,6 @@
143 '/usr/share/apache2/icons/',
144 'apache_version': '2.4.7',
145 'modules_to_disable': set(['modfoo']),
146- 'traceenable': 'off'})
147+ 'traceenable': 'off',
148+ 'honor_cipher_order': 'on',
149+ 'cipher_suite': 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'})
150
151=== modified file 'tests/contrib/hardening/host/checks/test_limits.py'
152--- tests/contrib/hardening/host/checks/test_limits.py 2016-07-06 14:41:05 +0000
153+++ tests/contrib/hardening/host/checks/test_limits.py 2017-03-17 10:36:56 +0000
154@@ -16,6 +16,7 @@
155
156 from mock import patch
157
158+from charmhelpers.contrib.hardening.audits import BaseAudit
159 from charmhelpers.contrib.hardening.host.checks import limits
160
161
162@@ -26,13 +27,8 @@
163 def test_core_dump_disabled(self):
164 audits = limits.get_audits()
165 self.assertEqual(2, len(audits))
166- audit = audits[0]
167- self.assertTrue(isinstance(audit, limits.DirectoryPermissionAudit))
168- self.assertEqual('/etc/security/limits.d', audit.paths[0])
169- audit = audits[1]
170- self.assertTrue(isinstance(audit, limits.TemplatedFile))
171- self.assertEqual('/etc/security/limits.d/10.hardcore.conf',
172- audit.paths[0])
173+ for audit in audits:
174+ self.assertTrue(isinstance(audit, BaseAudit))
175
176 @patch.object(limits.utils, 'get_settings', lambda x: {
177 'security': {'kernel_enable_core_dump': True}
178@@ -40,4 +36,5 @@
179 def test_core_dump_enabled(self):
180 audits = limits.get_audits()
181 self.assertEqual(1, len(audits))
182- self.assertTrue(isinstance(audits[0], limits.DirectoryPermissionAudit))
183+ for audit in audits:
184+ self.assertTrue(isinstance(audit, BaseAudit))
185
186=== added file 'tests/contrib/hardening/host/checks/test_openstack.py'
187--- tests/contrib/hardening/host/checks/test_openstack.py 1970-01-01 00:00:00 +0000
188+++ tests/contrib/hardening/host/checks/test_openstack.py 2017-03-17 10:36:56 +0000
189@@ -0,0 +1,24 @@
190+# Copyright 2016 Canonical Limited.
191+#
192+# Licensed under the Apache License, Version 2.0 (the "License");
193+# you may not use this file except in compliance with the License.
194+# You may obtain a copy of the License at
195+#
196+# http://www.apache.org/licenses/LICENSE-2.0
197+#
198+# Unless required by applicable law or agreed to in writing, software
199+# distributed under the License is distributed on an "AS IS" BASIS,
200+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
201+# See the License for the specific language governing permissions and
202+# limitations under the License.
203+
204+from unittest import TestCase
205+from charmhelpers.contrib.hardening.host.checks import openstack
206+from charmhelpers.contrib.hardening.audits.file import DirectoryPermissionAudit
207+
208+
209+class LimitsTestCase(TestCase):
210+ def test_core_dump_disabled(self):
211+ audits = openstack.get_audits()
212+ for audit in audits:
213+ self.assertTrue(isinstance(audit, DirectoryPermissionAudit))
214
215=== modified file 'tests/contrib/hardening/test_templating.py'
216--- tests/contrib/hardening/test_templating.py 2016-07-06 14:41:05 +0000
217+++ tests/contrib/hardening/test_templating.py 2017-03-17 10:36:56 +0000
218@@ -221,7 +221,9 @@
219 'hardening': {
220 'allowed_http_methods': {'GOGETEM'},
221 'modules_to_disable': {'modfoo'},
222- 'traceenable': 'off'
223+ 'traceenable': 'off',
224+ 'honor_cipher_order': 'on',
225+ 'cipher_suite': 'ALL:+MEDIUM:+HIGH:!LOW:!MD5:!RC4:!eNULL:!aNULL:!3DES'
226 }
227 })
228 @patch('charmhelpers.contrib.hardening.audits.file.os.path.exists',

Subscribers

People subscribed via source and target branches