From f384afa8f9fc9aa51c0222355ea6143a515fa4bb Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Tue, 28 Mar 2023 21:50:46 +0200 Subject: [PATCH] VPN: IPsec: Security Policy Database - Manual assignments linking to connection children (https://github.com/opnsense/core/issues/6451) Add connection child as option for manual SPDs, to make sure these are easily selectable we'll extend ModelRelationField to include a method to return it's value (so we can combine parent descriptions) --- .../IPsec/Api/ManualSpdController.php | 2 +- .../OPNsense/IPsec/forms/dialogSPD.xml | 9 ++++ .../Base/FieldTypes/ModelRelationField.php | 19 +++++++- .../mvc/app/models/OPNsense/IPsec/Swanctl.php | 23 ++++++--- .../mvc/app/models/OPNsense/IPsec/Swanctl.xml | 11 ++++- .../mvc/app/views/OPNsense/IPsec/spd.volt | 1 + src/opnsense/scripts/ipsec/updown_event.py | 47 ++++++++++--------- .../OPNsense/IPsec/reqid_events.conf | 21 ++++----- 8 files changed, 91 insertions(+), 42 deletions(-) diff --git a/src/opnsense/mvc/app/controllers/OPNsense/IPsec/Api/ManualSpdController.php b/src/opnsense/mvc/app/controllers/OPNsense/IPsec/Api/ManualSpdController.php index 82380d3e7..79a4332f9 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/IPsec/Api/ManualSpdController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/IPsec/Api/ManualSpdController.php @@ -43,7 +43,7 @@ class ManualSPDController extends ApiMutableModelControllerBase { return $this->searchBase( 'SPDs.SPD', - ['enabled', 'description', 'origin', 'reqid', 'source', 'destination'] + ['enabled', 'description', 'origin', 'reqid', 'connection_child', 'source', 'destination'] ); } diff --git a/src/opnsense/mvc/app/controllers/OPNsense/IPsec/forms/dialogSPD.xml b/src/opnsense/mvc/app/controllers/OPNsense/IPsec/forms/dialogSPD.xml index b72b856a1..615154c65 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/IPsec/forms/dialogSPD.xml +++ b/src/opnsense/mvc/app/controllers/OPNsense/IPsec/forms/dialogSPD.xml @@ -13,6 +13,15 @@ to reconnect in order to add the entry as we use an updown event. + + spd.connection_child + + dropdown + + Connection child to register this manual item on. Please note that an established tunnel needs + to reconnect in order to add the entry as we use an updown event. + + spd.source diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php index 2f923675f..01799550d 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php +++ b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php @@ -105,7 +105,11 @@ class ModelRelationField extends BaseListField $descriptions = []; foreach ($displayKeys as $displayKey) { if ($node->$displayKey != null) { - $descriptions[] = (string)$node->$displayKey; + if ($node->$displayKey->getObjectType() == 'ModelRelationField') { + $descriptions[] = $node->$displayKey->display_value(); + } else { + $descriptions[] = (string)$node->$displayKey; + } } else { $descriptions[] = ""; } @@ -206,6 +210,19 @@ class ModelRelationField extends BaseListField return parent::getNodeData(); } + /** + * @return string string display value of this field + */ + public function display_value() + { + $tmp = []; + foreach (explode(',', $this->internalValue) as $key) { + $tmp[] = $this->internalOptionList[$key] ?? ''; + } + return implode(',', $tmp); + } + + /** * retrieve field validators for this field type * @return array returns Text/regex validator diff --git a/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.php b/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.php index 57125381a..0d7f6e72c 100644 --- a/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.php +++ b/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.php @@ -45,6 +45,7 @@ class Swanctl extends BaseModel { $messages = parent::performValidation($validateFullModel); $vtis = []; + $spds = []; foreach ($this->getFlatNodes() as $key => $node) { if ($validateFullModel || $node->isFieldChanged()) { @@ -54,6 +55,8 @@ class Swanctl extends BaseModel $parentTagName = $parentNode->getInternalXMLTagName(); if ($parentTagName === 'VTI') { $vtis[$parentKey] = $parentNode; + } elseif ($parentTagName === 'SPD') { + $spds[$parentKey] = $parentNode; } } } @@ -71,6 +74,17 @@ class Swanctl extends BaseModel } } + foreach ($spds as $key => $node) { + if ( + ((string)$node->reqid == '' && (string)$node->connection_child == '') || + ((string)$node->reqid != '' && (string)$node->connection_child != '') + ) { + $messages->appendMessage( + new Message(gettext("Either reqid or child must be set"), $key . ".connection_child") + ); + } + } + return $messages; } @@ -174,12 +188,9 @@ class Swanctl extends BaseModel if (!isset($data['connections'][$parent]['children'])) { $data['connections'][$parent]['children'] = []; } - if (!empty($thisnode['reqid'])) { - // trigger updown event handler when a reqid is set. - // currently this only handles manual spd entries, we may extend/refactor the script later - // if needed - $thisnode['updown'] = '/usr/local/opnsense/scripts/ipsec/updown_event.py'; - } + $thisnode['updown'] = '/usr/local/opnsense/scripts/ipsec/updown_event.py --connection_child '; + $thisnode['updown'] .= $node_uuid; + $data['connections'][$parent]['children'][$node_uuid] = $thisnode; } else { $data['connections'][$parent][$key . '-' . $node_uuid] = $thisnode; diff --git a/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.xml b/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.xml index b061b5e8e..01f570651 100644 --- a/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.xml +++ b/src/opnsense/mvc/app/models/OPNsense/IPsec/Swanctl.xml @@ -413,8 +413,17 @@ 1 65535 - Y + + + + OPNsense.IPsec.Swanctl + children.child + connection,description + %s - %s + + + Y N diff --git a/src/opnsense/mvc/app/views/OPNsense/IPsec/spd.volt b/src/opnsense/mvc/app/views/OPNsense/IPsec/spd.volt index 18a3ae5a7..54800814d 100644 --- a/src/opnsense/mvc/app/views/OPNsense/IPsec/spd.volt +++ b/src/opnsense/mvc/app/views/OPNsense/IPsec/spd.volt @@ -158,6 +158,7 @@ {{ lang._('Origin') }} {{ lang._('Enabled') }} {{ lang._('Reqid') }} + {{ lang._('Child') }} {{ lang._('Source') }} {{ lang._('Destination') }} {{ lang._('Description') }} diff --git a/src/opnsense/scripts/ipsec/updown_event.py b/src/opnsense/scripts/ipsec/updown_event.py index 70c0a9c30..32cbe4021 100755 --- a/src/opnsense/scripts/ipsec/updown_event.py +++ b/src/opnsense/scripts/ipsec/updown_event.py @@ -1,7 +1,7 @@ #!/usr/local/bin/python3 """ - Copyright (c) 2022 Ad Schellevis + Copyright (c) 2022-2023 Ad Schellevis All rights reserved. Redistribution and use in source and binary forms, with or without @@ -44,6 +44,7 @@ spd_add_cmd = 'spdadd -%(ipproto)s %(source)s %(destination)s any ' \ if __name__ == '__main__': parser = argparse.ArgumentParser() + parser.add_argument('--connection_child', help='uuid of the connection child') parser.add_argument('--reqid', default=os.environ.get('PLUTO_REQID')) parser.add_argument('--local', default=os.environ.get('PLUTO_ME')) parser.add_argument('--remote', default=os.environ.get('PLUTO_PEER')) @@ -56,28 +57,32 @@ if __name__ == '__main__': if os.path.exists(spd_filename): cnf = ConfigParser() cnf.read(spd_filename) - conf_section = 'spd_%s' % cmd_args.reqid - if cnf.has_section(conf_section): - spds = {} - for opt in cnf.options(conf_section): - if opt.count('_') == 1: - tmp = opt.split('_') - seqid = tmp[1] - if seqid not in spds: - spds[seqid] = { - 'reqid': cmd_args.reqid, - 'local' : cmd_args.local, - 'remote' : cmd_args.remote, - 'destination': os.environ.get('PLUTO_PEER_CLIENT') - } - if cnf.get(conf_section, opt).strip() != '': - spds[seqid][tmp[0]] = cnf.get(conf_section, opt).strip() - # (re)aaply manual policies if specified - cur_spds = list_spds(req_id=cmd_args.reqid, automatic=False) + spds = [] + for section in cnf.sections(): + if cnf.get(section, 'reqid') == cmd_args.reqid \ + or cnf.get(section, 'connection_child') == cmd_args.connection_child: + spds.append({ + 'reqid': cmd_args.reqid, + 'local' : cmd_args.local, + 'remote' : cmd_args.remote, + 'destination': os.environ.get('PLUTO_PEER_CLIENT') + }) + for opt in cnf.options(section): + if cnf.get(section, opt).strip() != '': + spds[-1][opt] = cnf.get(section, opt).strip() + + # (re)apply manual policies if specified + cur_spds = list_spds(automatic=False) set_key = [] for spd in cur_spds: - set_key.append('spddelete -n %(src)s %(dst)s any -P %(direction)s;' % spd) - for spd in spds.values(): + policy_found = False + for mspd in spds: + if mspd['source'] == spd['src'] and mspd['destination'] == spd['dst']: + policy_found = True + if policy_found or spd['reqid'] == cmd_args.reqid: + set_key.append('spddelete -n %(src)s %(dst)s any -P %(direction)s;' % spd) + + for spd in spds: if None in spd.values(): # incomplete, skip continue diff --git a/src/opnsense/service/templates/OPNsense/IPsec/reqid_events.conf b/src/opnsense/service/templates/OPNsense/IPsec/reqid_events.conf index ac1f1ba52..9c3055f60 100644 --- a/src/opnsense/service/templates/OPNsense/IPsec/reqid_events.conf +++ b/src/opnsense/service/templates/OPNsense/IPsec/reqid_events.conf @@ -3,19 +3,16 @@ # Do not edit this file manually. # -{% set prev_spd = {'reqid' : None } %} -{% for spd in helpers.toList('OPNsense.Swanctl.SPDs.SPD', 'reqid', 'int') %} +{% for spd in helpers.toList('OPNsense.Swanctl.SPDs.SPD') %} {% if spd.enabled == '1' %} -{% if prev_spd.reqid != spd.reqid %} -[spd_{{spd.reqid}}] -{% endif %} -protocol_{{loop.index}}={{spd.protocol}} -reqid_{{loop.index}}={{spd.reqid}} -source_{{loop.index}}={{spd.source}} -destination_{{loop.index}}={{spd.destination}} -description_{{loop.index}}={{spd.description}} -uuid_{{loop.index}}={{spd['uuid']}} -{% do prev_spd.update({'reqid': spd.reqid}) %} +[spd_{{spd['@uuid']|replace('-', '')}}] +protocol ={{spd.protocol}} +reqid = {{spd.reqid}} +connection_child = {{spd.connection_child}} +source = {{spd.source}} +destination = {{spd.destination}} +description = {{spd.description}} +uuid = {{spd['@uuid']}} {% endif %} {% endfor %}