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.

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 s2n_prelude.h to consolidate defines #4465

Merged
merged 5 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
11 changes: 6 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be like this:

Suggested change
target_compile_options(${PROJECT_NAME} PUBLIC -fPIC -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h")
target_compile_options(${PROJECT_NAME} PUBLIC -fPIC)
target_compile_options(${PROJECT_NAME} PRIVATE -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h")

By making this PUBLIC, any project that depends, directly or indirectly, on S2N gets this compile option added to all their .c/.cpp files


Comment on lines +195 to 196
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if we accidentally remove this "include"? Does it fail loudly?

And as discussed offline, maybe an assert or a test would give a nice clear failure.

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)
goatgoose marked this conversation as resolved.
Show resolved Hide resolved
add_definitions(-DS2N_BUILD_RELEASE)
endif()

if(NO_STACK_PROTECTOR)
Expand Down Expand Up @@ -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
)
Expand Down
9 changes: 2 additions & 7 deletions api/s2n.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
goatgoose marked this conversation as resolved.
Show resolved Hide resolved
#endif /* __GNUC__ >= 4 || defined(__clang__) */
#endif

#ifdef __cplusplus
extern "C" {
Expand Down
16 changes: 11 additions & 5 deletions bindings/rust/s2n-tls-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -166,15 +171,16 @@ 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
.flag_if_supported("-z relro")
.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
}
Expand Down
5 changes: 3 additions & 2 deletions s2n.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 1 addition & 11 deletions tests/features/S2N_MADVISE_SUPPORTED.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, <sys/mman.h> cannot be imported (it requires __BSD_VISIBLE) */
#undef _POSIX_C_SOURCE
#elif !defined(__APPLE__) && !defined(_GNU_SOURCE)
#define _GNU_SOURCE
#endif

#include <stddef.h>
#include <sys/mman.h>
#include "utils/s2n_fork_detection_internal.h"

int main() {
madvise(NULL, 0, 0);
Expand Down
10 changes: 1 addition & 9 deletions tests/features/S2N_MINHERIT_SUPPORTED.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, <sys/mman.h> cannot be imported (it requires __BSD_VISIBLE) */
#undef _POSIX_C_SOURCE
#endif

#include <stddef.h>
#include <sys/mman.h>
#include "utils/s2n_fork_detection_internal.h"

int main() {
minherit(NULL, 0, 0);
Expand Down
32 changes: 5 additions & 27 deletions utils/s2n_fork_detection.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, <sys/mman.h> 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 <sys/mman.h>

/* Not always defined for Darwin */
#if !defined(MAP_ANONYMOUS)
#define MAP_ANONYMOUS MAP_ANON
#endif

#include <pthread.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#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)
Expand Down
43 changes: 43 additions & 0 deletions utils/s2n_fork_detection_internal.h
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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, <sys/mman.h> cannot be imported (it requires __BSD_VISIBLE) */
#undef _POSIX_C_SOURCE
#elif !defined(_GNU_SOURCE)
#define _GNU_SOURCE
#endif

#include <sys/mman.h>

/* Not always defined for Darwin */
#if !defined(MAP_ANONYMOUS)
#define MAP_ANONYMOUS MAP_ANON
#endif

#include <pthread.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
42 changes: 42 additions & 0 deletions utils/s2n_prelude.h
Original file line number Diff line number Diff line change
@@ -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
Loading