From 177519e0237d8a4e976367ef537b54a6f547d736 Mon Sep 17 00:00:00 2001 From: Cameron Bytheway Date: Thu, 21 Mar 2024 12:36:10 -0600 Subject: [PATCH 1/3] build: add s2n_prelude.h to consolidate defines --- CMakeLists.txt | 11 ++++--- api/s2n.h | 9 ++---- bindings/rust/s2n-tls-sys/build.rs | 16 ++++++--- s2n.mk | 5 +-- tests/features/S2N_MADVISE_SUPPORTED.c | 12 +------ tests/features/S2N_MINHERIT_SUPPORTED.c | 10 +----- utils/s2n_fork_detection.c | 32 +++--------------- utils/s2n_fork_detection_internal.h | 43 +++++++++++++++++++++++++ utils/s2n_prelude.h | 42 ++++++++++++++++++++++++ 9 files changed, 114 insertions(+), 66 deletions(-) create mode 100644 utils/s2n_fork_detection_internal.h create mode 100644 utils/s2n_prelude.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 77e76acc1a9..6ff981fa001 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -192,11 +192,12 @@ if(S2N_BLOCK_NONPORTABLE_OPTIMIZATIONS) target_compile_options(${PROJECT_NAME} PUBLIC -DS2N_BLOCK_NONPORTABLE_OPTIMIZATIONS=1) endif() -target_compile_options(${PROJECT_NAME} PUBLIC -fPIC) +target_compile_options(${PROJECT_NAME} PUBLIC -fPIC -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h") -add_definitions(-D_POSIX_C_SOURCE=200809L) -if(CMAKE_BUILD_TYPE MATCHES Release) - add_definitions(-D_FORTIFY_SOURCE=2) +# Match on Release, RelWithDebInfo and MinSizeRel +# See: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html#variable:CMAKE_BUILD_TYPE +if(CMAKE_BUILD_TYPE MATCHES Rel) + add_definitions(-DS2N_BUILD_RELEASE) endif() if(NO_STACK_PROTECTOR) @@ -331,7 +332,7 @@ function(feature_probe PROBE_NAME) SOURCES "${CMAKE_CURRENT_LIST_DIR}/tests/features/${PROBE_NAME}.c" LINK_LIBRARIES ${LINK_LIB} ${OS_LIBS} CMAKE_FLAGS ${ADDITIONAL_FLAGS} - COMPILE_DEFINITIONS -c ${GLOBAL_FLAGS} ${PROBE_FLAGS} + COMPILE_DEFINITIONS -I "${CMAKE_CURRENT_LIST_DIR}" -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h" -c ${GLOBAL_FLAGS} ${PROBE_FLAGS} ${ARGN} OUTPUT_VARIABLE TRY_COMPILE_OUTPUT ) diff --git a/api/s2n.h b/api/s2n.h index 48f70a6a41a..f104a5ee2af 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -22,17 +22,12 @@ #pragma once -#if ((__GNUC__ >= 4) || defined(__clang__)) && defined(S2N_EXPORTS) - /** - * Marks a function as belonging to the public s2n API. - */ - #define S2N_API __attribute__((visibility("default"))) -#else +#ifndef S2N_API /** * Marks a function as belonging to the public s2n API. */ #define S2N_API -#endif /* __GNUC__ >= 4 || defined(__clang__) */ +#endif #ifdef __cplusplus extern "C" { diff --git a/bindings/rust/s2n-tls-sys/build.rs b/bindings/rust/s2n-tls-sys/build.rs index 32b4efda1ea..d3473a03427 100644 --- a/bindings/rust/s2n-tls-sys/build.rs +++ b/bindings/rust/s2n-tls-sys/build.rs @@ -85,9 +85,14 @@ fn build_vendored() { build.files(include!("./files.rs")); - if env("PROFILE") == "release" { - // fortify source is only available in release mode - build.define("_FORTIFY_SOURCE", "2"); + // https://doc.rust-lang.org/cargo/reference/environment-variables.html + // * OPT_LEVEL, DEBUG — values of the corresponding variables for the profile currently being built. + // * PROFILE — release for release builds, debug for other builds. This is determined based on if + // the profile inherits from the dev or release profile. Using this environment variable is not + // recommended. Using other environment variables like OPT_LEVEL provide a more correct view of + // the actual settings being used. + if env("OPT_LEVEL") != "0" { + build.define("S2N_BUILD_RELEASE", "1"); build.define("NDEBUG", "1"); // build s2n-tls with LTO if supported @@ -166,6 +171,8 @@ fn builder(libcrypto: &Libcrypto) -> cc::Build { }; build + .flag("-include") + .flag("lib/utils/s2n_prelude.h") .flag("-std=c11") .flag("-fgnu89-inline") // make sure the stack is non-executable @@ -173,8 +180,7 @@ fn builder(libcrypto: &Libcrypto) -> cc::Build { .flag_if_supported("-z now") .flag_if_supported("-z noexecstack") // we use some deprecated libcrypto features so don't warn here - .flag_if_supported("-Wno-deprecated-declarations") - .define("_POSIX_C_SOURCE", "200112L"); + .flag_if_supported("-Wno-deprecated-declarations"); build } diff --git a/s2n.mk b/s2n.mk index 0195388eec2..a5c3bdaff93 100644 --- a/s2n.mk +++ b/s2n.mk @@ -49,9 +49,10 @@ endif DEFAULT_CFLAGS += -pedantic -Wall -Werror -Wimplicit -Wunused -Wcomment -Wchar-subscripts -Wuninitialized \ -Wshadow -Wcast-align -Wwrite-strings -fPIC -Wno-missing-braces\ - -D_POSIX_C_SOURCE=200809L -O2 -I$(LIBCRYPTO_ROOT)/include/ \ + -O2 -I$(LIBCRYPTO_ROOT)/include/ \ + -DS2N_BUILD_RELEASE -include utils/s2n_prelude.h \ -I$(S2N_ROOT)/api/ -I$(S2N_ROOT) -Wno-deprecated-declarations -Wno-unknown-pragmas -Wformat-security \ - -D_FORTIFY_SOURCE=2 -fgnu89-inline -fvisibility=hidden -DS2N_EXPORTS + -fgnu89-inline -fvisibility=hidden -DS2N_EXPORTS COVERAGE_CFLAGS = -fprofile-arcs -ftest-coverage COVERAGE_LDFLAGS = --coverage diff --git a/tests/features/S2N_MADVISE_SUPPORTED.c b/tests/features/S2N_MADVISE_SUPPORTED.c index 5b98cd23a38..bdde35c050d 100644 --- a/tests/features/S2N_MADVISE_SUPPORTED.c +++ b/tests/features/S2N_MADVISE_SUPPORTED.c @@ -13,17 +13,7 @@ * permissions and limitations under the License. */ -/* Keep in sync with utils/s2n_fork_detection.c */ -#if defined(__FreeBSD__) - /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) - * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ - #undef _POSIX_C_SOURCE -#elif !defined(__APPLE__) && !defined(_GNU_SOURCE) - #define _GNU_SOURCE -#endif - -#include -#include +#include "utils/s2n_fork_detection_internal.h" int main() { madvise(NULL, 0, 0); diff --git a/tests/features/S2N_MINHERIT_SUPPORTED.c b/tests/features/S2N_MINHERIT_SUPPORTED.c index 13ce6975ae8..25a38253762 100644 --- a/tests/features/S2N_MINHERIT_SUPPORTED.c +++ b/tests/features/S2N_MINHERIT_SUPPORTED.c @@ -13,15 +13,7 @@ * permissions and limitations under the License. */ -/* Keep in sync with utils/s2n_fork_detection.c */ -#if defined(__FreeBSD__) - /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) - * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ - #undef _POSIX_C_SOURCE -#endif - -#include -#include +#include "utils/s2n_fork_detection_internal.h" int main() { minherit(NULL, 0, 0); diff --git a/utils/s2n_fork_detection.c b/utils/s2n_fork_detection.c index 87d66ba4c63..6de67c1ee50 100644 --- a/utils/s2n_fork_detection.c +++ b/utils/s2n_fork_detection.c @@ -13,36 +13,14 @@ * permissions and limitations under the License. */ -/* This captures Darwin specialities. This is the only APPLE flavor we care about. - * Here we also capture varius required feature test macros. - */ -#if defined(__APPLE__) -typedef struct _opaque_pthread_once_t __darwin_pthread_once_t; -typedef __darwin_pthread_once_t pthread_once_t; - #define _DARWIN_C_SOURCE -#elif defined(__FreeBSD__) || defined(__OpenBSD__) - /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) - * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ - #undef _POSIX_C_SOURCE -#elif !defined(_GNU_SOURCE) - /* Keep in sync with feature probe tests/features/madvise.c */ - #define _GNU_SOURCE -#endif +/* force the internal header to be included first, since it modifies _GNU_SOURCE/_POSIX_C_SOURCE */ +/* clang-format off */ +#include "utils/s2n_fork_detection_internal.h" +/* clang-format on */ -#include - -/* Not always defined for Darwin */ -#if !defined(MAP_ANONYMOUS) - #define MAP_ANONYMOUS MAP_ANON -#endif - -#include -#include -#include -#include +#include "utils/s2n_fork_detection.h" #include "error/s2n_errno.h" -#include "utils/s2n_fork_detection.h" #include "utils/s2n_safety.h" #if defined(S2N_MADVISE_SUPPORTED) && defined(MADV_WIPEONFORK) diff --git a/utils/s2n_fork_detection_internal.h b/utils/s2n_fork_detection_internal.h new file mode 100644 index 00000000000..e0d78691b0a --- /dev/null +++ b/utils/s2n_fork_detection_internal.h @@ -0,0 +1,43 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#pragma once + +/* This captures Darwin specialities. This is the only APPLE flavor we care about. + * Here we also capture various required feature test macros. + */ +#if defined(__APPLE__) +typedef struct _opaque_pthread_once_t __darwin_pthread_once_t; +typedef __darwin_pthread_once_t pthread_once_t; + #define _DARWIN_C_SOURCE +#elif defined(__FreeBSD__) || defined(__OpenBSD__) + /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) + * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ + #undef _POSIX_C_SOURCE +#elif !defined(_GNU_SOURCE) + #define _GNU_SOURCE +#endif + +#include + +/* Not always defined for Darwin */ +#if !defined(MAP_ANONYMOUS) + #define MAP_ANONYMOUS MAP_ANON +#endif + +#include +#include +#include +#include diff --git a/utils/s2n_prelude.h b/utils/s2n_prelude.h new file mode 100644 index 00000000000..cbb820c44b5 --- /dev/null +++ b/utils/s2n_prelude.h @@ -0,0 +1,42 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#pragma once + +/* Define the POSIX API we are targeting */ +#ifndef _POSIX_C_SOURCE + #define _POSIX_C_SOURCE 200809L +#endif + +/** + * If we're building in release mode make sure _FORTIFY_SOURCE is set + * See: https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html + * https://man7.org/linux/man-pages/man7/feature_test_macros.7.html + * + * NOTE: _FORTIFY_SOURCE can only be set when optimizations are enabled. + * https://sourceware.org/git/?p=glibc.git;a=commit;f=include/features.h;h=05c2c9618f583ea4acd69b3fe5ae2a2922dd2ddc + */ +#if !defined(_FORTIFY_SOURCE) && defined(S2N_BUILD_RELEASE) + #define _FORTIFY_SOURCE 2 +#endif + +#if ((__GNUC__ >= 4) || defined(__clang__)) && defined(S2N_EXPORTS) + /** + * Marks a function as belonging to the public s2n API. + * + * See: https://gcc.gnu.org/wiki/Visibility + */ + #define S2N_API __attribute__((visibility("default"))) +#endif From 5ddd4c02b43ba5759c4d7cb313c52c05050f606b Mon Sep 17 00:00:00 2001 From: Cameron Bytheway Date: Mon, 28 Oct 2024 14:46:08 -0600 Subject: [PATCH 2/3] add prelude checks --- tests/features/S2N_MADVISE_SUPPORTED.c | 2 +- tests/features/S2N_MINHERIT_SUPPORTED.c | 2 +- tests/unit/s2n_build_test.c | 6 ++++++ tls/s2n_config.c | 5 +++++ utils/s2n_fork_detection.c | 2 +- ...k_detection_internal.h => s2n_fork_detection_features.h} | 0 utils/s2n_prelude.h | 3 +++ 7 files changed, 17 insertions(+), 3 deletions(-) rename utils/{s2n_fork_detection_internal.h => s2n_fork_detection_features.h} (100%) diff --git a/tests/features/S2N_MADVISE_SUPPORTED.c b/tests/features/S2N_MADVISE_SUPPORTED.c index bdde35c050d..312f84627fa 100644 --- a/tests/features/S2N_MADVISE_SUPPORTED.c +++ b/tests/features/S2N_MADVISE_SUPPORTED.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "utils/s2n_fork_detection_internal.h" +#include "utils/s2n_fork_detection_features.h" int main() { madvise(NULL, 0, 0); diff --git a/tests/features/S2N_MINHERIT_SUPPORTED.c b/tests/features/S2N_MINHERIT_SUPPORTED.c index 25a38253762..fc662367f36 100644 --- a/tests/features/S2N_MINHERIT_SUPPORTED.c +++ b/tests/features/S2N_MINHERIT_SUPPORTED.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "utils/s2n_fork_detection_internal.h" +#include "utils/s2n_fork_detection_features.h" int main() { minherit(NULL, 0, 0); diff --git a/tests/unit/s2n_build_test.c b/tests/unit/s2n_build_test.c index e7f40949d15..0c8055bc3bb 100644 --- a/tests/unit/s2n_build_test.c +++ b/tests/unit/s2n_build_test.c @@ -12,6 +12,12 @@ * express or implied. See the License for the specific language governing * permissions and limitations under the License. */ + +#ifndef _S2N_PRELUDE_INCLUDED +/* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ +#error "Expected s2n_prelude.h to be included as part of the compiler flags" +#endif + #define _GNU_SOURCE #include #include diff --git a/tls/s2n_config.c b/tls/s2n_config.c index 4db579dd465..2a23c7617b9 100644 --- a/tls/s2n_config.c +++ b/tls/s2n_config.c @@ -13,6 +13,11 @@ * permissions and limitations under the License. */ +#ifndef _S2N_PRELUDE_INCLUDED +/* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ +#error "Expected s2n_prelude.h to be included as part of the compiler flags" +#endif + #include #include diff --git a/utils/s2n_fork_detection.c b/utils/s2n_fork_detection.c index 6de67c1ee50..92d82cac604 100644 --- a/utils/s2n_fork_detection.c +++ b/utils/s2n_fork_detection.c @@ -15,7 +15,7 @@ /* force the internal header to be included first, since it modifies _GNU_SOURCE/_POSIX_C_SOURCE */ /* clang-format off */ -#include "utils/s2n_fork_detection_internal.h" +#include "utils/s2n_fork_detection_features.h" /* clang-format on */ #include "utils/s2n_fork_detection.h" diff --git a/utils/s2n_fork_detection_internal.h b/utils/s2n_fork_detection_features.h similarity index 100% rename from utils/s2n_fork_detection_internal.h rename to utils/s2n_fork_detection_features.h diff --git a/utils/s2n_prelude.h b/utils/s2n_prelude.h index cbb820c44b5..364cc82ca87 100644 --- a/utils/s2n_prelude.h +++ b/utils/s2n_prelude.h @@ -15,6 +15,9 @@ #pragma once +/* Let modules know that the prelude was included */ +#define _S2N_PRELUDE_INCLUDED + /* Define the POSIX API we are targeting */ #ifndef _POSIX_C_SOURCE #define _POSIX_C_SOURCE 200809L From 86431c9413206d6b948892f1abf9d8f9102b7e68 Mon Sep 17 00:00:00 2001 From: Cameron Bytheway Date: Mon, 28 Oct 2024 14:54:44 -0600 Subject: [PATCH 3/3] fix clang format --- tests/unit/s2n_build_test.c | 4 ++-- tls/s2n_config.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/s2n_build_test.c b/tests/unit/s2n_build_test.c index 0c8055bc3bb..87001ab3078 100644 --- a/tests/unit/s2n_build_test.c +++ b/tests/unit/s2n_build_test.c @@ -14,8 +14,8 @@ */ #ifndef _S2N_PRELUDE_INCLUDED -/* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ -#error "Expected s2n_prelude.h to be included as part of the compiler flags" + /* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ + #error "Expected s2n_prelude.h to be included as part of the compiler flags" #endif #define _GNU_SOURCE diff --git a/tls/s2n_config.c b/tls/s2n_config.c index 2a23c7617b9..ccc1940c0ac 100644 --- a/tls/s2n_config.c +++ b/tls/s2n_config.c @@ -14,8 +14,8 @@ */ #ifndef _S2N_PRELUDE_INCLUDED -/* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ -#error "Expected s2n_prelude.h to be included as part of the compiler flags" + /* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ + #error "Expected s2n_prelude.h to be included as part of the compiler flags" #endif #include