-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Conversation
@pprindeville
pprindeville
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand why a loop is used for a single argument...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a for loop if it's only 1ドル and not $@? Also there's no shift... Have a look at what net/strongswan/files/swanctl.init does with the same argument...
dhrm1k
commented
Jun 21, 2026
Not sure I understand why a loop is used for a single argument...
Ah yes. I was treating the value from config_get as a spaceseparated subnet
list and then looping over the words in 1ドル.
Would you prefer that I change this so the helper iterates over "$@", with the
call sites passing the subnet list as normal positional arguments?
That would make the intent clearer, and there would be no shift needed since
there is no leading argument to consume.
pprindeville
commented
Jun 21, 2026
@dhrm1k: Since it's a list shouldn't we use config_list_foreach in that case?
alvinstarr
commented
Jun 21, 2026
The left and right subnets are lists and I am not completely sure of the value searching for 0.0.0.0{/0}.
It seems to me just taking the value passed and using that would be the best.
Otherwise you should also check for an error like 10.250.0.0/16,10.250.1.0/24. or things like overlapping networks.
converting 0.0.0.0 to 0.0.0.0/0 may be a reasonable solution to people missing the /0.
Possibly a better check would be to disallow anything that does not have x.x.x.x/y.
pprindeville
commented
Jun 21, 2026
Possibly a better check would be to disallow anything that does not have x.x.x.x/y.
I could live with that.
Match only explicit default-route subnet tokens when normalizing leftsubnets and rightsubnets. The previous regex treated dots as wildcards. Values such as 10.250.0.0/16 and 10.0.0.0/8 were rewritten to 0.0.0.0/0. Fixes: openwrt/openwrt#23795 Signed-off-by: Dharmik Parmar <dharmikparmar2004@yahoo.com>
69bdf97 to
faa7df0
Compare
dhrm1k
commented
Jun 22, 2026
I changed this and pushed it again.
It now uses config_list_foreach for both leftsubnets and rightsubnets, so
each UCI list item is handled directly.
I kept the actual behavior change small. 0.0.0.0 is normalized to
0.0.0.0/0, 0.0.0.0/0 is left as-is, and other subnet values are passed
through unchanged.
lucize
commented
Jun 22, 2026
LGTM, will see what will be pushed first #29806
dhrm1k
commented
Jun 23, 2026
LGTM, will see what will be pushed first #29806
sure. thanks for looking into it!
@pprindeville
pprindeville
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Match only explicit default-route subnet tokens when normalizing
leftsubnetsand
rightsubnets.The previous regex treated dots as wildcards, so values such as
10.250.0.0/16and10.0.0.0/8could be rewritten to0.0.0.0/0.Fixes: openwrt/openwrt#23795
📦 Package Details
Maintainer: Lucian Cristian lucian.cristian@gmail.com (@lucize)
Description:
Fix the libreswan UCI init script so it only normalizes an explicit default
route subnet token,
0.0.0.0or0.0.0.0/0, instead of matching unrelatedprivate subnets that happen to contain
0.0.0.0in the string.🧪 Run Testing Details
Reproduced the issue on the VM with a temporary
/etc/config/libreswantunnelcontaining:
Before this change,
/etc/init.d/ipsec startgenerated:Also tested the new subnet matching helper in the VM shell:
Local checks:
✅ Formalities
If your PR contains a patch: