Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support dumping seccomp on aarch64 #125

Merged
merged 1 commit into from
Aug 28, 2020
Merged

Conversation

saagarjha
Copy link
Contributor

This adds support for dumping seccomp BPF on aarch64–doing it by hand for DEF CON is fine once, but I'd rather not have to do it again :) The setup I used to test this is extraordinarily janky (CTF challenge-provided binary/kernel running in QEMU using initramfs+a homegrown rootfs that has literally every single dependency in it available offline, since I can't figure out how to set up networking). Seems to work on some simple binaries I tested it against, but I couldn't figure out how to get the actual test suite running so if you have guidance there it would be much appreciated.

@david942j
Copy link
Owner

Thanks for this contribution! It basically looks fine but the tests failed with unclear reasons (I would guess it's related to the ptrace change)

Also please add tests to ensure the coverage reaches 100% ;)

ext/ptrace/ptrace.c Outdated Show resolved Hide resolved
ext/ptrace/ptrace.c Show resolved Hide resolved
ext/ptrace/ptrace.c Show resolved Hide resolved
@saagarjha
Copy link
Contributor Author

Thanks for looking at this! A couple of questions above, but regarding testing: I'm not a Rubyist by any means so I'm mostly just stumbling around trying things while working with this. To test locally I have been running bundle install and then bundle exec rake compile, then running bundle exec seccomp-tools dump on some test binaries. Is this the intended way to work with the project? If not, how should I be doing so? If I run bundle exec rake on my x86_64 machine (which I think runs the tests?) it hangs so I'm not sure if that is the right way to do it.

Regarding adding tests: I tried this against a binary from DEF CON and this example I found online, tweaked slightly. For the latter I changed out the architecture when installing the filter and changed it block writev, which is what musl uses for printf. For the former, I need to do a more roundabout technique because it forks a thread and applies the filter to that, so I grab the subtask ID from procfs and run seccomp-tools on a PID after launching it. Oh, and this is all running in the sketchiest qemu build ever–I'm not sure it is really fit for CI, if that's your goal. Again, your thoughts are very welcome :)

@david942j
Copy link
Owner

david942j commented Aug 19, 2020

Yes bundle exec rake on an x86_64 machine is the expect way to run tests. I don't know why it hangs to you though, but indeed the tests hang on travis CI as well (https://travis-ci.com/github/david942j/seccomp-tools/jobs/372610410).
This is the latest build on master branch (4 days ago) and it works fine, so I assume the hang is caused by your patches.

I see. So for now you can simply add tests against the functions don't need an ARM binary, such as disasm and emu.
Only the function dump needs the binary to be execute, I can take care of that (will add "arm64" config to travis CI) after this PR.

@saagarjha saagarjha force-pushed the master branch 8 times, most recently from 1c3ae83 to ecb29a7 Compare August 27, 2020 02:11
@saagarjha
Copy link
Contributor Author

Turns out Yama was being too mindful and rejecting trace requests :/ Then I fixed some of the offsets for i386, since Linux apparently uses a 4 byte pointer size with PTRACE_GETREGSET but native pointer size for PTRACE_PEEKUSER

I've added comments and checks for the things you've mentioned in ptrace.c, and there's a test for disasm and emu based off of the bdooos challenge from DEF CON–the binary for which I've committed as well. Let me know if there's anything else you'd like me to add.

ext/ptrace/ptrace.c Show resolved Hide resolved
spec/util_spec.rb Show resolved Hide resolved
ext/ptrace/ptrace.c Show resolved Hide resolved
As part of this change, fix some non-general code for detecting
architectures and switch to move off of PTRACE_GETUSER, which is not
functional on aarch64.
@saagarjha
Copy link
Contributor Author

Done, done, and done :)

@david942j
Copy link
Owner

Merged, thanks for the contribution!

@david942j david942j merged commit 8594353 into david942j:master Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants