Page MenuHomeFreeBSD

bhyveload: limit rights on the dirfds we create
ClosedPublic

Authored by kevans on Jan 4 2024, 3:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 3, 9:12 PM
Unknown Object (File)
Sun, Nov 3, 9:11 PM
Unknown Object (File)
Sun, Nov 3, 8:55 PM
Unknown Object (File)
Sun, Nov 3, 8:47 PM
Unknown Object (File)
Wed, Oct 23, 7:16 PM
Unknown Object (File)
Wed, Oct 23, 7:16 PM
Unknown Object (File)
Wed, Oct 23, 7:16 PM
Unknown Object (File)
Wed, Oct 23, 6:34 PM

Details

Summary

In neither case do we need write access to the directories we're working
with; userboot doesn't support fo_write on the host device, and the
bootfd is only ever needed for loader loading.

This improves on 8bf0882e18 ("bhyveload: enter capability mode [...]")
so that arbitrary code in the loader can't open writable fds to either
of the directories we need to maintain access to.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kevans requested review of this revision.Jan 4 2024, 3:54 PM

Only time we need to write is to nextboot.conf... Since we don't support that from the userboot boot loader... this should be fine. We can punch a hole for nextboot.conf if we ever add that support.

This revision is now accepted and ready to land.Jan 4 2024, 5:43 PM
In D43315#987334, @imp wrote:

Only time we need to write is to nextboot.conf... Since we don't support that from the userboot boot loader... this should be fine. We can punch a hole for nextboot.conf if we ever add that support.

That's in the guest filesystem though, here we're dealing with the host filesystem. bhyveload+nextboot works fine today.

jrtc27 added inline comments.
usr.sbin/bhyveload/bhyveload.c
880

Missed a space here

883–884

Isn't this just err(1, "...");? I guess copied from the existing open code?

usr.sbin/bhyveload/bhyveload.c
880

Will fix in the morning, thanks

883–884

Yeah, I tried to keep it consistent with the rest of the area... somewhere around just after the getopt loop it goes from err -> perror + exit (though I wouldn't be surprised if one could provide counterexamples), we should really make it all consistent.

usr.sbin/bhyveload/bhyveload.c
746

CAP_SEEK is probably also worth having? (Can be added by changing CAP_READ to CAP_PREAD.)

882

Again, I'd suggest using CAP_PREAD to avoid surprises.

usr.sbin/bhyveload/bhyveload.c
746

Sure- I missed that we actually have an lseek() callback for host: fs; we don't seem to use it in my normal operation, but that doesn't mean it's unused.

re: /boot, it doesn't look like rtld will try to do any seeking on dlopen'd objects, but yeah- I don't see the harm in allowing it.