howdy
howdy copied to clipboard
boltgolt
Use environnement variable instead hardcoded path
This is a PR related to https://github.com/boltgolt/howdy/issues/688
- The file
/src/shell/howdy.shand/src/shell/howdy.cshcontain the variableHOWDY_LIBwhich is not yet initialized - During the execution of the post install script, the variable is initialized
- All files which use the lib path have been modified (expect installer files)
- I have started to setup the v3 installers for Fedora and Arch (including initialization of environment variable)
- I have updated the installer for Debian in order to initialize environment variable
Isn't it risky though to use an environment variable to get the library path? An attacker would just have to override the variable to point to a script which would always accepts authentication.
Isn't it risky though to use an environment variable to get the library path? An attacker would just have to override the variable to point to a script which would always accepts authentication.
Is the PAM module is executed at root level ? If this is the case, the attacker must elevate his privileges in order to modify the env variable. And then he could also modify the behaviour of PAM.
If PAM is run at user level, indeed, this is not a good idea as the variable can easily be overwritten.
Workflow test fixed in 0bc0c2c3681b626805365fdacd58002d5b60e6e0
Edit: there is a way to know if the test will pass before pushing on GitHub ?
Isn't it risky though to use an environment variable to get the library path? An attacker would just have to override the variable to point to a script which would always accepts authentication.
Is the PAM module is executed at root level ? If this is the case, the attacker must elevate his privileges in order to modify the env variable. And then he could also modify the behaviour of PAM.
If PAM is run at user level, indeed, this is not a good idea as the variable can easily be overwritten.
Yes, I was talking at user level, how much would that be a risk compared to the big advantages that your approach provides?
Workflow test fixed in 0bc0c2c
Edit: there is a way to know if the test will pass before pushing on GitHub ?
If you're talking about the workflow, this is just a linter, which you can run locally with ninja clang-tidy after setting up the build folder with meson.
But I mean, is there any reason to talk about PAM at user level? Since the module is (I think) executed at root level. And because howdy's env variable is in read only for the user, an adversary can't modify it without elevate its privilege. And therefore, if the adversary is able to do this, it doesn't make sense to talk about env variable.
While doing some research I found the existence of pam_getenv() which allows doing exactly what getenv does but only in the scope of variables defined for PAM. Therefore, we could use this feature ?
Hmm, you're right then, uses of the module at user level can just be dismissed. The module is effectively mostly ran at root level for authentication, e.g. with sudo, GDM, etc.
The file
/src/shell/howdy.shand/src/shell/howdy.cshcontain the variableHOWDY_LIBwhich is not yet initializedDuring the execution of the post install script, the variable is initialized
Instead, you should use meson's configure_file() to define that path inside e.g. the *.cc and *.py files while setting up meson, by replacing @HOWDY_LIB@ in the files with the correct path during the build.
There's no need to add extraneous environment variables to the entire operating system (including all processes for all users).
Instead, you should use meson's configure_file() to define that path while setting up meson, and replacing @HOWDY_LIB@ in the files with the correct path during the build.
Not only main.cc needs to access to the Howdy libs. Moreover, it could allow other projects to easily integrate Howdy. For example with the howdy_gtk package, several Python files access to the howdy libs.
I had already clarified that in an edit, that yes I am in fact referring to both the .cc and the .py files. This does not change what I said.
Yes it is a good idea to use meson, but howdy_gtk which is another package needs to access to the howdy's files.
Here I have only created one variable, but we could go into more detail. For example, Fedora Flatpak based distributions (like Fedora Silverblue or Core), cannot use howdy properly because of some paths used to store config file, models, etc. Whereas the packager or even the user (sudo) could simply change the variable and everything would be fixed.
At the moment, I see this more as an advantage than a disadvantage. Especially because I don't see what the disadvantages are ? The security issue mentioned for the PAM module is no longer an issue because pam_getenv can be used instead.
Although, I agree that if we can do without using environment variables, we might as well do it.
but
howdy_gtkwhich is another package
... can be configured with -Dpam-securitydir=/lib/security, just like this package can be.
For example, Fedora Flatpak based distributions (like Fedora Silverblue or Core), cannot use howdy properly because of some paths used to store config file, models, etc.
And how does Fedora Flatpak use Linux-PAM itself?
... can be configured with -Dpam-securitydir=/lib/security, just like this package can be.
But how it can know where is installed Howdy because it is installed before. Ok, the maintainer of another project must inspect what the maintainer of the main package has done. Especially under Fedora distributions based, it is not really the maintainer who chooses precisely where the package is installed, because the installation file uses macros that represent locations which are determined when the package is built for the specific distro.
And how does Fedora Flatpak use Linux-PAM itself?
Like Fedora Workstation I guess, i.e. more or less like all other Linux distributions.
I'm not against the idea of doing something different, I was just proposing this env solution to solve the problems Howdy faces because it is used on so many distributions. And that, possibly, it could be used in the future by many other projects. But I just have a little experience in Linux application packaging. What I propose can indeed be something that is rarely done. Although, just because something is rarely done doesn't mean it's bad.
But concerning main.cc and all path used in the main howdy package, it clearly seems to be a good idea to use meson's configure_file().
Consequently, we could only keep the use of environment variables for howdy_gtk's files?
But how it can know where is installed Howdy because it is installed before. Ok, the maintainer of another project must inspect what the maintainer of the main package has done. Especially under Fedora distributions based, it is not really the maintainer who chooses precisely where the package is installed, because the installation file uses macros that represent locations which are determined when the package is built for the specific distro.
Because this isn't a choice that howdy is allowed to make, either.
It's a choice the distro made which applies to all PAM modules, so both howdy and howdy-gtk need to have the same macro logic as a bunch of other packages.
The thing is that this actually isn't a big deal. Simply configure both howdy and howdy-gtk with -Dpam_securitydir=%_pam_moduledir as per https://src.fedoraproject.org/rpms/pam/blob/rawhide/f/macros.pam
And really...
the maintainer of another project must inspect what the maintainer of the main package has done
Why is this a bad thing? :)
Why is this a bad thing? :)
An update of the main package could break the others
An update of the main package could break the others
An update of the main package to what? The pam package changing its security directory? This would be a cascading issue across a bunch of packages at the core of the distro.
But regardless, I still do not see the problem. This is the job of a package maintainer -- to maintain things and ensure they are integrated correctly.
Perhaps you should submit a feature request to the Linux-PAM project asking for the security directory to be defined as a pkg-config variable in pam.pc, so that projects can just look that up.
The problem is not about where is installed the howdy's pam module (and anyway we don't need to know), but about where are compare.py, models/, config.ini and dlib-data/
I see that we have trouble to agree. Let the v3 progress, and we will see if using environment variables is necessary or not
The problem is not about where is installed the howdy's pam module (and anyway we don't need to know), but about where are
compare.py,models/,config.inianddlib-data/
Okay, but that is always inside the security directory, isn't it?
If you're concerned that howdy may change from reusing the security directory, to using its own directory elsewhere, then the problem is still easily solved.
Just install a howdy.pc file containing a variable howdydir=/lib/security/howdy, and howdy-gtk can look up this pkg-config dependency at build time to find out which directory to use.
Updates to howdy (or configuration options to howdy) that change this location become reflected in the pkg-config file, NOT in a global systemwide environment variable, and rebuilding howdy-gtk will cause it to pick up the new location.
I see that we have trouble to agree. Let the v3 progress, and we will see if using environment variables is necessary or not
No, this is wrong, there are a variety of possible good solutions and none of them involve using an environment variable global to every. single. process on the entire computer. Doing it wrong because it requires a Linux distro packager to do what Linux distro packagers are there to do, is not a worthwhile tradeoff. Doing it wrong and inviting security flaws due to pam environment variable leakage is not a good idea. And doing work at runtime which can perfectly well be done at build time is inelegant.
What we are disagreeing about here is that I say what you want to do is possible without ~~bugs~~ environment variables, and you're saying that you want to use environment variables because it's easier and already implemented. :)
I also want to note that PAM does not specify what user it will run modules at. Some lockscreens try to run it as the non-privileged user they are trying to authenticate. This currently crashes Howdy.
In the future i want to set the setuid flag to force Howdy to run as root at all times.
Howdy IS going to be split into more appropriate folders (see #649 and #669). I think hardcoding those folders is probably fine, as their location really should not be changed.
I like the idea of using the env to know where to look for the python script, but i see some very valid points against it here too. Either way let's not get too up in arms in here, 3.0.0 development will continue for quite a bit more and if it comes in handy we will use this PR.
@eli-schwartz I've always taken into account what you say, several times I've clearly said many times that using meson to setup the paths of the main package howdy and everything that it contains (cli, pam, ...), is a great idea. By the way, thanks for introducing me to this solution, it will be useful to me in the future. I also said that if it is not necessary to use env var, then let's not do it.
As @bogolt says, if we indeed manage to use /etc/ to store modifiable files, we also do not need to use env var, since this directory has the same usage on all (I guess) distributions.
Now, if what's preventing us from using environment variables is revealing the location of files, the find command does that very well too.
On the other hand, I hadn't thought about it, but indeed, as mentioned by @musikid and @boltgolt, if the module is executed in user mode, then you should absolutely not use global environment variables. Instead, we should use local variable to PAM using pam_getenv() and /etc/security/pam_env.conf in order to avoid security issue. But anyway, if we use meson to determine the path at compile time, it is not necessary.