Replace literal uses of /usr/local with a macro defined in paths.h

[se]

Summary

The default value of LOCALBASE (/usr/local) has been added to paths.h as _PATH_LOCALBASE in an earlier commit.

This allows to remove 19 direct uses of /usr/local in C sources in the base system and to replace them with references to this macro.

This patch is meant as an initial step of removing literal uses of /usr/local from base (except for the definition in a single place, which could then be made depend on the LOCALBASE definition in src/Makefile.inc1).

It does not modify:

• C sources in contrib or sys/contrib
• comments in sources, man-pages, and references in text files/documentation
• configuration files

There is no functional change, all binaries created are unmodified (as long as _PATH_LOCALBASE is not changed)

Test Plan

Build the world with these patches applied
Verify that there are no changes to header files in /usr/include (beyond the already committed addition of _PATH_LOCALBASE to paths.h)

[dim]

crypto/openssh/pathnames.h

26 ↗

(On Diff #78737)

Note that crypto/openssh is really also contrib'd code, so take care. Maybe this should be placed in non-vendor files, to reduce diffs against upstream. Or convince OpenBSD to accept this patch ;)

[se]

crypto/openssh/pathnames.h

26 ↗

(On Diff #78737)

This will add less than 10 lines of additional differences to currently more than 16000 lines for a diff -r -u0 between crypto/openssh and the version in vendor-crypto/openssh/dist.

I agree that there should be as little changes as possible in contributed code (and I have ignored files in contrib and sys/contrib for this reason), but I do not expect these changes to cause problems with later merges (and they were easy to resolve, if they occurred).

It is of course also possible to pass the definitions of _PATH_SSH_ASKPASS_DEFAUT and _PATH_XAUTH via the Makefile, and I'll see whether this results in a more acceptable change.

[se]

crypto/openssh/pathnames.h

26 ↗

(On Diff #78737)

This will add less than 10 lines of additional differences to currently more than 16000 lines for a diff -r -u0 between crypto/openssh and the version in vendor-crypto/openssh/dist.

I agree that there should be as little changes as possible in contributed code (and I have ignored files in contrib and sys/contrib for this reason), but I do not expect these changes to cause problems with later merges (and they were easy to resolve, if they occurred).

It is of course also possible to pass the definitions of _PATH_SSH_ASKPASS_DEFAUT and _PATH_XAUTH via the Makefile, and I'll see whether this results in a more acceptable change.

The patches to pathnames.h and ssh-agent.c can be replaced by patches in 2 Makefiles in src/secure.

I'm not sure that these cryptic CFLAGS settings are better than the direct patches of the source files, especially since changes in the source files that use these parameters would become obvious due to collisions, while they'd go by unnoticed if the parameters in the Makefiles will just be ignored.

But this approach is definitely less intrusive and might thus be preferential:

Index: secure/lib/libssh/Makefile
===================================================================
--- secure/lib/libssh/Makefile	(revision 367044)
+++ secure/lib/libssh/Makefile	(working copy)
@@ -53,6 +53,10 @@
 SRCS+=	 krb5_config.h
 .endif
 
+.if defined(LOCALBASE)
+CFLAGS+= -D_PATH_SSH_ASKPASS_DEFAULT=\"${LOCALBASE}/bin/ssh-askpass\"
+.endif
+
 NO_LINT=
 
 LIBADD+=	crypto crypt z
Index: secure/usr.bin/ssh-agent/Makefile
===================================================================
--- secure/usr.bin/ssh-agent/Makefile	(revision 367044)
+++ secure/usr.bin/ssh-agent/Makefile	(working copy)
@@ -16,6 +16,10 @@
 #LDADD+=	-lldns
 .endif
 
+.if defined(LOCALBASE)
+CFLAGS+= -DDEFAULT_PKCS11_WHITELIST=\"/usr/lib*/*,${LOCALBASE}/lib*/*\"
+.endif
+
 LIBADD+=	crypto
 
 .include <bsd.prog.mk>

[se]

Alternative patch that does not modify openssh/pathnames.h but passes the LOCALBASE dependent strings in non-contributed Makefiles in src/secure/ssh

[rgrimes]

Though this is good intent, it is not a good approach. I'll assert again at one time we had erradicated all /usr/local from the base system, these above have crept in over time and again should be erradicated. There should never ever be a /usr/local #define in paths.h, having to recompile code to change this, again, is the wrong approach. This should _possibly_/_probably_ be moved to a kernel variable that can be changed at boot time.

[se]

In D26942#601180, @rgrimes wrote:

Though this is good intent, it is not a good approach. I'll assert again at one time we had erradicated all /usr/local from the base system, these above have crept in over time and again should be erradicated. There should never ever be a /usr/local #define in paths.h, having to recompile code to change this, again, is the wrong approach. This should _possibly_/_probably_ be moved to a kernel variable that can be changed at boot time.

While I agree that the base system should not needlessly depend on the value of LOCALBASE, this is not realistic if we want to be able to extend the base system via ports/packages.
On a system that does not have any ports/packages installed, references to LOCALBASE are not required, neither hard-coded nor by some dynamic mechanism.

But in reality, we want to have the ability to extend the base system via ports/packages - and the biggest fraction of users will keep the default path of /usr/local for the prefix.
With official packages you have no choice, but you could use a symlink e.g. from /opt -> /usr/local to hide this fact.

If you really want to use a different LOCALBASE and are willing to install from ports or build your own private package repository, changing a single definition in paths.h is the least possible effort.
I do not want to change the value of LOCALBASE on each reboot - why might you need a possibility to do that?

But if you really have such a need, then please provide patches that access the dynamic path (passed in the environment or via sysctl?) in all programs that depend on the value of LOCALBASE (and which I intend to make refer to _PATH_LOCALBASE).

You probably are aware of the security implications, e.g. if a program being executed with root privileges spawns a child process (or loads a dynamic library) and uses an environment variable to locate it.
And if you'd prefer to use a sysctl then consider the implications for jails (is it acceptable to have the same value in all jails?).

If we did live in a clean BSD world (with BSD the distribution as of BSD-4.3) and not in a world that is dominated by software development outside *BSD and often in some Linux environment, it might be possible to avoid using LOCALBASE. But with /usr/local hard-coded in a large number of externally developed programs you must have good reasons to deviate from this PREFIX and be able to bear the extra effort.

BTW: /usr/local is even specified in /etc/mtree/BSD.usr.dist as the official location for add-on packages ...

[imp]

I would suggest that the goal of no /usr/local references is past its sell by date.

So going from an unchangeable hard coding to a changeable one is progress. Having a dynamically changing one is harder and likely fraught with security issues done wrong.

I don't see a lot of support for removing /usr/local in the developer community, nor really in the larger FreeBSD community. It may have been a goal years ago, but the rest of the software world has basically said 'nope' to that position. Let's make some progress in adopting to this shift since the 90s to make it uniform.