Bug 275449 - certctl: changed link flags lead to inconsistencies when installing into non-standard DESTDIR
Summary: certctl: changed link flags lead to inconsistencies when installing into non-...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Dag-Erling Smørgrav
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-30 12:58 UTC by Martin Birgmeier
Modified: 2024-03-27 18:32 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Birgmeier 2023-11-30 12:58:20 UTC
Scenario:
- Virtual machine with UFS / and /usr (separate)
- In it, FreeBSD main at 51e7276365bad2871257ef121cead2c19bcd7434
- Running buildworld buildkernel
- Running installkernel installworld DESTDIR=/usr/tmp/x (the intent is to install to a temporary directory first to view the changes to the current installation)

Result:
- The files in /usr/tmp/x/etc/ssl/{certs,untrusted} are hard links to the files in /usr/tmp/x/usr/share/certs.
- However, the files in /etc/ssl/{certs,untrusted} are (and should be) symbolic links to those in /usr/share/certs.
- Blindly copying what is in /usr/tmp/x to / would break the hard link, resulting in duplicate regular files.

The root cause is that in certctl, the argument to "install" has been changed from "-lrs" to "-lm", leading to destination-dependent results. For the sake of consistency it would be good to revert this particular change.

-- Martin
Comment 1 Kyle Evans freebsd_committer freebsd_triage 2023-12-21 17:12:31 UTC
CC'd des@ because I don't think I actually caught why we're doing these as hard links
Comment 2 Dag-Erling Smørgrav freebsd_committer freebsd_triage 2023-12-21 17:24:26 UTC
The change is intentional.  Does it actually break anything?
Comment 3 Martin Birgmeier 2023-12-21 18:23:59 UTC
It makes the following scenario perform worse than before the change:
- Install into /usr/tmp/x (on one filesystem)
- From there copying of changed files to /
- But / and /usr are on separate filesystems
- As a result, the files in /etc/ssl are now single-link copies of what is also in /usr/share/certs, leading to a waste of space

I believe keeping the symlinks would not hurt as they are typically short enough to fit into the inode itself, thereby not occupying more space than a hard link.

-- Martin
Comment 4 Martin Birgmeier 2023-12-21 18:37:54 UTC
Maybe the best solution would be not to clutter /etc with so many files but rather keep both the certificates and their hashes in the same directories, and only linking to these directories.

-- Martin
Comment 5 DKnoto 2024-02-02 19:32:18 UTC
(In reply to Dag-Erling Smørgrav from comment #2)

This change disallows installation with unusual disk partitioning as published on https://wiki.freebsd.org/RootOnZFS/UFSBoot.

Here is a screenshot of the installation error: https://www.dropbox.com/scl/fi/i75b73ukw26rxalbxopv3/FreeBSD-15-C-2024-01-29-Base-extracting-error-c16.png?rlkey=754c99ie5s2qvbj3dtnaadtzo&dl=0
Comment 6 Piotr Pawel Stefaniak freebsd_committer freebsd_triage 2024-02-03 13:43:24 UTC
The reasoning is not clear enough and confusing even for FreeBSD developers.
Comment 7 Martin Birgmeier 2024-02-11 06:57:46 UTC
I am using the following patch:

diff --git a/usr.sbin/certctl/certctl.sh b/usr.sbin/certctl/certctl.sh
index 997a7d835d53..f95b4561d852 100755
--- a/usr.sbin/certctl/certctl.sh
+++ b/usr.sbin/certctl/certctl.sh
@@ -110,7 +110,7 @@ create_trusted()
 {
        local hash certhash otherfile otherhash
        local suffix
-       local link=${2:+-lm}
+       local link=${2:+-lrs}
 
        hash=$(do_hash "$1") || return
        certhash=$(openssl x509 -sha1 -in "$1" -noout -fingerprint)
@@ -159,7 +159,7 @@ resolve_certname()
 create_untrusted()
 {
        local srcfile filename
-       local link=${2:+-lm}
+       local link=${2:+-lrs}
 
        set -- $(resolve_certname "$1")
        srcfile=$1

-- Martin