Re: [Kimchi-devel] [PATCH] [Kimchi 6/9] Virt-Viewer launcher: adding FirewallManager class

Lucio, after doing some thinking I believe that this firewall class should remain in Kimchi for now. There is no immediate gain into adding the FirewallManager somewhere else while keeping this patch set waiting - specially considering that this is the only feature in all WoK plug-ins that is using it. Note that this does not mean that we can't start a RFC proposing that the FirewallManager should be hosted in WoK (perhaps Gingerbase?). We can send a RFC with a more robust proposal that can also deal with firewall zones configuration (Ginger has a request for this specific feature at https://github.com/kimchi-project/ginger/issues/107) and with more generic rules (adding udp ports for example). After this feature is discussed and contributed we can go back to this code and change it to use this new firewall backend.

Daniel On 07/07/2016 02:44 PM, Daniel Henrique Barboza wrote: > > > On 07/07/2016 12:10 PM, Lucio Correia wrote: >> This is very nice code and IMHO looks like a server feature, to be >> added to Wok. >> >> So my suggestion is to split this patch between Wok and Kimchi, >> keeping on Kimchi, as an extension of Wok class, only: >> > + def add_vm_graphics_port(self, vm_name, port): >> > + def remove_vm_graphics_port(self, vm_name): >> > + def remove_all_vms_ports(self): > > Yeah we can discuss if this feature can be deployed @ WoK. There's > a feature request for it in Ginger github too. > >> >> Also, isn't necessary a --reload after por is opened by firewall_cmd? >> > > No. In fact a '--reload' iwill reload the firewall with its permanent > rules set, overwriting any 'transient' changes done. > >> >> On 07-07-2016 09:57, dhbarboza82(a)gmail.com wrote: >>> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; >>> >>> The FirewallManager class opens and closes firewall ports >>> to allow for virt viewer connections in the graphics >>> server of the VM. >>> >>> For Fedora distros and Ubuntu, 'firewall-cmd' and 'ufw' is >>> used respectively. For all other distros, 'iptables' is >>> used. >>> >>> Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; >>> --- >>> model/virtviewerfile.py | 94 >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 94 insertions(+) >>> >>> diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py >>> index baccc8a..398b8a3 100644 >>> --- a/model/virtviewerfile.py >>> +++ b/model/virtviewerfile.py >>> @@ -26,6 +26,7 @@ from wok.config import config as wok_config >>> from wok.exception import NotFoundError, OperationFailed >>> from wok.plugins.kimchi import config as kimchi_config >>> from wok.plugins.kimchi.model.vms import VMModel >>> +from wok.utils import run_command, wok_log >>> >>> >>> def write_virt_viewer_file(params): >>> @@ -98,3 +99,96 @@ class VMVirtViewerFileModel(object): >>> >>> return 'plugins/kimchi/data/virtviewerfiles/%s' %\ >>> os.path.basename(file_path) >>> + >>> + >>> +class FirewallManager(object): >>> + >>> + @staticmethod >>> + def check_if_firewall_cmd_enabled(): >>> + _, _, r_code = run_command(['firewall-cmd', '--state', '-q']) >>> + return r_code == 0 >>> + >>> + @staticmethod >>> + def check_if_ufw_enabled(): >>> + _, _, r_code = run_command(['ufw', 'status']) >>> + return r_code == 0 >>> + >>> + def __init__(self): >>> + self.opened_ports = {} >>> + self.firewall_provider = None >>> + >>> + if self.check_if_firewall_cmd_enabled(): >>> + self.firewall_provider = FirewallCMDProvider() >>> + elif self.check_if_ufw_enabled(): >>> + self.firewall_provider = UFWProvider() >>> + else: >>> + self.firewall_provider = IPTablesProvider() >>> + >>> + def add_vm_graphics_port(self, vm_name, port): >>> + self.firewall_provider.enable_tcp_port(port) >>> + self.opened_ports[vm_name] = port >>> + >>> + def remove_vm_graphics_port(self, vm_name): >>> + port = self.opened_ports.pop(vm_name, None) >>> + if port: >>> + self.firewall_provider.disable_tcp_port(port) >>> + >>> + def remove_all_vms_ports(self): >>> + for port in self.opened_ports.values(): >>> + self.firewall_provider.disable_tcp_port(port) >>> + >>> + self.opened_ports = {} >>> + >>> + >>> +class FirewallCMDProvider(object): >>> + >>> + @staticmethod >>> + def enable_tcp_port(port): >>> + _, err, r_code = run_command( >>> + ['firewall-cmd', '--add-port=%s/tcp' % port] >>> + ) >>> + if r_code != 0: >>> + wok_log.error('Error when adding port to firewall-cmd: >>> %s' % err) >>> + >>> + @staticmethod >>> + def disable_tcp_port(port): >>> + _, err, r_code = run_command( >>> + ['firewall-cmd', '--remove-port=%s/tcp' % port] >>> + ) >>> + if r_code != 0: >>> + wok_log.error('Error when removing port from ' >>> + 'firewall-cmd: %s' % err) >>> + >>> + >>> +class UFWProvider(object): >>> + >>> + @staticmethod >>> + def enable_tcp_port(port): >>> + _, err, r_code = run_command(['ufw', 'allow', '%s/tcp' % >>> port]) >>> + if r_code != 0: >>> + wok_log.error('Error when adding port to ufw: %s' % err) >>> + >>> + @staticmethod >>> + def disable_tcp_port(port): >>> + _, err, r_code = run_command(['ufw', 'deny', '%s/tcp' % port]) >>> + if r_code != 0: >>> + wok_log.error('Error when removing port from ufw: %s' % >>> err) >>> + >>> + >>> +class IPTablesProvider(object): >>> + >>> + @staticmethod >>> + def enable_tcp_port(port): >>> + cmd = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', >>> + port, '-j', 'ACCEPT'] >>> + _, err, r_code = run_command(cmd) >>> + if r_code != 0: >>> + wok_log.error('Error when adding port to iptables: %s' >>> % err) >>> + >>> + @staticmethod >>> + def disable_tcp_port(port): >>> + cmd = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', >>> + port, '-j', 'ACCEPT'] >>> + _, err, r_code = run_command(cmd) >>> + if r_code != 0: >>> + wok_log.error('Error when removing port from itables: >>> %s' % err) >>> >> >> > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel