https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=284749
--- Comment #15 from Jordan Morningstar <ports.maintai...@evilphi.com> --- (In reply to Michael Osipov from comment #14) The first consideration I had was that this can't change how certctl gets used by things that need a "just update the trust store" command. That includes freebsd-update, orchestration scripts, ca_root_nss, etc.. Those can't realistically all be changed in fair time, so the established `certctl rehash` command has to do the right thing whether it's maintaining just CERTDESTDIR, or both that and CERTDESTFILE. With CERTDESTFILE, there's a binary initial state and a binary final state: - Whether or not CERTDESTFILE already exists when `certctl rehash` is run; - Whether or not CERTDESTFILE exists when `certctl rehash` finishes. That means there's four possible operating cases to cover: 1. CERTDESTFILE exists and should continue existing; 2. CERTDESTFILE doesn't exist and should continue not existing; 3. CERTDESTFILE doesn't exist, but must start existing; 4. CERTDESTFILE exists, but must stop existing. The first two cases are the default, "status quo" cases: whatever is or isn't there, maintain that situation. What the current users of `certctl rehash` expect. The "if -e CERTDESTFILE" block in the sweep phase of cmd_rehash does that by setting WANTCERTDESTFILE=1 to signal to the create phase to run create_bundle. If that's all we had to worry about, it wouldn't need the "if WANTCERTDESTFILE != 0" test, but there are two other cases to handle. Case 3 is covered by -b. If CERTDESTFILE doesn't exist, then the "if -e CERTDESTFILE" test fails and the cmd_rehash sweep phase doesn't set WANTCERTDESTFILE=1, but we need it to be =1 so the getopts for -b sets it instead. Case 4 is covered by -B. This is where WANTCERTDESTFILE being ternary comes in. In the default cases, cmd_rehash runs with WANTCERTDESTFILE=-1, meaning nothing has given an explicit instruction either way about the continued existence of CERTDESTFILE. The getopts for -B sets WANTCERTDESTFILE=0, giving that explicit instruction that makes the create phase of cmd_rehash skip running create_bundle even if the "if -e CERTDESTFILE" test passed. That's why the test is "if WANTCERTDESTFILE != 0"--it has to match both 1 and -1. In plain terms, the "!= 0" says "if we haven't been explicitly told not to...". So WANTCERTDESTFILE needs to be able to express both explicit yes and explicit no in addition to the implicit state. The nounset condition means it also needs an initial value that can't be either explicit signal. It could have been an empty string, but empty strings and nounset historically haven't always played nice. An explicit third state value is safer. I chose "-1" because that's the traditional/canonical value for the "don't care" state of a ternary. I feel I should also mention that while cases 3 and 4 do map to many things, they correspond to install and uninstall, respectively, steps in ca_root_nss. It's improper for a port to delete files it didn't install, so ca_root_nss must have certctl do that bit on its behalf. The port must run `certctl rehash` anyway[1], so an easy way to link all this up is just have a couple of flags that can be added to the exec and unexec calls. Note 1: I suspect certctl seeing LOCALBASE/usr/share/certs/ca-root-nss.crt is (part of) why do_scan does its SPLITDIR thing: certctl can pick up the full bundle and dismantle it into individual hashed certificates. That behaviour also makes life easier for users with local certificates, be that a server certificate trust chain, enterprise/appliance CA bundle, or third-party tools that sync to NSS/CCADB directly. The SPLITDIR trick means the user can simply drop those files into /usr/local/etc/ssl/certs without needing to know much about file content interactions. Yes, this isn't what OpenSSL does; but, to me at least, there's better adherence to POLA this way because it "just works". No leaving the user wondering why certctl ignored part of their file, then ultimately tasked with manual splitting. p.s., In writing all this out, I realized "if WANTCERTDESTFILE = -1" in the sweep phase should work as well. I can test and change "!= 0" to "= -1" if you feel that would make the logic easier to understand. p.p.s., I also realized that I used C semantics for WANTCERTDESTFILE (0=false). I can switch it to Bourne semantics (0=true) if you want. It's perhaps the run of the nit litter, but it is a Bourne script. p.p.p.s., I do love how teaching something to others can help find stuff I didn't notice before. -- You are receiving this mail because: You are the assignee for the bug.
