Page MenuHomeFreeBSD

freebsd-update: rehash certs
ClosedPublic

Authored by kevans on Sep 26 2019, 5:38 PM.

Details

Summary

With the inclusion of caroot bits, we'll need to also rehash on update as we do in mergemaster/etcupdate.

If certctl's installed on the system, just unconditionally rehash. This isn't an expensive operation, and we can refine it to compare INDEX-{OLD,NEW} later if we really want to.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

delphij added inline comments.
usr.sbin/freebsd-update/freebsd-update.sh
2949 ↗(On Diff #62607)

Not sure if I followed -- why do we need to search INDEX-OLD here?

usr.sbin/freebsd-update/freebsd-update.sh
2949 ↗(On Diff #62607)

I'll have to re-read again to see if I can follow my logic, but it was likely a bad misunderstanding on my part

usr.sbin/freebsd-update/freebsd-update.sh
2949 ↗(On Diff #62607)

Sorry, this took a little longer to circle back to than I'd like -- it looks like what I really wanted to do was rehash if /usr/local/certs/* files either disappeared between INDEX-OLD and INDEX-NEW or just appeared in INDEX-NEW.

I think this should probably use the logic from install_delete, then check killfiles and INDEX-NEW for cert changes.

Take #2; still know very little about freebsd-update. =-)

Refactored the check for whether we need to rehash out into another function. First we check INDEX-NEW because this will trivially tell us if we've had any new certs or cert modifications (as far as I understand what the index is composed of), then we check if any certs have been removed.

Tag Colin, who may still be in hiding =D

usr.sbin/freebsd-update/freebsd-update.sh
2858 ↗(On Diff #62846)

s/nuke/remove/

2908 ↗(On Diff #62846)

probably need to update this comment, maybe just "update generated files" or something like that

usr.sbin/freebsd-update/freebsd-update.sh
2850 ↗(On Diff #62846)

One general question I did have- what do INDEX-OLD/INDEX-NEW actually contain? Is this check here actually triggering if the contents of a cert changed (for some reason) but not the path?

I'm somewhat suspecting that I should either be doing something like the below sort/join/sort/cut/tr invocation but from old -> new to make sure we only rehash when something's actually changed, or we should drop the illusion that we rehash only on changes that need a rehash and just rehash if a cert appears in either INDEX because a rehash should generally do no harm.

Is this stalled?

I'm working on setting up a freebsd-update server to test the change; I've tested it against a 12.1 update from the standard freebsd-update server with no certs and it at least didn't blow up there, but I'm attempting to do builds that:

1.) Introduce certs where they didn't exist before, and
2.) adding/removing certs

I was initially blocked by people.f.o truncating the .iso download to half of the original size, and now the build drives my machine OOM.

kevans marked 2 inline comments as done.
kevans retitled this revision from freebsd-update: rehash certs as needed to freebsd-update: rehash certs.
kevans edited the summary of this revision. (Show Details)

Simplify to hopefully unblock: if there's a certctl in path, use it to rehash. This is an attempt to unblock this review; the extra complexity isn't necessary for an initial revision and there's some level of desire to have a bundle included with the impending 11.4.

With DESTDIR support in certctl now, this looks good to me

This revision is now accepted and ready to land.Apr 24 2020, 5:26 PM
This revision was automatically updated to reflect the committed changes.