Page MenuHomeFreeBSD

Allow secure-netboot
ClosedPublic

Authored by sjg on Jul 6 2025, 11:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 12, 11:43 PM
Unknown Object (File)
Sun, Oct 12, 10:05 AM
Unknown Object (File)
Sun, Oct 12, 2:33 AM
Unknown Object (File)
Fri, Oct 10, 5:41 AM
Unknown Object (File)
Fri, Oct 10, 5:41 AM
Unknown Object (File)
Fri, Oct 10, 3:43 AM
Unknown Object (File)
Thu, Oct 9, 8:08 PM
Unknown Object (File)
Thu, Oct 9, 8:07 PM
Subscribers

Details

Summary

When doing file verification, tftp needs to be able to handle multiple
open files concurrently.
We also need tftp_stat() to provide useful values for st_dev and st_ino.

Allow an architecture to define NETPROTO_DEFAULT.
The default is NET_NFS for backwards compatability.

In net_parse_rootpath() fix parsing of
<scheme>://<ip>[:<port]/<path>
and ensure we return INADDR_NONE unless we successfully
parsed an addr, so we don't end up clobbering rootip obtained
from bootp().

In install() if proto has not been determined, but
netproto is NET_TFTP set proto to tftp_fsops.

Put a common DEBUG_PRINTF impl in stand.h so we can over time have
more consistent approach to debugging bits of the loader.

Sponsored by: Juniper Networks, Inc.

Diff Detail

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

Event Timeline

sjg requested review of this revision.Jul 6 2025, 11:54 PM

Can you do the debug level stuff first so we get that out of the way? There's also a lot of different fixes bundled together which makes me super nervous for future bisecting...

stand/common/dev_net.c
311

This is the only non debug level change. Please do the debug level stuff as a separate commit.

stand/libsa/open.c
164 ↗(On Diff #158075)

Ugg why? Seems like an extreme layering violation... this whole force it so do devopen is super weird too. Also why? I really hate hacks like this... almost all the other ones like it have bit my a** at some point...

In D51187#1170063, @imp wrote:

Can you do the debug level stuff first so we get that out of the way? There's also a lot of different fixes bundled together which makes me super nervous for future bisecting...

See D51269 since it is purely DEBUG_PRINTF (as used by libsecureboot) I added the fixes for pkgfs.c and bootp.c

Rebase after commit DEBUG_PRINTF

stand/libsa/open.c
164 ↗(On Diff #158075)

that is a good question, need to do some archeology this was originally a marcel change that got merged in 2017

Will see what I can find, I can confirm PXE boot breaks without this:

Testing hash: sha256                            Passed
Testing hash: sha384                            Passed
Testing verify certificate: EngineeringEcCA     Passed
Verified /boot/../manifest signed by PackageDevelopmentECP256_2021
Verified /boot/boot.4th
Junos PXE installer bootstrap for EFI
Trying: /935db760-789f-11eb-9f46-6c0b84655934.tgz...
Trying: /58-9c-fc-00-74-c6.tgz...
Trying: /C0A84544.tgz...
Trying: /C0A8454.tgz...
Trying: /C0A845.tgz...
Trying: /C0A84.tgz...
Trying: /C0A8.tgz...
Trying: /C0A.tgz...
Trying: /C0.tgz...
Trying: /C.tgz...
Trying: /default.tgz...
Verified /manifest signed by PackageDevelopmentECP256_2021

can't load 'kernel'

Type '?' for a list of commands, 'help' for more detailed help.

vs

Setting currdev to net0:
Testing hash: sha1                              Passed
Testing hash: sha256                            Passed
Testing hash: sha384                            Passed
Testing verify certificate: EngineeringEcCA     Passed
Verified /boot/../manifest signed by PackageDevelopmentECP256_2021
Verified /boot/boot.4th
Junos PXE installer bootstrap for EFI
Trying: /935db760-789f-11eb-9f46-6c0b84655934.tgz...
Trying: /58-9c-fc-00-74-c6.tgz...
Trying: /C0A84544.tgz...
Trying: /C0A8454.tgz...
Trying: /C0A845.tgz...
Trying: /C0A84.tgz...
Trying: /C0A8.tgz...
Trying: /C0A.tgz...
Trying: /C0.tgz...
Trying: /C.tgz...
Trying: /default.tgz...
Verified /manifest signed by PackageDevelopmentECP256_2021
Verified /manifest signed by PackageDevelopmentECP256_2023
/kernel text=0xc65e8 text=0x5e28f0 text=0xe05e4 data=0x140 data=0x556100 0x8+0xd6a70+0x8+0xb972f/
Verified /kernel
/crypto.ko size 0x48820 at 0x10e8000
Verified /crypto.ko
/ichwd.ko size 0x6a70 at 0x1131000
Verified /ichwd.ko
/iflib.ko size 0x1f1f0 at 0x1138000
Verified /iflib.ko
/miibus.ko size 0x385d8 at 0x1158000
Verified /miibus.ko
/if_em.ko size 0x697c8 at 0x1191000
Verified /if_em.ko
/if_fxp.ko size 0xf3a8 at 0x11fb000
Verified /if_fxp.ko
/virtio.ko size 0x5340 at 0x120b000
Verified /virtio.ko

etc

What about adding another field to fs_ops so a flag like skip devopen can be set?

Alternative means of skipping devopen for pkgfs

Make the code in open() neater

stand/common/dev_net.c
418

the trip through uinptr_t I think isn't needed. portp and ptr are both char * and can just be subtracted. Maybe the cast to (int) after is still needed, though...

432

The changes in this file, I think, should be a separate commit, but no real need to break them down further.

stand/common/install.c
141 ↗(On Diff #158749)

This should be a separate commit, since it adds a different thing (even if it's related).

295 ↗(On Diff #158749)

Is this fallback still needed? If so, document why and commit it separately.

stand/common/misc.c
204 ↗(On Diff #158749)

Why is NET special here?
We do the same env_setenv if the mount succeeds, with an unmount if there was a previous value. Why does net need to skip that unmount?

stand/libsa/globals.c
20

The = 0 is redundant.

stand/libsa/open.c
159 ↗(On Diff #158749)

So I really don't understand this. for pkgfs, exclusive_file_system will start out non-NULL, since we're opening the package (with exclsuive_filesyste_set just before the open.
We then set it back to NULL and do new_package (which just reads the file that opened, so there's no opens.
We then set the exclusive_fielsystem to pkgfs, but also set the _devopen bool to 1 in that case. So, we always go through devopen for all future opens, when before we wouldn't. If that succeeds, then we now have a handle on the raw device... then we go to the overriding filesystem. And I'm not even sure I have all the double negatives parsed right, and the _devopen is ever unset unless another package comes in.

This seems way too convoluted. Seems a simpler to have a flag to communicate something like this, as you've suggested below.

stand/libsa/tftp.c
903

These changes look fairly independent... You could split the debugging stuff out, but there's not likely to be a need to bisect around it, so I wouldn't bother splitting below this file level.

sjg marked 3 inline comments as done.Jul 18 2025, 10:12 PM
sjg added inline comments.
stand/common/dev_net.c
432

I don't mind breaking into separate commits, but at this point would like to avoid additional reviews, if the code itself is ok.

stand/common/install.c
295 ↗(On Diff #158749)

We can get to here via PXE boot, or via an explicit install command at loader prompt, and possibly one other route on u-boot platforms. IIRC one of those can get us here with proto NULL
Will add a comment.

stand/common/misc.c
204 ↗(On Diff #158749)

Its the mount we're skipping - which makes no sense via tftp.

With a FreeBSD tftp server the attempt doesn't do a lot of harm, but with other servers, it can result in a 15 minute delay in booting which is not acceptible.

stand/libsa/open.c
159 ↗(On Diff #158749)

So I really don't understand this. for pkgfs, exclusive_file_system will start out non-NULL, since we're opening the package (with exclsuive_filesyste_set just before the open.

It is complicated for sure.

pkgfs_init can be called with proto initially set to something other than pkgfs_fsops, and if so we need to do the devopen (eg for net0:)
but for pkgfs_fsops devopen will not work.

We then set it back to NULL and do new_package (which just reads the file that opened, so there's no opens.
We then set the exclusive_fielsystem to pkgfs, but also set the _devopen bool to 1 in that case. So, we always go through devopen for all future opens, when before we wouldn't. If that succeeds, then we now have a handle on the raw device... then we go to the overriding filesystem. And I'm not even sure I have all the double negatives parsed right, and the _devopen is ever unset unless another package comes in.

This seems way too convoluted. Seems a simpler to have a flag to communicate something like this, as you've suggested below.

I'm ok with adding a flag to fsops - this was at minimum an experiment to verify that approach would actually work.

Add flag to fs_ops for skipping devopen

sjg marked 2 inline comments as done.Jul 18 2025, 10:35 PM

Mention pkgfs_fsops in libsa.3

Move install.c change to D51446
Move misc.c change to D51447

Split out fs_ops.fs_flag to D51684

This revision was not accepted when it landed; it landed in state Needs Review.Aug 20 2025, 10:56 PM
Closed by commit rG5bfb3045d25b: Allow secure-netboot (authored by sjg). · Explain Why
This revision was automatically updated to reflect the committed changes.