boltgolt / howdy

🛡️ Windows Hello™ style facial authentication for Linux
MIT License
5.75k stars 299 forks source link

Remove use of Environment Python #904

Open dthelegend opened 5 months ago

dthelegend commented 5 months ago

Hi,

I was running this on Arch Kernel 6.8.2 and I recently moved from the howdy AUR package to the howdy-beta-git package.

I use Pyenv to manage multiple python versions and howdy seems to always want to use the environment python, which I believe is incorrect behavior, especially from a security standpoint wherein (this example is admittedly quite stupid, but I hope it tracks) a malicious program could launch a subprocess with sudo that uses howdy and a modified environment to launch an executable of the attackers choice from the PAM context as long as it is named python.

This patch removes the use of the "!/usr/bin/env python3" and replaces it with "!/usr/bin/python3" and also changes the python3 executable to "/usr/bin/python3" in "compare.py".

I have patched and installed this on my local system and it seems to work fine.

I hope this helps!

dthelegend commented 5 months ago

Apologies I have not had a chance to review and test your changes, my laptop died last Monday and I'm still out of a laptop. I'd say that just on a skim your changes look good 👍🏾

dthelegend commented 4 months ago

Hi, my laptop is still dead and ASUS want to charge me £1200 so I'll be out of a laptop a bit longer. I'm making this PR from a backup PC, but I don't have any cameras, so whether this works is uncertain, but it compiles and I used grep to check the strings have been correctly replaced.

I HIGHLY ADVISE SOMEONE WHO CAN TEST IT PLEASE TEST IT. Otherwise I think this is ready to merge in.

EDIT: Also I use Arch (btw), so I have tested the makepkg and that works fine, but I haven't tested the Debian or Fedora package

dthelegend commented 3 months ago

Anyone been able to test this? (My laptop is still dead)

w-A-L-L-e commented 3 months ago

Don't think this is super important nor does it fix the actual vulnerability. You can still just copy over a malicious exe onto /usr/bin/python3 if you want to. Or even worse, just change a few lines in compare.py to always just return a good match.

But the reasoning is you don't have login nor file permissions to do so. More important is fixing the externally_managed_environment error (and not by removing or renaming EXTERNALLY-MANAGED file but having the right packages for your python code in apt available to install without getting that error).

dthelegend commented 3 months ago

My argument is mainly that calling a local environment version of python is susceptible in a scenario where for example a user has sudo privileges and a malicious program in user space with user privileges could point to a malicious user-owned python3 executable. This exploit would already require user privileges, but could be an escalation of privileges. It would also fix the issue where people with user-mode python shims or similar can't use Howdy and enable maintainers to better pin Howdy to a specific python and associated dependencies that is also packaged by them.

I will say I am not an official maintainer anywhere, so I wouldn't know if this is a huge issue, but I often see tickets about it when I scroll through.

w-A-L-L-e commented 3 months ago

Good point. However in current state you have to take a risk to mess up your main python dependencies due to that externally-managed error you get during the apt install (just pointing that this was the main reason for me not to use howdy yet on my own machine). And security wise I'd like to see some more parts converted to c or have some more binaries involved that are less easy to tamper. Same also applies to all python scripts and installed packages for howdy: they must be chowned/chmodded just right to make sure your same argument about the ENV here does not apply to a python source file used by howdy during login.

principis commented 2 months ago

I will say I am not an official maintainer anywhere, so I wouldn't know if this is a huge issue, but I often see tickets about it when I scroll through.

This is indeed an issue and package maintainers should patch it (which it is in my Fedora copr repo).

EDIT: Also I use Arch (btw), so I have tested the makepkg and that works fine, but I haven't tested the Debian or Fedora package

Should be fine for any distro!