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.

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

iptables: fix helper script #4324

Merged
merged 1 commit into from Apr 17, 2020
Merged

iptables: fix helper script #4324

merged 1 commit into from Apr 17, 2020

Conversation

MilhouseVH
Copy link
Contributor

@MilhouseVH MilhouseVH commented Apr 15, 2020

I'm pretty sure check_tether() has never worked correctly.

The commands:

$CONNMANCTL technologies | grep -e -A5 technology/wifi -e 'Tethering = True'`"
$CONNMANCTL technologies | grep -e -A5 technology/ethernet -e 'Tethering = True'`"

result in the following errors being logged:

Apr 15 04:13:04 NUC systemd[1]: Starting IPTABLES Packet Filtering...
Apr 15 04:13:04 NUC iptables_helper[669]: grep: technology/wifi: No such file or directory
Apr 15 04:13:04 NUC iptables_helper[767]: grep: technology/ethernet: No such file or directory
Apr 15 04:13:04 NUC iptables_helper[768]: Error 'tether': Invalid argument
Apr 15 04:13:05 NUC iptables_helper[836]: Error 'tether': Invalid argument
Apr 15 04:13:05 NUC systemd[1]: Finished IPTABLES Packet Filtering.

as grep -e -A5 technology/wifi -e 'Tethering = True' is attempting to open the file technology/wifi due to -A5 being interpreted as the pattern for -e, causing technology/wifi to be interpreted as the file to be searched.

Furthermore, once the grep statement is corrected by moving the -A5 argument before -e, the grep statement doesn't actually work as intended.

It appears the script is attempting to identify the Tethering status of a specific technology (wifi and ethernet), and has been written under the assumption that multiple -e patterns must all be matched to return any result. However, this is not true, at least not with Busybox grep, which will in fact match either of the patterns, meaning that a result would be returned for either technology whether or not it has Tethering capability. For example:

NUC:~ # connmanctl technologies | grep -A5 -e technology/wifi -e 'Tethering = True'
/net/connman/technology/wifi
  Name = WiFi
  Type = wifi
  Powered = True
  Connected = False
  Tethering = False
NUC:~ # connmanctl technologies | grep -A5 -e technology/ethernet -e 'Tethering = True'
/net/connman/technology/ethernet
  Name = Wired
  Type = ethernet
  Powered = True
  Connected = True
  Tethering = False

The grep command also assumes the Tether status will be present in the first 5 lines after the technology, which may be true today, but not necessarily true with a future version of connman which might add another capability bumping Tethering down a line.

To resolve all the issues, I've added a small awk program that now reliably extracts the capabilities of the requested technology which can then be parsed reliably.

I suspect this should be backported.

Also: anyone know what #!/bin/sh - is meant to be? I'm assuming it's a typo.

@MilhouseVH
Copy link
Contributor Author

Backport cherry-picked into #4247.

@MilhouseVH MilhouseVH requested a review from chewitt April 16, 2020 20:56
Copy link
Member

@chewitt chewitt left a comment

Choose a reason for hiding this comment

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

LGTM

@chewitt chewitt merged commit 677f569 into LibreELEC:master Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants