Solar, this is the sfperms change I mentioned to you briefly awhile ago. Rather than saying "in my opinion" 50 times below, please assume that everything said below is my opinion and my idea of security implementation. Here is my logic behind it: First of all, there is no difference between handling SetGIDs and SetUIDs. The file has elevated privileges, and should not be edited by anyone. This logic is consistent with the UNIX logic that when you change the file owner of a SetUID file, it loses its SetUID bit. It follows that we should remove all read/write bits from any SetxID file. So I have grouped together the two blocks and handled them identically. Now, the find statement is ridiculous, but I believe it has to be that way. We do not care about files that are any combination of -x,+s. These are most likely the result of broken install scripts anyway. At most, we could produce a warning for this. I believe the sequence of ORs I use is the only way to properly catch all the possible setuid/setgid combinations that actually do something (i.e. that we care about). Finally, the `file..` check is to catch setuid/setgid scripts. Scripts will not run without read permission, so we cannot remove that. At most, we can remove the write permission, so there is an else clause for that. What do you think? I'm running this now, and I like the results. I wasn't sure what to set "component" or "keywords" to for this.
Created attachment 30197 [details] ebuild.sh.diff
Created attachment 30199 [details] ebuild.sh.diff previous diff was the wrong version
the script problem isn't solved by your file check. i can quite easily dump some text info a file, WITHOUT the '#!...' header, that that is a perfectly valid script. How about checking that the file is ELF type instead?
I just checked and I don't see where ebuild.sh is removing the +w bit anymore. (I'll check look harder in a few to verify). Anyway sure we could really do 'go-rw' on all setXid files. Bitwise permissions to only get the executable ones is not what the feature was designed to do or how it should behave. Think least privilege (ie what if file has a really bad set of perms say 7777) We also don't want to make sure it's ELF,COFF,a.out, script or otherwise. Only thing that matters is if it's type 'f'. If a script has a setXid permission then I for one want it to break. An example of this would be bug #42908 I'd much rather have a user/dev report it as a bug so that the .ebuild may be corrected. file(1) is also something I'd rather us not depend on for this feature.
To respond to the file command not detecting all scripts, I'm not sure scripts without the #!/path/to/shell are perfectly valid. Scripts without that are usually sourced. I understand if you do not want to use the file command in ebuild.sh, though. I believe there are definite legit uses of a setgid script. I'm not sure about a setuid one, but maybe. For instance, you could have a script to wall SMB messages from samba. This would need to be setgid tty if you had wall -s. I actually use a script like this, and I check at the beginning for $PPID = 1, which seems to be fairly safe. I don't think scripts with a +s are necessarily broken, and removing +r from those scripts will certainly break them. What is the intention of the permission check, if it's not for executable setXids? Do you want to make a separate check for 777 files? How do we know a 777 file is a mistake? Some ebuilds probably need this.
Scripts should never be suid this is a well documented fact. A distribution surely should not be dishing them out either. If you want to play risky games with your local security then that's fine for your local box but not acceptable for gentoo QA. I certainly don't want to change this functionality or see it changed in anyway that shifts it away from a least privilege model. If you know a single ebuild that will yields o+w or a seXid script then I want to know about it because that's a potential security risk and must be fixed, package.masked and or removed from the tree. In short the sfperms feature is perfect and does exactly what it does the way it's supposed to and is documented as doing so. The only change we could/should make if any is tie in full go-rw is if 'strict' was found in FEATURES. If you think otherwise then please propose a new feature.
I do not agree about the scripts, but it's not a big issue. I do not know of any examples. What about changing the find so it ignores non-executable setXids?
no.
Solar's disagreement sounds right to me.
