Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Add hardening options #4538

Merged
merged 1 commit into from
Nov 15, 2024
Merged

build: Add hardening options #4538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 60 additions & 22 deletions src/cmake/compiler.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ elseif (CMAKE_CXX_COMPILER_ID MATCHES "Intel")
set (CMAKE_COMPILER_IS_INTEL 1)
message (VERBOSE "Using Intel as the compiler")
endif ()
if (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG)
set (COMPILER_IS_GCC_OR_ANY_CLANG TRUE)
endif ()


###########################################################################
Expand All @@ -102,12 +105,12 @@ else ()
endif()
option (EXTRA_WARNINGS "Enable lots of extra pedantic warnings" OFF)
if (NOT MSVC)
add_compile_options ("-Wall")
add_compile_options (-Wall -Wformat -Wformat=2)
if (EXTRA_WARNINGS)
add_compile_options ("-Wextra")
add_compile_options (-Wextra)
endif ()
if (STOP_ON_WARNING)
add_compile_options ("-Werror")
add_compile_options (-Werror)
endif ()
endif ()

Expand Down Expand Up @@ -462,25 +465,60 @@ endif ()


###########################################################################
# Fortification and hardening options
#
# In modern gcc and clang, FORTIFY_SOURCE provides buffer overflow checks
# (with some compiler-assisted deduction of buffer lengths) for the following
# functions: memcpy, mempcpy, memmove, memset, strcpy, stpcpy, strncpy,
# strcat, strncat, sprintf, vsprintf, snprintf, vsnprintf, gets.
#
# We try to avoid these unsafe functions anyway, but it's good to have the
# extra protection, at least as an extra set of checks during CI. Some users
# may also wish to enable it at some level if they are deploying it in a
# security-sensitive environment. FORTIFY_SOURCE=3 may have minor performance
# impact, though FORTIFY_SOURCE=2 should not have a measurable effect on
# performance versus not doing any fortification. All fortification levels are
# not available in all compilers.

set (FORTIFY_SOURCE "0" CACHE STRING "Turn on Fortification level (0, 1, 2, 3)")
if (FORTIFY_SOURCE AND (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG))
message (STATUS "Compiling with _FORTIFY_SOURCE=${FORTIFY_SOURCE}")
add_compile_options (-D_FORTIFY_SOURCE=${FORTIFY_SOURCE})
# Safety/security hardening options
#
# Explanation of levels:
# 0 : do nothing, not recommended.
# 1 : enable features that have no (or nearly no) performance impact,
# recommended default for optimized, shipping code.
# 2 : enable features that trade off performance for security, recommended
# for debugging or deploying in security-sensitive environments.
# 3 : enable features that have a significant performance impact, only
# recommended for debugging.
#
# Some documentation:
# https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
# https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html
# https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
# https://libcxx.llvm.org/Hardening.html
#
if (${CMAKE_BUILD_TYPE} STREQUAL "Debug")
set (${PROJ_NAME}_HARDENING_DEFAULT 3)
else ()
set (${PROJ_NAME}_HARDENING_DEFAULT 1)
endif ()
set_cache (${PROJ_NAME}_HARDENING ${${PROJ_NAME}_HARDENING_DEFAULT}
"Turn on security hardening features 0, 1, 2, 3")
# Implementation:
if (HARDENING GREATER_EQUAL 1)
# Features that should not detectably affect performance
if (COMPILER_IS_GCC_OR_ANY_CLANG)
# Enable PIE and pie to build as position-independent executables,
# needed for address space randomiztion used by some kernels.
add_compile_options (-fPIE -pie)
add_link_options (-fPIE -pie)
# Protect against stack overwrites. Is allegedly not a performance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you measured this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly. I'm trusting the docs of the compilers and the fact that many distros are starting to build their compilers to have these enabled by default.

The docs say that the pie options were measurable cost on 32 bit platforms, but on 64 is like 0.1%, barely detectable.

If anybody is concerned, then can set OIIO_HARDENING=0 and measure on their own workloads.

# tradeoff.
add_compile_options (-fstack-protector-strong)
add_link_options (-fstack-protector-strong)
endif ()
# Defining _FORTIFY_SOURCE provides buffer overflow checks in modern gcc &
# clang with some compiler-assisted deduction of buffer lengths) for the
# many C functions such as memcpy, strcpy, sprintf, etc. See:
add_compile_definitions (_FORTIFY_SOURCE=${${PROJ_NAME}_HARDENING})
# Setting _LIBCPP_HARDENING_MODE enables various hardening features in
# clang/llvm's libc++ 18.0 and later.
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST)
endif ()
if (HARDENING GREATER_EQUAL 2)
# Features that might impact performance measurably
add_compile_definitions (_GLIBCXX_ASSERTIONS)
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE)
endif ()
if (HARDENING GREATER_EQUAL 3)
# Debugging features that might impact performance significantly
add_compile_definitions (_GLIBCXX_DEBUG)
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG)
endif ()


Expand Down
4 changes: 2 additions & 2 deletions src/cmake/set_utils.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ macro (super_set name value)
set (${name} ${_sce_val} ${_sce_extra_args})
endif ()
if (_sce_VERBOSE)
message (STATUS "${name} = ${${name}}")
message (STATUS "${name} = ${${name}} (${_sce_DOC})")
else ()
message (VERBOSE "${name} = ${${name}}")
message (VERBOSE "${name} = ${${name}} (${_sce_DOC})")
endif ()
if (_sce_ADVANCED)
mark_as_advanced (${name})
Expand Down
7 changes: 5 additions & 2 deletions src/libutil/SHA1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,13 @@ bool CSHA1::ReportHash(TCHAR* tszReport, REPORT_TYPE rtReportType) const
_sntprintf(tszTemp, 15, _T("%02X"), m_digest[0]);
Strutil::safe_strcpy(tszReport, tszTemp, HASH_TEMP_BUFFER_SIZE - 1);

const TCHAR* lpFmt = ((rtReportType == REPORT_HEX) ? _T(" %02X") : _T("%02X"));
// const TCHAR* lpFmt = ((rtReportType == REPORT_HEX) ? _T(" %02X") : _T("%02X"));
for(size_t i = 1; i < 20; ++i)
{
_sntprintf(tszTemp, 15, lpFmt, m_digest[i]);
if (rtReportType == REPORT_HEX)
_sntprintf(tszTemp, 15, _T(" %02X"), m_digest[i]);
else
_sntprintf(tszTemp, 15, _T("%02X"), m_digest[i]);
Strutil::safe_strcat(tszReport, tszTemp, HASH_TEMP_BUFFER_SIZE - 1);
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/tiff.imageio/tiffinput.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,10 @@ class TIFFInput final : public ImageInput {
{
TIFFInput* self = (TIFFInput*)user_data;
spin_lock lock(self->m_last_error_mutex);
OIIO_PRAGMA_WARNING_PUSH
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
self->m_last_error = Strutil::vsprintf(fmt, ap);
OIIO_PRAGMA_WARNING_POP
return 1;
}

Expand All @@ -488,7 +491,10 @@ class TIFFInput final : public ImageInput {
{
TIFFInput* self = (TIFFInput*)user_data;
spin_lock lock(self->m_last_error_mutex);
OIIO_PRAGMA_WARNING_PUSH
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
self->m_last_error = Strutil::vsprintf(fmt, ap);
OIIO_PRAGMA_WARNING_POP
return 1;
}
#endif
Expand Down Expand Up @@ -534,7 +540,10 @@ oiio_tiff_last_error()
static void
my_error_handler(const char* /*str*/, const char* format, va_list ap)
{
OIIO_PRAGMA_WARNING_PUSH
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
oiio_tiff_last_error() = Strutil::vsprintf(format, ap);
OIIO_PRAGMA_WARNING_POP
}


Expand Down
6 changes: 6 additions & 0 deletions src/tiff.imageio/tiffoutput.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ class TIFFOutput final : public ImageOutput {
{
TIFFOutput* self = (TIFFOutput*)user_data;
spin_lock lock(self->m_last_error_mutex);
OIIO_PRAGMA_WARNING_PUSH
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
self->m_last_error = Strutil::vsprintf(fmt, ap);
OIIO_PRAGMA_WARNING_POP
return 1;
}

Expand All @@ -241,7 +244,10 @@ class TIFFOutput final : public ImageOutput {
{
TIFFOutput* self = (TIFFOutput*)user_data;
spin_lock lock(self->m_last_error_mutex);
OIIO_PRAGMA_WARNING_PUSH
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
self->m_last_error = Strutil::vsprintf(fmt, ap);
OIIO_PRAGMA_WARNING_POP
return 1;
}
#endif
Expand Down
Loading