[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value mbasti-rh commented: """ Closing this PR, how to handle environment variables must be discussed and designed first. """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257905927 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ +1 on using absolute paths. I don't recall any cases where KRB5_TRACE was needed so is this a theoretical use case or an actual one? Yes, LD_PRELOAD or PYTHONPATH can be tweaked but this just proves my point: the environment is untrustworthy. """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257867492 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value mbasti-rh commented: """ https://fedorahosted.org/freeipa/ticket/6449 """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257847185 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value pspacek commented: """ The approach with wiping env adds another layer of problems, e.g. inability to use `KRB5_TRACE` environment variable for debugging etc. IMHO we should use absolute paths whenever we call an external program and let the env be. If an attacker is controling env the game is already over. He could mess with `LD_PRELOAD` or any other other current or future sensitive variables. """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257838182 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ This isn't about replacing existing binaries, it's about putting binaries into unexpected places that are in the default PATH (e.g. ~/bin or /usr/local/bin). PATH cannot be overridden by an attacker without making code changes, in which case it's already game over (or it shouldn't, I didn't look for every execution of ipautil.run() where env is passed in. I don't disagree on being platform dependent. As for documentation, it just got missed. It's not an excuse, just the reality. It is generally accepted best-practice to not trust user input, including environment variables. See https://www.securecoding.cert.org/confluence/display/c/ENV03-C.+Sanitize+the+environment+when+invoking+external+programs This isn't followed completely, but at least the environment by default is wiped and PATH is controlled for the most part. Originally the commands were called explicitly, e.g. /usr/kerberos/sbin/kadmin.local, but because of the Fedora 14 issue we had to rely on PATH (see d0ea0bb63891babd1c5778df2e291b527c8e927c). """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257667140 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value mbasti-rh commented: """ > PATH is untrustworthy because there is no knowing what is in it, or the > order. It could easily have /usr/local/bin first and some rogue version of a > program installed there, or it could have something in ~/bin. Calling exec() > is dangerous by its very nature so we opted to be paranoid. > /usr/bin is untrostworthy in the same way, you dont know if an attacker changed some binary files, should we have fingerprints and check before exec? AFAIK path is the standard way how to say programs where should check for binarries if they are installed in nonstandard directory In case that enviroment variables are really considered to be an security risk in a way you are saying, then I have bad news: - our custom path can be overriden by attacker - this kind of attack can be currently done directly from python we don't need anything else in IPA, so our ipautil.run() cannot save users - you can easily DOS a user of IPA And this should be platform dependent, so we should move path to ipaplatform > Your archaeology is right, this wasn't exactly documented. Perhaps it was > discussed on IRC in relation to the bug but I remember talking to Simo about > this. It wasn't documented. That is not nice if this is a security feature """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257663432 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ PATH is untrustworthy because there is no knowing what is in it, or the order. It could easily have /usr/local/bin first and some rogue version of a program installed there, or it could have something in ~/bin. Calling exec() is dangerous by its very nature so we opted to be paranoid. Your archaeology is right, this wasn't exactly documented. Perhaps it was discussed on IRC in relation to the bug but I remember talking to Simo about this. """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257655506 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value mbasti-rh commented: """ Can you elaborate more about that attack? Do you have any links to share? If an attacker has permission to set a user environment variables, IMO the user has already lot of problems and it is too late to save that situation. I did git archaeology and this was the commit where it was added, so it was hard to find reason why it was added. """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257640644 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ NACK. I'd be fine with changing the PATH to remove cruft but the primary purpose is to prevent an attacker from providing their own PATH with unknown executables. For those few places where one must control PATH then env can be (and is) passed in. No ticket? """ See the full comment at https://github.com/freeipa/freeipa/pull/204#issuecomment-257628641 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
