Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net/udpbroadcastrelay: Add options for MSEARCH and Allow/Block CIDR #4169

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

net/udpbroadcastrelay: Add options for MSEARCH and Allow/Block CIDR #4169

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bf306bf
Add fields for Allow CIDRs, Block CIDRs, and M-SEARCH
ZephireNZ Jan 9, 2024
4fe2c64
Update version
ZephireNZ Jan 9, 2024
2c76194
Fix help text
ZephireNZ Jan 9, 2024
fd5be3e
Fix M-SEARCH mask
ZephireNZ Jan 9, 2024
2277ece
Merge remote-tracking branch 'upstream/master' into udpbroadcastrelay…
ZephireNZ Apr 28, 2024
e1baa76
Merge remote-tracking branch 'upstream/master' into udpbroadcastrelay…
ZephireNZ Aug 10, 2024
07c0ae0
Update net/udpbroadcastrelay/Makefile
fichtner Aug 27, 2024
665595a
Cleanup help descriptions
ZephireNZ Aug 31, 2024
f57531a
Update pkg-descr with changelog and example that uses M-SEARCH
ZephireNZ Aug 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions net/udpbroadcastrelay/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
PLUGIN_NAME= udpbroadcastrelay
PLUGIN_VERSION= 1.0
PLUGIN_REVISION= 5
PLUGIN_VERSION= 1.1
PLUGIN_COMMENT= Control udpbroadcastrelay processes
PLUGIN_DEPENDS= udpbroadcastrelay
PLUGIN_MAINTAINER= [email protected]
Expand Down
25 changes: 25 additions & 0 deletions ...astrelay/src/opnsense/mvc/app/controllers/OPNsense/UDPBroadcastRelay/forms/dialogEdit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,31 @@
<type>checkbox</type>
<help><![CDATA[Use the ID to set the TTL Value of the packet (Original method) rather than DSCP, default is not ticked]]></help>
</field>
<field>
<id>udpbroadcastrelay.allowcidr</id>
<label>Allowed CIDRs</label>
<style>tokenize</style>
<type>select_multiple</type>
<allownew>true</allownew>
<help><![CDATA[Only allow packets from a range of IP source addresses]]></help>
ZephireNZ marked this conversation as resolved.
Show resolved Hide resolved
<advanced>true</advanced>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see all of these are advanced. I don't mind, but it looks interesting releasing a 1.1 and not offering new features at first glance :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSearch definitely should be advanced feature (given the specific sytax requirements and function).

Maybe Allow/Block CIDR could be non-advanced?

</field>
<field>
<id>udpbroadcastrelay.blockcidr</id>
<label>Blocked CIDRs</label>
<style>tokenize</style>
<type>select_multiple</type>
<allownew>true</allownew>
<help><![CDATA[Block packets from a range of IP source addresses<br/>Where overlapping CIDRs are specified with the Block/Allow CIDRs, the most specific match (longest prefix) will take effect]]></help>
<advanced>true</advanced>
</field>
<field>
<id>udpbroadcastrelay.msearch</id>
<label>M-SEARCH processing</label>
<type>text</type>
<help><![CDATA[A space-separated list of M-SEARCH processing actions, use a comma before search terms for example <code>block,search-term</code><br/>Please refer to the <a href="https://github.com/marjohn56/udpbroadcastrelay#usage">UDP Broadcast Relay documentation</a> for more information]]></help>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTML in these parts just ends up bouncing the help text from translations because the translation could break the HTML due to typos which are hard to spot and debug. We don't have a good solution for this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want me to remove the link to the documentation? Otherwise not sure what I am meant to do for this one

<advanced>true</advanced>
</field>
<field>
<id>udpbroadcastrelay.description</id>
<label>Description</label>
Expand Down
26 changes: 26 additions & 0 deletions ...oadcastrelay/src/opnsense/mvc/app/models/OPNsense/UDPBroadcastRelay/UDPBroadcastRelay.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,32 @@
<default>0</default>
<Required>Y</Required>
</RevertTTL>
<allowcidr type="NetworkField">
<default></default>
<Required>N</Required>
<FieldSeparator>,</FieldSeparator>
<AsList>Y</AsList>
<NetMaskRequired>N</NetMaskRequired>
<NetMaskAllowed>Y</NetMaskAllowed>
<AddressFamily>ipv4</AddressFamily>
<WildcardEnabled>N</WildcardEnabled>
</allowcidr>
<blockcidr type="NetworkField">
<default></default>
<Required>N</Required>
<FieldSeparator>,</FieldSeparator>
<AsList>Y</AsList>
<NetMaskRequired>N</NetMaskRequired>
<NetMaskAllowed>Y</NetMaskAllowed>
<AddressFamily>ipv4</AddressFamily>
<WildcardEnabled>N</WildcardEnabled>
</blockcidr>
<msearch type="TextField">
<default></default>
<Required>N</Required>
<mask>/^(?:(?:block|fwd|proxy|dial)(?:,\S+|) )*(?:block|fwd|proxy|dial)(?:,\S+|)$/</mask>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably as fragile as it gets with CSV and freestyle syntax matched via regex. :(

Copy link
Author

@ZephireNZ ZephireNZ Aug 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... is quite the ugly one. It could just be free text with no restriction, but not sure that's any better as people will wonder why its not working 😅

I did space separated as it needed to be split into separate args, is there a better input type to use in the UI for that?

<ValidationMessage>Must be a space separated list of actions, with optional search term.</ValidationMessage>
</msearch>
<description type="TextField">
<Required>N</Required>
<mask>/^([\t\n\v\f\r 0-9a-zA-Z.,_\x{00A0}-\x{FFFF}]){1,255}$/u</mask>
Expand Down
15 changes: 15 additions & 0 deletions net/udpbroadcastrelay/src/opnsense/service/templates/OPNsense/UDPBroadcastRelay/rc.conf.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ osudpbroadcastrelay_enable="YES"
{% if osudpbroadcastrelay.sourceaddress %}
{% do Parameters.append("-s " ~ osudpbroadcastrelay.sourceaddress) %}
{% endif %}
{% if osudpbroadcastrelay.allowcidr %}
{% for allowcidr in osudpbroadcastrelay.allowcidr %}
{% do Parameters.append("--allowcidr " ~ allowcidr) %}
{% endfor %}
{% endif %}
{% if osudpbroadcastrelay.blockcidr %}
{% for blockcidr in osudpbroadcastrelay.blockcidr %}
{% do Parameters.append("--allowcidr " ~ blockcidr) %}
{% endfor %}
{% endif %}
{% if osudpbroadcastrelay.msearch %}
{% for msearch in osudpbroadcastrelay.msearch.split(' ') %}
{% do Parameters.append("--msearch " ~ (msearch | shlex_quote)) %}
{% endfor %}
{% endif %}
{% if osudpbroadcastrelay.RevertTTL|default('0') == '1' %}
{% do Parameters.append("-t ") %}
{% endif %}
Expand Down