Bug 283340 - build: distributeworld -DNO_ROOT does not set SSL cert links uname / gname in METALOG
Summary: build: distributeworld -DNO_ROOT does not set SSL cert links uname / gname in...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL: https://github.com/freebsd/freebsd-sr...
Keywords: reproducible-builds
Depends on:
Blocks: 283214
  Show dependency treegraph
 
Reported: 2024-12-15 08:12 UTC by Pat Maddox
Modified: 2025-01-21 22:35 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pat Maddox 2024-12-15 08:12:17 UTC
As of 6e55ba5b3, the distributeworld METALOG does not include uname / gname entries for the links in etc/ssl, e.g.

/etc/ssl/certs/002c0b4f.0 type=link mode=0444 link=../../../usr/share/certs/trusted/GlobalSign_Root_R46.pem

It should be:

/etc/ssl/certs/002c0b4f.0 type=link uname=root gname=wheel mode=0444 link=../../../usr/share/certs/trusted/GlobalSign_Root_R46.pem

The lack of uname/gname results in the links being added to base.txz with the building user's uid.
Comment 2 commit-hook freebsd_committer freebsd_triage 2024-12-30 17:33:22 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=10fa3f2518d4582c98d74527f79af9f30b1eceab

commit 10fa3f2518d4582c98d74527f79af9f30b1eceab
Author:     Pat Maddox <pat@patmaddox.com>
AuthorDate: 2024-12-15 09:20:24 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-12-30 17:32:23 +0000

    certctl: Set METALOG ownership to root:wheel

    This sets the correct ownership values when building base.txz

    PR:             283340
    Reviewed by:    allanjude
    Pull request:   https://github.com/freebsd/freebsd-src/pull/1550

    Signed-off-by: Pat Maddox <pat@patmaddox.com>

 usr.sbin/certctl/certctl.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 3 Jose Luis Duran freebsd_committer freebsd_triage 2025-01-16 18:40:00 UTC
The metalog file should have a:
    /set uname=root gname=wheel
Definition at the beginning of the file that should prevent this from happening.

At any rate, the patch applied in 10fa3f2518d4 ("certctl: Set METALOG ownership to root:wheel") broke my NanoBSD NO_ROOT builds (WIP) because the password database file does not yet exist when certctl.sh is rehashed:
    install: Can't open `/usr/obj/jlduran/obj/_.w/etc/group': No such file or directory
    install: Unable to use user and group databases in `/usr/obj/jlduran/obj/_.w/etc': No such file or directory

One “hack” that could work is to remove the -N flag from the INSTALLFLAGS.
Or perhaps move the rehash later in the Makefile?
Comment 4 Jessica Clarke freebsd_committer freebsd_triage 2025-01-16 19:01:10 UTC
> The metalog file should have a:
>     /set uname=root gname=wheel
> Definition at the beginning of the file that should prevent this from happening.

No it shouldn't. That would hide potential bugs. If you want anything it should be to have a validation pass that every entry has uname and gname set.

> At any rate, the patch applied in 10fa3f2518d4 ("certctl: Set METALOG ownership to root:wheel") broke my NanoBSD NO_ROOT builds (WIP) because the password database file does not yet exist when certctl.sh is rehashed:
>     install: Can't open `/usr/obj/jlduran/obj/_.w/etc/group': No such file or directory
>     install: Unable to use user and group databases in `/usr/obj/jlduran/obj/_.w/etc': No such file or directory
> 
> One “hack” that could work is to remove the -N flag from the INSTALLFLAGS.
> Or perhaps move the rehash later in the Makefile?

Yes and no; the patch is broken. certctl.sh rehash is run during installworld, but the destination won't have the relevant files until distribution is run, which at least for cheribuild comes after installworld, and I assume is true for your build too. Makefile.inc1 uses -N ${.CURDIR}/etc, i.e. the *source* tree's, DB. I think certctl.sh needs to do the same during installworld.
Comment 5 commit-hook freebsd_committer freebsd_triage 2025-01-17 17:57:06 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=98bebc20cef7527ccb15f8defc9d52e803a0d506

commit 98bebc20cef7527ccb15f8defc9d52e803a0d506
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2025-01-17 17:49:01 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2025-01-17 17:56:30 +0000

    Revert "certctl: Set METALOG ownership to root:wheel"

    This introduces a circular dependency because it requires an existing
    dbdir for install -N, which might not yet exist.

    I imagine we can use install -o 0 -g -0, avoiding the need for the
    dbdir, but install emits uname=0 gname=0 rather than uid=0 gid=0.
    So just revert for now pending a full fix.

    This reverts commit 10fa3f2518d4582c98d74527f79af9f30b1eceab.

    PR:             283340
    Event:          January 2025 Bug-busting session
    Sponsored by:   The FreeBSD Foundation

 usr.sbin/certctl/certctl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 Pat Maddox 2025-01-17 19:07:46 UTC
fwiw I don't think this needs -N at all. It's root and wheel - is there a scenario where that wouldn't be correct?

In any case, can you provide info on how to replicate this failure?

Is it simply that https://docs.freebsd.org/en/articles/nanobsd/#_building_a_nanobsd_image fails with this patch, and same with https://github.com/CTSRD-CHERI/cheribsd ?
Comment 7 Ed Maste freebsd_committer freebsd_triage 2025-01-17 19:59:03 UTC
The issue occurs with installworld & distribution ordering. installworld can use the dbdir from OBJDIR but we don't have a(n easy / existing) way to pass that to certctl, so I just reverted for now to address the immediate issue.

(In reply to Pat Maddox from comment #6)
> fwiw I don't think this needs -N at all. It's root and wheel - is there a scenario where that wouldn't be correct?

We can probably do without -N and rely on the fact that root=0 and wheel=0 in the host's db as well, but I think we can go one step further and just use -u 0 -g 0. install -U -M currently has a bug with that though (PR283355) so my plan is fix that in install, then restore this change but with -u 0 -g 0.
Comment 8 Jose Luis Duran freebsd_committer freebsd_triage 2025-01-17 20:09:24 UTC
(In reply to Pat Maddox from comment #6)
Yes, that was my workaround, remove the -N flag, before it was reverted.

I think it will be re-submitted as -o 0 -g 0 (without the reference to a user database file), once https://reviews.freebsd.org/D48504 fixes the issue stated in the revert comment (install produces an invalid spec, uname/gname=0 instead of uid/gid=0)

The NO_ROOT build for NanoBSD is still a WIP, but can be found at:
https://github.com/jlduran/freebsd-src/tree/nanobsd-no-root-build

As a non-privileged user:
% cd /usr/src/tools/tools/nanobsd
% sh nanobsd.sh -U -c generic.nano
...
<should build a NanoBSD image under /home/<user>/obj/_.disk.full>

As root:
# sh /usr/share/examples/bhyve/vmrun.sh -u -m 1G -t tap0 -d /home/<user>/obj/_.disk.full nano
Comment 9 Jessica Clarke freebsd_committer freebsd_triage 2025-01-17 20:49:04 UTC
Currently METALOG only expresses ownership via uname/gname, not uid/gid. I would much rather see certctl.sh be slightly extended to use the source's etc and be consistent rather than introduce this inconsistency. I'm willing to write the patch if that's what it'd take.
Comment 10 Ed Maste freebsd_committer freebsd_triage 2025-01-17 21:20:06 UTC
(In reply to Jessica Clarke from comment #9)
I looked at the METALOG files I have sitting around and release/dist/METALOG contains some uid=0 gid=0 entries. Maybe this is an artifact of some WIP or another issue to be addressed assuming that we should be using uname and gname universally. The install issue in 284119 is a bug that needs to be fixed either way.

With uname/gname as a requirement I suggest we start by reapplying the commit without -N, relying on the fact that install doesn't validate them in any way (PR283355). In a subsequent change we can add the ability to plumb the appropriate dbdir through certctl.
Comment 11 commit-hook freebsd_committer freebsd_triage 2025-01-17 21:43:05 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4d15b58365ea706129bedfdb37e0c5e8661a640f

commit 4d15b58365ea706129bedfdb37e0c5e8661a640f
Author:     Pat Maddox <pat@patmaddox.com>
AuthorDate: 2024-12-15 09:20:24 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2025-01-17 21:41:02 +0000

    certctl: Set METALOG ownership to root:wheel

    This sets the correct ownership values when building base.txz

    This is an updated version of commit 10fa3f2518d4, omitting the
    `-N ${DESTDIR}${DISTBASE}/etc` from the original commit.

    install(1) does not validate the arguments passed to -o or -g
    (see PR283355) so there's no need to have the passwd db available
    for now.  Future work includes plumbing the appropriate passwd db
    path through certctl, and validating uid and gid in install(1).

    PR:             283340
    Reviewed by:    jrtc27
    Differential Revision: https://reviews.freebsd.org/D48506

 usr.sbin/certctl/certctl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 12 Ed Maste freebsd_committer freebsd_triage 2025-01-21 22:35:20 UTC
(In reply to Jessica Clarke from comment #4)
> If you want anything it should be to have a validation pass that every entry has
> uname and gname set.

We have a basic tool for validating METALOG in tools/pkgbase/metalog_reader.lua. Right now it's used to catch duplicate entries. It should be straightforward to add support for verifying that all entries have uname/gname set.