From: guido@trentalancia.net (Guido Trentalancia) Date: Wed, 26 Apr 2017 23:42:05 +0200 Subject: [refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!) In-Reply-To: References: <1492802281.4493.1.camel@trentalancia.net> <201704270220.27679.russell@coker.com.au> <201704270323.25612.russell@coker.com.au> <20170426175826.GB23409@julius> <1493229617.17296.1.camel@trentalancia.net> Message-ID: To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com Hello Christopher. You can possibly remove dac_override, provided that you grant dac_read_search. You should not remove sys_admin. However, there is very limited benefit in making such change, I would prefer to leave things as they are. You can only get some substantial gain by dontauditing sys_admin in the getty module, without loosing functionality, as already explained several times. Regards, Guido On the 26st of April 2017 23:04:35 CEST, Chris PeBenito wrote: >On 04/26/2017 02:00 PM, Guido Trentalancia via refpolicy wrote: >> This patch has already been submitted. >> >> Feel free to submit a patch to improve things as you say... >> >> On Wed, 26/04/2017 at 19.58 +0200, Dominick Grift via >> refpolicy wrote: >>> On Thu, Apr 27, 2017 at 03:23:25AM +1000, Russell Coker via >refpolicy >>> wrote: >>>> On Thu, 27 Apr 2017 02:32:05 AM Guido Trentalancia via refpolicy >>>> wrote: >>>>> Unfortunately, your sulogin patch didn't work, so it was not just >>>>> a matter >>>>> of unneeded permissions! >>>>> >>>>> You can check by yourself that it was missing critical >>>>> permissions while >>>>> granting unneeded ones... >>>> >>>> It worked for me last time I tested it on Debian. Maybe other >>>> distributions >>>> need different permissions. Maybe the Debian sulogin changed to >>>> require more >>>> permissions since the last time I tested it. But I don't submit >>>> policy based >>>> on what I imagine programs might do, it's based on what I observe >>>> them doing. >>> >>> I can confirm that cap_dac_override and cap_sys_admin arent needed >>> for sulogin in debian stretch >>> >>> https://www.youtube.com/watch?v=NBj2W7yiu_c >>> >>>> >>>>> Also, I have already stressed out several times that getty should >>>>> probably >>>>> run without the sys_admin capability. They didn't want to change >>>>> it, I am >>>>> not going to tell that again. >>>> >>>> As the previous discussion that I linked to showed there was a >>>> situation where >>>> a character could be lost if that permission wasn't granted. I >>>> expect that >>>> getty could be changed to address that issue. But I also recall >>>> that there >>>> was another issue which I couldn't get the details of in 10 minutes >>>> of >>>> Googling. >>>> >>>>> Feel free to submit your sys_admin capability patch for getty, >>>>> sulogin or >>>>> both. Consider, I have not tested other variations for sulogin, I >>>>> consider >>>>> the change of minor importance compared to this patch. >>>> >>>> As I have stated several times sulogin has a sole purpose of >>>> running a shell >>>> with ultimate privileges and therefore I think that restricting >>>> it's access is >>>> futile. > >Ideally, I'd like least privilege across the board; however, I also >tend >to agree with Russell in this case. I'd rather spend more time on >domains where risk is higher. If sulogin is somehow compromised, the >system is in serious trouble, but the risk of that is pretty low. > >That being said, I'm still happy to take further patches on this, if >you >can agree on the changes (I can't test all distros to make sure it >works). I'd prefer to avoid adding and removing the same permissions >repeatedly.