freebsd-update: rehash certs
ClosedPublic
Actions

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

Details

Reviewers

grembo
emaste
cperciva
allanjude

Group Reviewers

Core Team
secteam

Commits

rS360287: freebsd-update: rehash certs

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kevans created this revision.Sep 26 2019, 5:38 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 26 2019, 5:38 PM

Harbormaster completed remote builds in B26721: Diff 62607.Sep 26 2019, 5:38 PM

kevans added a parent revision: D16857: Introduce certctl(8).Sep 26 2019, 5:38 PM

delphij added a subscriber: delphij.Sep 26 2019, 9:01 PM

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?

kevans added inline comments.Sep 26 2019, 9:28 PM

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

kevans added inline comments.Sep 30 2019, 1:42 PM

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.

Harbormaster completed remote builds in B26834: Diff 62846.Oct 2 2019, 5:32 PM

Tag Colin, who may still be in hiding =D

emaste added inline comments.Dec 5 2019, 2:59 PM

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

kevans added inline comments.Dec 5 2019, 3:07 PM

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.

kevans mentioned this in D16857: Introduce certctl(8).Mar 2 2020, 3:23 PM

Is this stalled?

In D21805#528394, @allanjude wrote:

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.

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.

Harbormaster completed remote builds in B30702: Diff 70947.Apr 24 2020, 5:11 PM