arminbiere / kissat

MIT License
452 stars 85 forks source link

test fails if too broad privileges (e.g. in root) #19

Closed lordlouckster closed 2 years ago

lordlouckster commented 3 years ago

file '/etc/shadow' determined to be readable unexpectedly file '/etc/passwd' determined to be writable unexpectedly

arminbiere commented 3 years ago

The simple fix is to not compile and particularly test it as root which is hazardous I think. What is the reason you want to do that?

lordlouckster commented 3 years ago

Why do you think it's hazardous?

arminbiere commented 3 years ago

One should always be careful in running third-party programs as root. But again, what is the reason you want to do that?

lordlouckster commented 3 years ago

Why should you block it completely? Just put a warning

arminbiere commented 3 years ago

You did not answer my question, why you want to run the tests as root. Still I am happy to explain what this test is about. This is part of testing the low-level access functions for files, in particularly here trying to write to an existing file to which you do not have write resp. read access. This test makes it easier to port those functions. The two files do have the right privileges for this test (unless you run the test as root which you should not). Otherwise to test them one would need to set up files with the right privileges (and remove them too after the test finishes), which will rely on other low-level functionality being correct (changing privileges), which you would need to port first.

aytey commented 3 years ago

An outsider opinion (I just follow kissat): as someone who has to build/install packages for other people/servers, it isn't unusually for me to build/test as root before doing a system-wide install. Yes, one could do the build as non-root and then do something like sudo make install, but in a number of instances, I just go do sudo -i and get on with it.

in particularly here trying to write to an existing file to which you do not have write resp. read access.

Couldn't the test create the file and chmod away the permissions before running the test?

arminbiere commented 3 years ago

Yes, that is what I had before, but this part is also not really easy to port and the main point of testing this functionality is to help porting it (and you do not want to make it harder by requiring to first port the test code). Then there is the issue of removing the generated files, which requires additional steps (and a strategy when to do that). It is way simpler to use '/etc/{shadow,passwd}' (if they exist) assuming that you do not want to run 'make test' as root (which I still do not understand why you want to do that).

aytey commented 3 years ago

You already have makefile.in calling-out to some shell scripts, and such, so let's assume you're targeting POSIX. Couldn't make test, touch these files, chmod them -r and then remove them afterwards?

arminbiere commented 3 years ago

First, it will not be the same, as those files will still be owned by you. Second, your build directory will then contain non-readable or even non-writable files (at least until 'make clean'). Third, this will fail for root too ;-)

$ sudo -i
# cd /tmp
# echo hello > world
# chmod 000 world
# ls -l world
---------- 1 root root 6 Okt 28 13:55 world
# cat world
hello
# rm world
aytey commented 3 years ago

Ha! I learnt something today: turns out you can't chmod 000 a file for root :D