pbr: update to 1.1.4-5 #23684

stangri · 2024-03-18T02:02:47Z

Maintainer: me
Compile tested: x86_64, Sophos XG-135r3, OpenWrt 23.05.2
Run tested: x86_64, Sophos XG-135r3, OpenWrt 23.05.2

Description:
This update includes the following changes:

Makefile

update copyright
attempt to implement the proper variants to avoid luci-app dependency on both variants
quietly stop service on uninstall

Config-file

add the list of dnsmasq instances to target in supported dnsmasq modes
for default pbr variant, set the resolver_set to dnsmasq.nftset
for iptables pbr variant, set the resolver_set to dnsmasq.ipset
add the nft_file_support (disabled by default)
introduce procd_boot_delay to delay service start on boot
introduce the following nft set creation options:
- nft_set_auto_merge
- nft_set_counter
- nft_set_flags_interval
- nft_set_flags_timeout
- nft_set_gc_interval
- nft_set_policy
- nft_set_timeout
add the pbr.user.wg_server_and_client custom user script to allow running wg server and client at the same time
add the "Ignore Local Requests" sample policy

Hotplug firewall/interface scripts

better logged messages

The pbr and pbr-iptables uci defaults script

use functions from the init script
improve vpn-policy-routing migration

The pbr-netifd uci defaults script

use functions from the init script
improve uci operations

Introduce the firewall.include file
Improve pbr.user.aws custom user script
Improve pbr.user.netflix custom user script
Introduce pbr.user.wg_server_and_client custom user script
Update the init file:

refactor some code to allow the init script file to be sourced by the uci defaults scripts and the luci rpcd script for shared functions
add support for nft_file_mode in which service prepares the fw4-compatible atomic nft/include file for faster operations on service reload
improve Tor support (nft mode only)
implement support for nft set options
update validation functions for new options/parameters

systemcrash · 2024-03-18T14:33:28Z

net/pbr/files/usr/share/pbr/pbr.user.netflix

-[ -z "$nft" ] && nft="$(command -v nft)"
-_ret=1
+#DB_SOURCE='api.bgpview.io'
+REGEX_IPV4='[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\/[0-9]\{1,\}'


Nitpick... Are there ever cases when the mask is > 32? Perhaps it's sensible to close the range?:

REGEX_IPV4='[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\/[0-9]\{1,2\}'

Paul, thank you for your review. As I don't use these scripts myself, I have no way of testing for the desired result, so I don't want to change things in them unless I have to. If things work as is, I'd prefer to keep code as is.

systemcrash · 2024-03-18T14:35:58Z

net/pbr/files/usr/share/pbr/pbr.user.netflix

-_ret=1
+#DB_SOURCE='api.bgpview.io'
+REGEX_IPV4='[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\/[0-9]\{1,\}'
+REGEX_IPV6='.*::.*'


I know we like simple expressions, but... Does this match if someone/something provides a non-abbreviated IPv6?

2001:0db8:0000:0000:0000:8a2e:0370:7334? I don't know what the likelihood of that is here.

Is a workaround to look for [ and ]?

If iPv4 don't have ports in them, perhaps just check for :

Paul, thank you for your review. As I don't use these scripts myself, I have no way of testing for the desired result, so I don't want to change things in them unless I have to. If things work as is, I'd prefer to keep code as is.

pesa1234 · 2024-03-18T17:08:47Z

@stangri I can not compile it, I get error:
install: cannot stat './files/usr/share/pbr/.keep': No such file or directory

$(INSTALL_DATA) ./files/usr/share/pbr/.keep $(1)/usr/share/pbr/.keep

Take the .keep file from your repo and now it compile

systemcrash · 2024-03-18T20:38:42Z

@stangri I can not compile it, I get error: install: cannot stat './files/usr/share/pbr/.keep': No such file or directory

$(INSTALL_DATA) ./files/usr/share/pbr/.keep $(1)/usr/share/pbr/.keep

Take the .keep file from your repo and now it compile

Yeah, test cases fail at this stage.

systemcrash · 2024-03-19T08:05:11Z

Currently failing with:

...
Failed to connect to ubus
Failed to connect to ubus
Failed to connect to ubus
Failed to connect to ubus
Failed to connect to ubus
ERROR: The pbr service is currently disabled!
WARNING: Resolver set (dnsmasq.ipset) is not supported on this system.
WARNING: Resolver set (dnsmasq.ipset) is not supported on this system.
touch: /var/run/pbr.lock: No such file or directory

stangri · 2024-03-20T10:26:17Z

Currently failing with:

It's only failing for some architectures and not others. I actually build and test on x86_64, and CI is failing that arch. I believe this was also the case with the previous PR.

Can anyone shed any light on why CI is trying to run the service and where failed ubus calls are coming from?

pesa1234 · 2024-03-20T14:18:15Z

Currently failing with:

It's only failing for some architectures and not others. I actually build and test on x86_64, and CI is failing that arch. I believe this was also the case with the previous PR.

Can anyone shed any light on why CI is trying to run the service and where failed ubus calls are coming from?

Where the build is fine seems that the test with docker container is skipped.

This update includes the following changes: 1. Makefile * update copyright * attempt to implement the proper variants to avoid luci-app dependency on both variants * quietly stop service on uninstall 2. Config-file * add the list of dnsmasq instances to target in supported dnsmasq modes * for default pbr variant, set the `resolver_set` to `dnsmasq.nftset` * for iptables pbr variant, set the `resolver_set` to `dnsmasq.ipset` * add the `nft_file_support` (disabled by default) * introduce `procd_boot_delay` to delay service start on boot * introduce the following nft set creation options: * nft_set_auto_merge * nft_set_counter * nft_set_flags_interval * nft_set_flags_timeout * nft_set_gc_interval * nft_set_policy * nft_set_timeout * add the pbr.user.wg_server_and_client custom user script to allow running wg server and client at the same time * add the "Ignore Local Requests" sample policy 3. Hotplug firewall/interface scripts * better logged messages 4. The pbr and pbr-iptables uci defaults script * use functions from the init script * improve vpn-policy-routing migration 5. The pbr-netifd uci defaults script * use functions from the init script * improve uci operations 6. Introduce the firewall.include file 7. Improve pbr.user.aws custom user script 8. Improve pbr.user.netflix custom user script 9. Introduce pbr.user.wg_server_and_client custom user script 10. Update the init file: * refactor some code to allow the init script file to be sourced by the uci defaults scripts and the luci rpcd script for shared functions * add support for `nft_file_mode` in which service prepares the fw4-compatible atomic nft/include file for faster operations on service reload * improve Tor support (nft mode only) * implement support for nft set options * update validation functions for new options/parameters Signed-off-by: Stan Grishin <stangri@melmac.ca>

systemcrash · 2024-03-20T19:50:45Z

Green across the board 🥳

pesa1234 · 2024-03-23T01:07:27Z

Can you please update to 1.1.4.-r7 thanks?

Sorry, I have just see the PR. Thanks a lot

stangri · 2024-04-09T16:02:36Z

@hnyman Hannu, could you please do me a favor and have a look at/comment on the Makefile? I believe I was following your instructions from the mail-list or forum trying to do the variants better, but some users on the forum still say that for them when installing luci-app-pbr which depends on pbr, the opkg picks the pbr-iptables variant. I was trying to make the nft-capable variant called just pbr and be a default variant. Thanks!

hnyman · 2024-04-09T19:52:04Z

some users on the forum still say that for them when installing luci-app-pbr which depends on pbr, the opkg picks the pbr-iptables variant

Looking at the luci app, it actually depends on the "pbr-service", not "pbr" ???
https://github.com/openwrt/luci/blob/11819152de9751d1edbbeff0bb6d543217b5213a/applications/luci-app-pbr/Makefile#L12-L12
LUCI_DEPENDS:=+luci-base +jsonfilter +pbr-service

And all your variants provide "pbr-service".

I think that the most clear naming and dependency might be something like "pbr-nftables" and "pbr-legacy-iptables", and both of them would provide "pbr". and pbr-nftables would be the DEFAULT_VARIANT

I haven't worked with variants for a while, so my thoughts might be out of date...

It might also be something simple, like that your declaration of DEFAULT_VARIANT:=1 is before the VARIANT line in the definition of "pbr".

One clear example of working variant is libustreamssl:
https://github.com/openwrt/openwrt/blob/main/package/libs/ustream-ssl/Makefile

stangri · 2024-04-09T20:35:54Z

I think that the most clear naming and dependency might be something like "pbr-nftables" and "pbr-legacy-iptables", and both of them would provide "pbr". and pbr-nftables would be the DEFAULT_VARIANT

Thank you for your prompt reply. I really want to keep the name of the default package just pbr, hence the invention of the pbr-service as a dependency. I'll try moving the VARIANT definition up.

Looking at libustream it does provide differently named packages and there are no PROVIDES statements in there, so it's not clear what/how I can depend on from luci-app-pbr to install the default variant (by default) unless the iptables-variant is installed manually to satisfy the dependency.

Or do I misunderstand the intention of DEFAULT_VARIANT and it's used at the make time and not at the package install time?

My goals for user experience are:

If opkg install luci-app-pbr is ran, unless any variant of pbr is installed, the default (nftables-capable) variant is automatically installed as a dependency.
To have the default, nftables-capable, variant named simply pbr or have the pbr-nftables variant installed when opkg install pbr is ran.

I thought I achieved that with the Makefile in this commit, but evidently I was wrong.

Can you help me please?

stangri mentioned this pull request Mar 18, 2024

luci-app-pbr: update to 1.1.4-5 openwrt/luci#6999

Merged

systemcrash reviewed Mar 18, 2024

View reviewed changes

stangri force-pushed the master-pbr branch 3 times, most recently from b0522c3 to 44cf2f6 Compare March 19, 2024 00:11

stangri force-pushed the master-pbr branch from 44cf2f6 to 790753f Compare March 20, 2024 19:10

stangri merged commit 19bba88 into openwrt:master Mar 23, 2024
12 checks passed

stangri deleted the master-pbr branch March 23, 2024 01:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pbr: update to 1.1.4-5 #23684

pbr: update to 1.1.4-5 #23684

stangri commented Mar 18, 2024 •

edited

systemcrash Mar 18, 2024

stangri Mar 19, 2024

systemcrash Mar 18, 2024

systemcrash Mar 18, 2024

stangri Mar 19, 2024

pesa1234 commented Mar 18, 2024 •

edited

systemcrash commented Mar 18, 2024

systemcrash commented Mar 19, 2024

stangri commented Mar 20, 2024

pesa1234 commented Mar 20, 2024

systemcrash commented Mar 20, 2024

pesa1234 commented Mar 23, 2024 •

edited

stangri commented Apr 9, 2024

hnyman commented Apr 9, 2024

stangri commented Apr 9, 2024 •

edited

pbr: update to 1.1.4-5 #23684

pbr: update to 1.1.4-5 #23684

Conversation

stangri commented Mar 18, 2024 • edited

systemcrash Mar 18, 2024

Choose a reason for hiding this comment

stangri Mar 19, 2024

Choose a reason for hiding this comment

systemcrash Mar 18, 2024

Choose a reason for hiding this comment

systemcrash Mar 18, 2024

Choose a reason for hiding this comment

stangri Mar 19, 2024

Choose a reason for hiding this comment

pesa1234 commented Mar 18, 2024 • edited

systemcrash commented Mar 18, 2024

systemcrash commented Mar 19, 2024

stangri commented Mar 20, 2024

pesa1234 commented Mar 20, 2024

systemcrash commented Mar 20, 2024

pesa1234 commented Mar 23, 2024 • edited

stangri commented Apr 9, 2024

hnyman commented Apr 9, 2024

stangri commented Apr 9, 2024 • edited

stangri commented Mar 18, 2024 •

edited

pesa1234 commented Mar 18, 2024 •

edited

pesa1234 commented Mar 23, 2024 •

edited

stangri commented Apr 9, 2024 •

edited