sio2project / sio2jail

A tool for supervising execution of programs submitted in algorithmic competitions
MIT License
30 stars 10 forks source link

Removed syscalls ```open``` and ```openat``` from policy defined in addExecutionControlRules #45

Open mikimasn opened 7 months ago

mikimasn commented 7 months ago

Syscalls open and openat were handled two times.

Once without any restrictions in https://github.com/sio2project/sio2jail/blob/b5903c6d1fb04901b5ac025eaf97b84cedf27608/src/seccomp/policy/DefaultPolicy.cc#L45-L47

And the second time with a check to enforce read-only mode on the file system https://github.com/sio2project/sio2jail/blob/b5903c6d1fb04901b5ac025eaf97b84cedf27608/src/seccomp/policy/DefaultPolicy.cc#L184-L192

The first policy was more permissive and made the second one useless(it always allowed syscall open without checking access mode) so I removed it.

Wolf480pl commented 7 months ago

Looks like the duplicate open/openat were added by https://github.com/sio2project/sio2jail/pull/27/files?diff=unified&w=1

I looked through OI admins' internal chat logs and it looks like we allowed those syscalls in response to python3.9+numpy having an issue:

intercepted forbidden syscall openat(257) (4294967196, 47589183160736, 524481, 420, 0, 3346281667394566245)

which means

openat(-100, somepointer, 0o2000301, 0o644)

a.k.a.

openat(AT_FDCWD, somepointer, O_CLOEXEC | O_EXCL | O_CREAT | O_WRONLY, 0644 /* rw-r--r-- */)

but that error was difficult to reproduce (it only happened on old kernels)

In any case, I think the right thing to do would be to return either EPERM or EROFS when someone's trying to open a file for writing - the libraries that do it implicitly hopefully handle that error gracefully.

Unfortunately this means when a contestant's program explicitly tries to create a temporary file, we can't explicitly report that as a Rule Violation, instead the program will probably fail to handle the error and the contestant will see a generic Runtime Error. But that was already an issue with the changes introduced in #27 so I guess we'll have to live with it.

When I have time, I'll try to reproduce the error with python3.9+numpy.

Meanwhile, you can change this part https://github.com/sio2project/sio2jail/blob/b5903c6d1fb04901b5ac025eaf97b84cedf27608/src/seccomp/policy/DefaultPolicy.cc#L184-L192 to return EROFS or EPERM when open is not read-only, maybe similar to this https://github.com/sio2project/sio2jail/blob/8e65e3114acc8ba3d3daff3635c46ad933fa9a6c/src/limits/ThreadsLimitListener.cc#L32-L39 so that we don't have to worry which rules apply first.

mikimasn commented 7 months ago

Now attempts to open a file in write mode when read-only mode is enforced fail with EROFS error.

mikimasn commented 7 months ago

Sorry for taking so long to make changes but I was participating in the competition and didn't have time to do it.

Wolf480pl commented 7 months ago

no worries, it'll take me long to test it anyway :P

mikimasn commented 6 months ago

How is the testing going?