aniketpanjwani / chomper

Internet blocker for the Linux desktop.
https://addictedto.tech/chomper/
GNU General Public License v3.0
356 stars 43 forks source link

reset.py assumes iptables command is in PATH. #22

Closed sarg closed 6 years ago

sarg commented 6 years ago

Operating system: Debian Sid reset.py assumes that iptables is in the PATH. This is not always true, because it requires that USER has /sbin in PATH.

Stacktrace

``` root@thinkpad:/home/sarg# crontab -l 26 8 27 2 * env PATH=/home/sarg/devel/bin:/home/sarg/.local/bin:/home/sarg/devel/android-sdk-linux/tools:/home/sarg/devel/android-sdk-linux/platform-tools:/home/sarg/devel/bin:/home/sarg/.local/bin:/home/sarg/devel/android-sdk-linux/tools:/home/sarg/devel/android-sdk-linux/platform-tools:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/home/sarg/.antigen/bundles/zsh-users/zsh-completions/src /home/sarg/.local/share/virtualenvs/chomper-LZFI31mM/bin/python /home/sarg/devel/chomper/chomper/reset.py # Chomper timer root@thinkpad:/home/sarg# env PATH=/home/sarg/devel/bin:/home/sarg/.local/bin:/home/sarg/devel/android-sdk-linux/tools:/home/sarg/devel/android-sdk-linux/platform-tools:/home/sarg/devel/bin:/home/sarg/.local/bin:/home/sarg/devel/android-sdk-linux/tools:/home/sarg/devel/android-sdk-linux/platform-tools:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/home/sarg/.antigen/bundles/zsh-users/zsh-completions/src /home/sarg/.local/share/virtualenvs/chomper-LZFI31mM/bin/python /home/sarg/devel/chomper/chomper/reset.py Traceback (most recent call last): File "/home/sarg/devel/chomper/chomper/reset.py", line 15, in main() File "/home/sarg/devel/chomper/chomper/reset.py", line 10, in main utils.reset_nat() File "/home/sarg/devel/chomper/chomper/utils.py", line 22, in reset_nat exec_command('iptables -t nat -F') File "/home/sarg/devel/chomper/chomper/utils.py", line 13, in exec_command p = subprocess.Popen(shlex.split(command)) File "/usr/lib/python3.6/subprocess.py", line 709, in __init__ restore_signals, start_new_session) File "/usr/lib/python3.6/subprocess.py", line 1344, in _execute_child raise child_exception_type(errno_num, err_msg, err_filename) FileNotFoundError: [Errno 2] No such file or directory: 'iptables': 'iptables' ```

aniketpanjwani commented 6 years ago

Good point - thanks! One fix is to just change env PATH=$(PATH) to env PATH=$(PATH):/sbin. That should only alter the PATH variable in the context of the sudo command. However, I'm actually not sure now whether env PATH=$(PATH) is needed in the first place, since we have absolute paths to the interpreter and reset.py. It's pretty late here, so I'll think about what to do about this tomorrow, but happy to hear your thoughts too.

sarg commented 6 years ago

Another option is to use python library for interacting with iptables - https://github.com/ldx/python-iptables

btw, iptables could be located in /usr/sbin too

aniketpanjwani commented 6 years ago

Interesting library, but I'd rather not add on another dependency if it's not necessary.

Another idea: use which iptables to figure out where iptables is located on the system, parse out the directory, and make sure it's on path during the sudo command. Specifically, add the following new variable to the top of the Makefile

IPTABLES_PATH = dirname $(which iptables)

and then modify the reset task as follows

reset:
    sudo env PATH=$(PATH):$(IPTABLES_PATH) ${INTERPRETER}3.6m ${CURRENT_DIR}/chomper/reset.py
sarg commented 6 years ago

which works by searching current PATH. I suggest you to add /sbin:/usr/sbin to env and forget about this ticket until someone reports it again.

aniketpanjwani commented 6 years ago

Yeah you're right - which isn't necessary if it's already on PATH. Let's just do what you're suggesting - add /sbin:/usr/sbin to the env.

Could you make a PR? Thanks!