Allow GRE traffic in converged architecture

In a converged architecture with storage and compute on the same
host, UFW can get in the way of tunneled traffic interpreting it as
INVALID. UFW makes solving this more difficult than it needs to be.
See http://northernmost.org/blog/gre-tunnels-and-ufw/index.html for
context.

This change updates /etc/ufw/before.rules to add GRE as an allowed
input.

Also, guarantee ufw is installed for LP #1763716

Please review and merge charm-helpers first:
https://github.com/juju/charm-helpers/pull/170

Change-Id: I789854c33e3af12f7412633dbf7c921beb0ed2b5
Closes-Bug: #1757564
Closes-Bug: #1763716
This commit is contained in:
David Ames 2018-05-01 10:14:54 -07:00 committed by Edward Hope-Morley
parent 6346a7458b
commit 18d0a891db
6 changed files with 128 additions and 1 deletions

2
.gitignore vendored
View File

@ -6,3 +6,5 @@ bin
tags
*.sw[nop]
*.pyc
tests/cirros-*
func-results.json

View File

@ -151,6 +151,29 @@ def enable(soft_fail=False):
return True
def reload():
"""
Reload ufw
:returns: True if ufw is successfully enabled
"""
output = subprocess.check_output(['ufw', 'reload'],
universal_newlines=True,
env={'LANG': 'en_US',
'PATH': os.environ['PATH']})
m = re.findall('^Firewall reloaded\n',
output, re.M)
hookenv.log(output, level='DEBUG')
if len(m) == 0:
hookenv.log("ufw couldn't be reloaded", level='WARN')
return False
else:
hookenv.log("ufw reloaded", level='INFO')
return True
def disable():
"""
Disable ufw

View File

@ -171,6 +171,13 @@ options:
Sample rate determines what percentage of the metric points a
client should send to the server.
Only takes effect if statsd-host is set.
enable-firewall:
type: boolean
default: True
description: |
By default the swift-storage charm will use the UFW firewall to
protect storage daemons. This option allows the administrator to
disable this feature.
allow-ufw-ip6-softfail:
description: |
When this option is set to True the charm will disable the IPv6

View File

@ -15,7 +15,9 @@
# limitations under the License.
import os
import shutil
import sys
import tempfile
from lib.swift_storage_utils import (
PACKAGES,
@ -87,6 +89,49 @@ CONFIGS = register_configs()
NAGIOS_PLUGINS = '/usr/local/lib/nagios/plugins'
SUDOERS_D = '/etc/sudoers.d'
STORAGE_MOUNT_PATH = '/srv/node'
UFW_DIR = '/etc/ufw'
def add_ufw_gre_rule(ufw_rules_path):
"""Add allow gre rule to UFW
Make a copy of existing UFW before rules, insert our new rule and replace
existing rules with updated version.
"""
rule = '-A ufw-before-input -p 47 -j ACCEPT'
rule_exists = False
with tempfile.NamedTemporaryFile() as tmpfile:
# Close pre-opened file so that we can replace it with a copy of our
# config file.
tmpfile.file.close()
dst = tmpfile.name
# Copy over ufw rules file
shutil.copyfile(ufw_rules_path, dst)
with open(dst, 'r') as fd:
lines = fd.readlines()
# Check whether the line we are adding already exists
for line in lines:
if rule in line:
rule_exists = True
break
added = False
if not rule_exists:
# Insert our rule as the first rule.
with open(dst, 'w') as fd:
for line in lines:
if not added and line.startswith('-A'):
fd.write('# Allow GRE Traffic (added by swift-storage '
'charm)\n')
fd.write('{}\n'.format(rule))
fd.write(line)
added = True
else:
fd.write(line)
# Replace existing config with updated one.
shutil.copyfile(dst, ufw_rules_path)
def initialize_ufw():
@ -96,6 +141,11 @@ def initialize_ufw():
:return: None
"""
if not config('enable-firewall'):
log("Firewall has been administratively disabled", "DEBUG")
return
# this charm will monitor exclusively the ports used, using 'allow' as
# default policy enables sharing the machine with other services
ufw.default_policy('allow', 'incoming')
@ -108,6 +158,10 @@ def initialize_ufw():
# Enable
ufw.enable(soft_fail=config('allow-ufw-ip6-softfail'))
# Allow GRE traffic
add_ufw_gre_rule(os.path.join(UFW_DIR, 'before.rules'))
ufw.reload()
@hooks.hook('install.real')
@harden()
@ -128,7 +182,10 @@ def install():
@pause_aware_restart_on_change(RESTART_MAP)
@harden()
def config_changed():
initialize_ufw()
if config('enable-firewall'):
initialize_ufw()
else:
ufw.disable()
if config('prefer-ipv6'):
status_set('maintenance', 'Configuring ipv6')
assert_charm_supports_ipv6()

View File

@ -87,6 +87,7 @@ from charmhelpers.core.decorators import (
PACKAGES = [
'swift', 'swift-account', 'swift-container', 'swift-object',
'xfsprogs', 'gdisk', 'lvm2', 'python-jinja2', 'python-psutil',
'ufw',
]
VERSION_PACKAGE = 'swift-account'
@ -621,6 +622,11 @@ def setup_ufw():
:side effect: calls several external functions
:return: None
"""
if not config('enable-firewall'):
log("Firewall has been administratively disabled", "DEBUG")
return
ports = [config('object-server-port'),
config('container-server-port'),
config('account-server-port')]

View File

@ -16,6 +16,7 @@ from mock import patch
import os
import json
import uuid
import tempfile
from test_utils import CharmTestCase, patch_open
@ -53,6 +54,7 @@ TO_PATCH = [
'fetch_swift_rings',
'save_script_rc',
'setup_rsync',
'rsync',
'setup_storage',
'register_configs',
'update_nrpe_config',
@ -67,6 +69,22 @@ TO_PATCH = [
]
UFW_DUMMY_RULES = """
# Don't delete these required lines, otherwise there will be errors
*filter
:ufw-before-input - [0:0]
:ufw-before-output - [0:0]
:ufw-before-forward - [0:0]
:ufw-not-local - [0:0]
# End required lines
# allow all on loopback
-A ufw-before-input -i lo -j ACCEPT
-A ufw-before-output -o lo -j ACCEPT
"""
class SwiftStorageRelationsTests(CharmTestCase):
def setUp(self):
@ -76,10 +94,12 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.relation_get.side_effect = self.test_relation.get
self.get_relation_ip.return_value = '10.10.10.2'
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
def test_prunepath(self):
hooks.config_changed()
self.add_to_updatedb_prunepath.assert_called_with("/srv/node")
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
def test_install_hook(self):
self.test_config.set('openstack-origin', 'cloud:precise-havana')
hooks.install()
@ -92,6 +112,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertTrue(self.setup_storage.called)
self.assertTrue(self.execd_preinstall.called)
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
def test_config_changed_no_upgrade_available(self):
self.openstack_upgrade_available.return_value = False
self.relations_of_type.return_value = False
@ -102,6 +123,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertTrue(self.CONFIGS.write_all.called)
self.assertTrue(self.setup_rsync.called)
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
def test_config_changed_upgrade_available(self):
self.openstack_upgrade_available.return_value = True
self.relations_of_type.return_value = False
@ -111,6 +133,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertTrue(self.do_openstack_upgrade.called)
self.assertTrue(self.CONFIGS.write_all.called)
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
def test_config_changed_with_openstack_upgrade_action(self):
self.openstack_upgrade_available.return_value = True
self.test_config.set('action-managed-upgrade', True)
@ -121,6 +144,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertFalse(self.do_openstack_upgrade.called)
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
def test_config_changed_nrpe_master(self):
self.openstack_upgrade_available.return_value = False
self.relations_of_type.return_value = True
@ -131,6 +155,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertTrue(self.setup_rsync.called)
self.assertTrue(self.update_nrpe_config.called)
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
@patch.object(hooks, 'assert_charm_supports_ipv6')
def test_config_changed_ipv6(self, mock_assert_charm_supports_ipv6):
self.test_config.set('prefer-ipv6', True)
@ -142,6 +167,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertTrue(self.CONFIGS.write_all.called)
self.assertTrue(self.setup_rsync.called)
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
@patch.object(hooks, 'ensure_devs_tracked')
def test_upgrade_charm(self, mock_ensure_devs_tracked):
self.filter_installed_packages.return_value = [
@ -323,3 +349,9 @@ class SwiftStorageRelationsTests(CharmTestCase):
def test_main_hook_missing(self, _argv):
hooks.main()
self.assertTrue(self.log.called)
def test_add_ufw_gre_rule(self):
with tempfile.NamedTemporaryFile() as tmpfile:
tmpfile.file.write(UFW_DUMMY_RULES)
tmpfile.file.close()
hooks.add_ufw_gre_rule(tmpfile.name)