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

Remove use of Environment Python #904

Open
wants to merge 5 commits into
base: beta
Choose a base branch
from
Open

Conversation

dthelegend
Copy link

@dthelegend dthelegend commented Apr 10, 2024

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!

Copy link
Collaborator

@saidsay-so saidsay-so left a comment

Choose a reason for hiding this comment

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

You're absolutely right, although it would be better to allow the packages' maintainers to specify the path because it isn't always guaranteed to be in /usr/bin.

howdy/src/compare.py Outdated Show resolved Hide resolved
howdy/src/cli.py Outdated Show resolved Hide resolved
howdy-gtk/src/init.py Outdated Show resolved Hide resolved
howdy-gtk/bin/howdy-gtk.in Outdated Show resolved Hide resolved
howdy/src/pam/main.cc Outdated Show resolved Hide resolved
howdy/src/bin/howdy.in Outdated Show resolved Hide resolved
@dthelegend
Copy link
Author

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 👍🏾

Thanks @musikid

Co-authored-by: Sayafdine Said <[email protected]>
@dthelegend
Copy link
Author

dthelegend commented May 20, 2024

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
Copy link
Author

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

@w-A-L-L-e
Copy link

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
Copy link
Author

dthelegend commented Jun 24, 2024

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
Copy link

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
Copy link
Contributor

principis commented Jul 24, 2024

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!


datadir = get_option('prefix') / get_option('datadir') / 'howdy'
py_conf = configuration_data(paths_dict)
py_conf.set('data_dir', datadir)

py = import('python').find_installation(paths_dict.get('python_path'))
Copy link
Collaborator

@saidsay-so saidsay-so Aug 11, 2024

Choose a reason for hiding this comment

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

Not sure about this, python_path could be another path that the packager would like to set?

Copy link
Collaborator

@saidsay-so saidsay-so left a comment

Choose a reason for hiding this comment

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

Except for the above, it should good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use Howdy with software managing multiple Python versions
4 participants