From 54a8d929aaae0eeffdae3c4fac3fb309f31d09a7 Mon Sep 17 00:00:00 2001
From: David Schmitz <schmitz@lrz.de>
Date: Thu, 1 Jun 2023 06:46:05 +0000
Subject: [PATCH] fix/prefix_overlap_handling: introduced setting
settings.ROUTES_DUPLICATES_CHECKING__ENFORCED_FOR_ALREADY_EXISTING_RULES to
disable/enable allowed editing of overlapping rule in case if it is already
existing in DB and no actual FlowSpec match/action parameters are changed
---
flowspec/views.py | 43 ++++++++++++++++++++++++++++++++++++++--
flowspy/settings.py.dist | 1 +
templates/apply.html | 7 ++++++-
3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/flowspec/views.py b/flowspec/views.py
index 29dd96c1..987037f6 100644
--- a/flowspec/views.py
+++ b/flowspec/views.py
@@ -479,6 +479,25 @@ def edit_route(request, route_slug):
)
return HttpResponseRedirect(reverse("group-routes"))
route_original = deepcopy(route_edit)
+
+ critical_changed_values = ['source', 'destination', 'sourceport', 'destinationport', 'port', 'protocol', 'then', 'fragmenttype']
+
+ try:
+ setting_dup_routes_chk = settings.ROUTES_DUPLICATES_CHECKING
+ except:
+ setting_dup_routes_chk = False
+ try:
+ setting_dup_routes_chk__lazy0 = not settings.ROUTES_DUPLICATES_CHECKING__ENFORCED_FOR_ALREADY_EXISTING_RULES
+ except:
+ setting_dup_routes_chk__lazy0 = True
+ setting_dup_routes_chk__lazy = setting_dup_routes_chk and setting_dup_routes_chk__lazy0
+ logger.info("setting_dup_routes_chk="+str(setting_dup_routes_chk))
+ logger.info("setting_dup_routes_chk__lazy0="+str(setting_dup_routes_chk__lazy0))
+ logger.info("setting_dup_routes_chk__lazy="+str(setting_dup_routes_chk__lazy))
+
+ add_error_msg=''
+ add_error_msg__overlapping_rule_already_existing_with_unchanged_flowspec = '(either solve the conflict by ensuring differing FlowSpec parameters for both rules, or as the rule already exists commit without changing any FlowSpec parameters)'
+
if request.POST:
request_data = request.POST.copy()
if request.user.is_superuser:
@@ -493,7 +512,6 @@ def edit_route(request, route_slug):
request_data,
instance=route_edit
)
- critical_changed_values = ['source', 'destination', 'sourceport', 'destinationport', 'port', 'protocol', 'then', 'fragmenttype']
form_is_valid = form.is_valid()
changed_data = form.changed_data
@@ -501,7 +519,7 @@ def edit_route(request, route_slug):
logger.info("view::edit(): => changed_data="+str(changed_data))
flowspec_attributes_changed = bool(set(changed_data) & set(critical_changed_values))
logger.info("view::edit(): => flowspec_attributes_changed="+str(flowspec_attributes_changed))
- if not form_is_valid and not flowspec_attributes_changed:
+ if setting_dup_routes_chk__lazy and not form_is_valid and not flowspec_attributes_changed:
logger.warn("view::edit(): WARNING, NOT form_is_valid, but not flowspec_attributes_changed, so trying with RouteForm_lightweight again")
form2 = RouteForm_lightweight(
request_data,
@@ -512,6 +530,7 @@ def edit_route(request, route_slug):
form = form2
form_is_valid = form.is_valid()
changed_data = form.changed_data
+ add_error_msg = add_error_msg__overlapping_rule_already_existing_with_unchanged_flowspec
else:
logger.warn("view::edit(): WARNING, NOT form_is_valid, but not flowspec_attributes_changed: trying with RouteForm_lightweight failed")
elif not form_is_valid:
@@ -547,6 +566,7 @@ def edit_route(request, route_slug):
{
'form': form,
'edit': True,
+ 'add_error' : add_error_msg,
'applier': applier,
'maxexpires': settings.MAX_RULE_EXPIRE_DAYS
}
@@ -563,6 +583,7 @@ def edit_route(request, route_slug):
{
'form': form,
'edit': True,
+ 'add_error' : add_error_msg,
'applier': applier,
'maxexpires': settings.MAX_RULE_EXPIRE_DAYS
}
@@ -601,6 +622,7 @@ def edit_route(request, route_slug):
'form': form,
'edit': True,
'applier': applier,
+ 'add_error' : add_error_msg,
'maxexpires': settings.MAX_RULE_EXPIRE_DAYS
})
else:
@@ -616,7 +638,23 @@ def edit_route(request, route_slug):
del dictionary['issuperuser']
except:
pass
+
form = RouteForm(dictionary)
+ form_is_valid = form.is_valid()
+ logger.info("view::edit(): form_is_valid="+str(form_is_valid))
+ #changed_data = form.changed_data
+ #flowspec_attributes_changed = bool(set(changed_data) & set(critical_changed_values))
+ #if not form_is_valid and not flowspec_attributes_changed:
+ if setting_dup_routes_chk__lazy and not form_is_valid:
+ logger.warn("view::edit(): WARNING, NOT form_is_valid, so trying with RouteForm_lightweight again")
+ form2 = RouteForm_lightweight(dictionary)
+ if form2.is_valid():
+ logger.warn("view::edit(): WARNING, NOT form_is_valid, RouteForm_lightweight is_valid")
+ add_error_msg = add_error_msg__overlapping_rule_already_existing_with_unchanged_flowspec
+ #form = form2
+ #form_is_valid = form.is_valid()
+ #changed_data = form.changed_data
+
if not request.user.is_superuser:
form.fields['then'] = forms.ModelMultipleChoiceField(queryset=ThenAction.objects.filter(action__in=settings.UI_USER_THEN_ACTIONS).order_by('action'), required=True)
form.fields['protocol'] = forms.ModelMultipleChoiceField(queryset=MatchProtocol.objects.filter(protocol__in=settings.UI_USER_PROTOCOLS).order_by('protocol'), required=False)
@@ -624,6 +662,7 @@ def edit_route(request, route_slug):
{
'form': form,
'edit': True,
+ 'add_error' : add_error_msg,
'applier': applier,
'maxexpires': settings.MAX_RULE_EXPIRE_DAYS
})
diff --git a/flowspy/settings.py.dist b/flowspy/settings.py.dist
index 4555ec22..3618e0d4 100644
--- a/flowspy/settings.py.dist
+++ b/flowspy/settings.py.dist
@@ -642,6 +642,7 @@ ALLOW_DELETE_FULL_FOR_USER_LIST = [
# with the same addresses). By default it is enabled (True).
#DISABLE_RULE_OVERLAP_CHECK = False
ROUTES_DUPLICATES_CHECKING = True
+ROUTES_DUPLICATES_CHECKING__ENFORCED_FOR_ALREADY_EXISTING_RULES = False
##############################################################################
##############################################################################
diff --git a/templates/apply.html b/templates/apply.html
index 77569afd..a1ebb6de 100644
--- a/templates/apply.html
+++ b/templates/apply.html
@@ -282,11 +282,15 @@
{% if form.non_field_errors %}
<div class="form-group {% if form.non_field_errors %} has-error {% endif %}" style="color: #A94442;" id="apply_rule_id">
{{ form.non_field_errors|unescape}}
- </div>
+ {% if edit %}{% if add_error %}<ul class="errorlist nonfield"><li>{{ add_error | unescape }}</li></ul>{% endif %}{% endif %}
+
+ </div>
{% endif %}
<fieldset {% if edit %} style="display:none;" {% endif %}>
<div class="form-group {% if form.name.errors %} has-error {% endif %}">
+
+
<label for="id_name" class="col-md-2 control-label"><b>{% trans "Name" %}</b></label>
<div class="col-md-8">
{% render_field form.name class+="form-control" placeholder="Eg. ntpattack. A unique identifier will be added as a suffix" %}
@@ -299,6 +303,7 @@
<fieldset>
<hr>
<div class="form-group {% if form.applier.errors %} has-error {% endif %}">
+
<label for="id_name" class="col-md-2 control-label"><b>{% trans "Applier" %}</b></label>
<div class="col-md-8">
{% render_field form.applier class+="form-control" %}
--
GitLab