Re: git: 9bfd3b4076a7 - main - Add a build knob for _FORTIFY_SOURCE

From: Pedro Giffuni <pfg_at_freebsd.org>
Date: Sat, 18 May 2024 19:07:15 UTC
 Sorry for noticing so late ... I was unaware this was being worked on and  I was very busy with since my dad passed away recently.
The static checker component of Fortify source only works well on GCC, for clang this lacks the support that was added by Google on Android's libc (which is not bery useful either).
We already had some stubs for the ssp functions but we never used them and this just adds bloat to our libc.
I suggest reverting. Again sorry.
Pedro.
    On Monday, May 13, 2024 at 12:24:16 AM GMT-5, Kyle Evans <kevans@freebsd.org> wrote:  
 
 The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=9bfd3b4076a7b0dfd27ab22318e5113dc84fea28

commit 9bfd3b4076a7b0dfd27ab22318e5113dc84fea28
Author:    Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2024-05-13 05:23:50 +0000
Commit:    Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-05-13 05:23:50 +0000

    Add a build knob for _FORTIFY_SOURCE
    
    In the future, we will Default to _FORTIFY_SOURCE=2 if SSP is enabled,
    otherwise default to _FORTIFY_SOURCE=0.  For now we default it to 0
    unconditionally to ease bisect across older versions without the new
    symbols, and we'll put out a call for testing.
    
    include/*.h include their ssp/*.h equivalents as needed based on the
    knob. Programs and users are allowed to override FORTIFY_SOURCE in their
    Makefiles or src.conf/make.conf to force it off.
    
    Reviewed by:    des, markj
    Relnotes:      yes
    Sponsored by:  Stormshield
    Sponsored by:  Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D32308
---
 include/stdio.h                |  3 ++
 include/string.h                |  3 ++
 include/strings.h              |  3 ++
 include/unistd.h                |  4 +++
 lib/libthr/Makefile            |  3 ++
 libexec/rtld-elf/Makefile      |  4 +++
 share/man/man7/security.7      | 75 +++++++++++++++++++++++++++++++++++++++++
 share/mk/bsd.sys.mk            |  7 ++++
 tools/build/options/WITHOUT_SSP |  3 ++
 tools/build/options/WITH_SSP    |  3 ++
 10 files changed, 108 insertions(+)

diff --git a/include/stdio.h b/include/stdio.h
index fe7a6f7d6f82..30bc638082d8 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -530,4 +530,7 @@ extern int __isthreaded;
 __END_DECLS
 __NULLABILITY_PRAGMA_POP
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include <ssp/stdio.h>
+#endif
 #endif /* !_STDIO_H_ */
diff --git a/include/string.h b/include/string.h
index 597308020cdb..a595f6e3e260 100644
--- a/include/string.h
+++ b/include/string.h
@@ -168,4 +168,7 @@ errno_t memset_s(void *, rsize_t, int, rsize_t);
 #endif /* __EXT1_VISIBLE */
 __END_DECLS
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include <ssp/string.h>
+#endif
 #endif /* _STRING_H_ */
diff --git a/include/strings.h b/include/strings.h
index fde007186e04..6fe6a09e7dd3 100644
--- a/include/strings.h
+++ b/include/strings.h
@@ -68,4 +68,7 @@ int    strncasecmp(const char *, const char *, size_t) __pure;
 #endif
 __END_DECLS
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include <ssp/strings.h>
+#endif
 #endif /* _STRINGS_H_ */
diff --git a/include/unistd.h b/include/unistd.h
index e4e5c62fbb67..59738cbf6e68 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -37,6 +37,10 @@
 #include <sys/_null.h>
 #include <sys/_types.h>
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include <ssp/unistd.h>
+#endif
+
 #ifndef _GID_T_DECLARED
 typedef    __gid_t        gid_t;
 #define    _GID_T_DECLARED
diff --git a/lib/libthr/Makefile b/lib/libthr/Makefile
index a5bf5da44170..85c028f521a1 100644
--- a/lib/libthr/Makefile
+++ b/lib/libthr/Makefile
@@ -11,6 +11,9 @@ LDFLAGS+=    -Wl,--rpath=/usr/lib${COMPAT_libcompat}
 
 .include <src.opts.mk>
 MK_SSP=    no
+# SSP forced off already implies FORTIFY_SOURCE=0, but we must make sure that
+# one cannot turn it back on.
+FORTIFY_SOURCE=    0
 
 LIB=thr
 SHLIB_MAJOR= 3
diff --git a/libexec/rtld-elf/Makefile b/libexec/rtld-elf/Makefile
index 37c3840538d5..864448ad782a 100644
--- a/libexec/rtld-elf/Makefile
+++ b/libexec/rtld-elf/Makefile
@@ -15,6 +15,10 @@ MK_UBSAN=    no
 
 .include <bsd.compat.pre.mk>
 
+# SSP forced off already implies FORTIFY_SOURCE=0, but we must make sure that
+# one cannot turn it back on.
+FORTIFY_SOURCE=    0
+
 .if !defined(NEED_COMPAT)
 CONFS=        libmap.conf
 .endif
diff --git a/share/man/man7/security.7 b/share/man/man7/security.7
index ccbeeb4575ce..2e690e35d534 100644
--- a/share/man/man7/security.7
+++ b/share/man/man7/security.7
@@ -939,6 +939,81 @@ option that SSH allows in its
 .Pa authorized_keys
 file to make the key only usable to entities logging in from specific
 machines.
+.Sh STACK OVERFLOW PROTECTION
+.Fx
+supports stack overflow protection using the Stack Smashing Protector
+.Pq SSP
+compiler feature.
+In userland, SSP adds a per-process randomized canary at the end of every stack
+frame which is checked for corruption upon return from the function.
+In the kernel, a single randomized canary is used globally except on aarch64,
+which has a
+.Dv PERTHREAD_SSP
+.Xr config 8
+option to enable per-thread randomized canaries.
+If stack corruption is detected, then the process aborts to avoid potentially
+malicious execution as a result of the corruption.
+SSP may be enabled or disabled when building
+.Fx
+base with the
+.Xr src.conf 5
+SSP knob.
+.Pp
+When
+.Va WITH_SSP
+is enabled, which is the default, world is built with the
+.Fl fstack-protector-strong
+compiler option.
+The kernel is built with the
+.Fl fstack-protector
+option.
+.Pp
+In addition to SSP, a
+.Dq FORTIFY_SOURCE
+implementation is supported up to level 2 by defining
+.Va _FORTIFY_SOURCE
+to
+.Dv 1
+or
+.Dv 2
+before including any
+.Fx
+headers.
+.Fx
+world builds can set
+.Va FORTIFY_SOURCE
+to provide a default value for
+.Va _FORTIFY_SOURCE .
+When enabled,
+.Dq FORTIFY_SOURCE
+enables extra bounds checking in various functions that accept buffers to be
+written into.
+These functions currently have extra bounds checking support:
+.Bl -column -offset indent "snprintf" "memmove" "strncpy" "vsnprintf" "readlink"
+.It bcopy    Ta bzero    Ta fgets    Ta getcwd    Ta gets
+.It memcpy  Ta memmove  Ta memset    Ta read      Ta readlink
+.It snprintf Ta sprintf  Ta stpcpy    Ta stpncpy  Ta strcat
+.It strcpy  Ta strncat  Ta strncpy  Ta vsnprintf Ta vsprintf
+.El
+.Pp
+.Dq FORTIFY_SOURCE
+requires compiler support from
+.Xr clang 1
+or
+.Xr gcc 1 ,
+which provide the
+.Xr __builtin_object_size 3
+function that is used to determine the bounds of an object.
+This feature works best at optimization levels
+.Fl O1
+and above, as some object sizes may be less obvious without some data that the
+compiler would collect in an optimization pass.
+.Pp
+Similar to SSP, violating the bounds of an object will cause the program to
+abort in an effort to avoid malicious execution.
+This effectively provides finer-grained protection than SSP for some class of
+function and system calls, along with some protection for buffers allocated as
+part of the program data.
 .Sh KNOBS AND TWEAKS
 .Fx
 provides several knobs and tweak handles that make some introspection
diff --git a/share/mk/bsd.sys.mk b/share/mk/bsd.sys.mk
index de91e00d8cc7..52c3d07746c7 100644
--- a/share/mk/bsd.sys.mk
+++ b/share/mk/bsd.sys.mk
@@ -294,11 +294,18 @@ CFLAGS.clang+=    -Qunused-arguments
 # but not yet.
 CXXFLAGS.clang+=    -Wno-c++11-extensions
 
+# XXX This should be defaulted to 2 when WITH_SSP is in use after further
+# testing and soak time.
+FORTIFY_SOURCE?=    0
 .if ${MK_SSP} != "no"
 # Don't use -Wstack-protector as it breaks world with -Werror.
 SSP_CFLAGS?=    -fstack-protector-strong
 CFLAGS+=    ${SSP_CFLAGS}
 .endif # SSP
+.if ${FORTIFY_SOURCE} > 0
+CFLAGS+=    -D_FORTIFY_SOURCE=${FORTIFY_SOURCE}
+CXXFLAGS+=    -D_FORTIFY_SOURCE=${FORTIFY_SOURCE}
+.endif
 
 # Additional flags passed in CFLAGS and CXXFLAGS when MK_DEBUG_FILES is
 # enabled.
diff --git a/tools/build/options/WITHOUT_SSP b/tools/build/options/WITHOUT_SSP
index 88162cecf14a..7a773fe1e5aa 100644
--- a/tools/build/options/WITHOUT_SSP
+++ b/tools/build/options/WITHOUT_SSP
@@ -1 +1,4 @@
 Do not build world with stack smashing protection.
+See
+.Xr security 7
+for more information.
diff --git a/tools/build/options/WITH_SSP b/tools/build/options/WITH_SSP
index 0088dd133782..4f06a73d4173 100644
--- a/tools/build/options/WITH_SSP
+++ b/tools/build/options/WITH_SSP
@@ -1 +1,4 @@
 Build world with stack smashing protection.
+See
+.Xr security 7
+for more information.