omar-polo / gmid

a Gemini server
https://gmid.omarpolo.com
ISC License
102 stars 7 forks source link

sandbox.c causes compilation failure on Linux in some cases #4

Closed mxkrsv closed 3 years ago

mxkrsv commented 3 years ago

On musl libc on archs different from x86_64 and aarch64 sandbox.c causes errors when compiling, probably due to musl not having proper implementation of __NR functions for them. Having a build-time option to disable sandboxing would be nice.

omar-polo commented 3 years ago

Yes, sorry for that; I've only tested on amd64 and aarch64 so far. Adding a build-time option to disable sandboxing would indeed be easy, but I would prefer to have it working whenever possible.

I'll try to build on x86 to hopefully fix that (from the failure it seems that on x86 the actual syscalls are different).

mxkrsv commented 3 years ago

Thanks. On some arches it fails because of missing SECCOMP_AUDIT_ARCH defines. I found the most complete (and the most boilerplate) solution which contains all Alpine's arches here.

omar-polo commented 3 years ago

I updated the seccomp filter and added a --disable-sandbox flag for the configure script (that I dislike so it isn't advertised anywhere, but if it's necessary, it's there)

Thanks for liking that bit from dhcpcd, it was really helpful! :)

make regress now passes on alpine x86 and amd64, and on void aarch64 too, so hopefully I haven't screwed anything. 137def5ff4c0f9720391ca88191cf9fee6d8ae9a and 8bb8cf2ad488151879b1d7e5ec7436d38553b1b5 should apply cleanly on top of the 1.6.1 but I can tag a 1.6.2 if it's easier for you, just let me know.

mxkrsv commented 3 years ago

Thanks, now it compiles on all architectures (except riscv64, no gitlab runner to test yet) even without --disable-sandbox. However, patch complains about ChangeLog in 137def5, so tagging would be cleaner.

omar-polo commented 3 years ago

I haven't thought about the ChangeLog file, sorry! I tagged the 1.6.2: it's the 1.6.1 with 137def5 and 8bb8cf2 on top.

I'm not familiar with how alpine infrastructure works, so I hope you'd excuse this probably stupid question, but the regression suite (make regress) is ran too as as part of building a package?

Thanks for testing!

mxkrsv commented 3 years ago

Yes, make regress is done in check(). It passes on all architectures except ppc64le, where it hangs on ./runtime somewhy.

omar-polo commented 3 years ago

Thanks for the clarification! :)

The hangs smell like a possible seccomp failure (where the daemon got killed but the client is still waiting for the reply), even though one hour of wait is still probably a bit too much. I'm going to emulate a ppc64 and see what's going on.

(also adding a timeout to the regress suite wouldn't harm, sorry for hanging your builds!)

mxkrsv commented 3 years ago

Thanks for efforts

omar-polo commented 3 years ago

No efforts at all -- I got to play with (virtualized) ppc64 :D

The problem was that ppc64le was matched as ppc64 (big endian). The seccomp filter would then presume that the current architecture is not the one the program was built on and so disallow any syscall. It should be fixed as of b24021d, on my vm make regress passes 🎉

mxkrsv commented 3 years ago

Thanks, with this patch it works perfectly. Alpine package will probably be merged soon.