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

Jump to bottom

ci: add more libcryptos for fuzz batch & follow cmake idioms #4795

Merged
merged 11 commits into from
Oct 10, 2024
Merged

ci: add more libcryptos for fuzz batch & follow cmake idioms #4795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8ce9f26
additional libcrypto to be built with
jouho Sep 23, 2024
687e920
fix typo
jouho Sep 23, 2024
764a564
follow cmake build idioms
jouho Sep 24, 2024
3337157
try awslc-fips-2022
jouho Sep 24, 2024
94268dc
remove awslc-fips-2022
jouho Sep 25, 2024
e9c2b53
Merge branch 'main' into additional-libcryptos-to-fuzzing
jouho Sep 25, 2024
c38b095
address PR feedback
jouho Sep 25, 2024
99dc431
address PR feedback
jouho Sep 25, 2024
29f77e1
use relative path from cmake source
jouho Sep 26, 2024
0fb3b4b
Merge branch 'main' into additional-libcryptos-to-fuzzing
jouho Oct 10, 2024
7a78285
Merge branch 'main' into additional-libcryptos-to-fuzzing
jouho Oct 10, 2024
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
30 changes: 10 additions & 20 deletions CMakeLists.txt
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Modified CMakeLists.txt to remove logic that generates test binaries and library files in specific folders

Do you know why it was using specific folders before? Was is possibly due to concerns about name collisions / to avoid overwriting a non-fuzzing library or binary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a result of inheriting make build configuration. CMake by default stores executables and libraries under build/ folder. In order to mimic the behavior of make, I had to specify specific folder to store these files. Now that I am making things a bit more "cmaky", I am removing that logic.

I couldn't find any mention of the original intention for where to store these files, but that is a good call-out. Should we store all the fuzz-related binaries under a specific folder, something like build/fuzz/bin for executables and build/fuzz/lib for libraries?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to keep the cmake standard paths, but we should make sure LD_PRELOAD is getting set to the actual libraries and not just the build/lib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should make sure LD_PRELOAD is getting set to the actual libraries and not just the build/lib

What do you mean by this? Currently the .so files are being generated in build/lib, and we set the LD_PRELOAD path to be "${BUILD_DIR_PATH}/lib/lib${TEST_NAME}_overrides.so" and "${BUILD_DIR_PATH}/lib/libglobal_overrides.so". Do you suggest storing the .so files elsewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, that's great.

To Lindsay's point, moving everything to a generic build/lib would have been a problem if we were only specifying the LD_PRELOAD by folder (LD_PRELOAD=build/lib/), because then shenanigans would ensue, and all of the test specific overrides would be pulled in for all tests.

Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ if (BUILD_TESTING)
if(S2N_FUZZ_TEST)
message(STATUS "Fuzz build enabled")
set(SCRIPT_PATH "${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz/runFuzzTest.sh")
set(BUILD_DIR_PATH "${CMAKE_CURRENT_SOURCE_DIR}/build")
file(GLOB FUZZ_TEST_SRCS "${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz/*.c")

file(GLOB TESTLIB_SRC "tests/testlib/*.c")
Expand Down Expand Up @@ -688,7 +689,6 @@ if (BUILD_TESTING)
endif()

# Build LD_PRELOAD shared libraries
set(LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz/LD_PRELOAD)
file(GLOB LIBRARY_SRCS "${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz/LD_PRELOAD/*.c")
foreach(SRC ${LIBRARY_SRCS})
get_filename_component(LIB_NAME ${SRC} NAME_WE)
Expand All @@ -697,14 +697,6 @@ if (BUILD_TESTING)
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/api
)
# Set the output directory and remove the default "lib" prefix
set_target_properties(${LIB_NAME} PROPERTIES
PREFIX ""
LIBRARY_OUTPUT_DIRECTORY ${LIBRARY_OUTPUT_DIRECTORY}
)
target_compile_options(${LIB_NAME} PRIVATE
-fPIC
)
endforeach()

set(CMAKE_C_COMPILER clang)
Expand All @@ -724,18 +716,16 @@ if (BUILD_TESTING)
fuzztest
)

# Set the output directory for the fuzzing binaries
set(FUZZ_BIN_DIR "${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz")
set_target_properties(${TEST_NAME} PROPERTIES
RUNTIME_OUTPUT_DIRECTORY ${FUZZ_BIN_DIR}
)

add_test(NAME ${TEST_NAME}
COMMAND ${CMAKE_COMMAND} -E env
DYLD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}/lib:${CMAKE_CURRENT_BINARY_DIR}/tests/testlib:${CMAKE_CURRENT_SOURCE_DIR}/libcrypto-root/lib:$ENV{DYLD_LIBRARY_PATH}
LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}/lib:${CMAKE_CURRENT_BINARY_DIR}/tests/testlib:${CMAKE_CURRENT_SOURCE_DIR}/libcrypto-root/lib:$ENV{LD_LIBRARY_PATH}
bash ${SCRIPT_PATH} ${TEST_NAME} ${FUZZ_TIMEOUT_SEC} ${CORPUS_UPLOAD_LOC} ${ARTIFACT_UPLOAD_LOC}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz)
COMMAND ${CMAKE_COMMAND} -E env
bash ${SCRIPT_PATH}
${TEST_NAME}
${FUZZ_TIMEOUT_SEC}
${BUILD_DIR_PATH}
${CORPUS_UPLOAD_LOC}
jouho marked this conversation as resolved.
Show resolved Hide resolved
${ARTIFACT_UPLOAD_LOC}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz
)
set_property(TEST ${TEST_NAME} PROPERTY LABELS "fuzz")
endforeach()
endif()
Expand Down
26 changes: 24 additions & 2 deletions codebuild/spec/buildspec_fuzz_batch.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ version: 0.2
# Parameter motivation

# LIBCRYPTOS
# awslc: happy path libcrypto for s2n-tls
# openssl 3: libcrypto that is widely used
# awslc: happy path libcrypto for s2n-tls
# openssl 1.0.2: old version of libcrypto that is still supported by s2n-tls
# openssl 1.1.1: old version of libcrypto that is still supported by s2n-tls
# openssl 3: libcrypto that is widely used

batch:
build-list:
Expand All @@ -37,6 +39,26 @@ batch:
variables:
S2N_LIBCRYPTO: awslc
COMPILER: clang
- identifier: clang_openssl_1_0_2
buildspec: codebuild/spec/buildspec_fuzz.yml
debug-session: true
env:
compute-type: BUILD_GENERAL1_XLARGE
image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
privileged-mode: true
variables:
S2N_LIBCRYPTO: openssl-1.0.2
COMPILER: clang
- identifier: clang_openssl_1_1_1
buildspec: codebuild/spec/buildspec_fuzz.yml
debug-session: true
env:
compute-type: BUILD_GENERAL1_XLARGE
image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
privileged-mode: true
variables:
S2N_LIBCRYPTO: openssl-1.1.1
COMPILER: clang
- identifier: clang_openssl_3_0
buildspec: codebuild/spec/buildspec_fuzz.yml
debug-session: true
Expand Down
22 changes: 14 additions & 8 deletions tests/fuzz/runFuzzTest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ usage() {
exit 1
}

if [ "$#" -ne "4" ]; then
if [ "$#" -ne "5" ]; then
usage
fi

TEST_NAME=$1
FUZZ_TIMEOUT_SEC=$2
CORPUS_UPLOAD_LOC=$3
ARTIFACT_UPLOAD_LOC=$4
BUILD_DIR_PATH=$3
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these arguments going to be updated in a future PR? or rather, can I review how this script is being called somewhere ?

Copy link
Contributor Author

@jouho jouho Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This script is getting called in CMakeList.txt here:
https://github.com/jouho/s2n-tls/blob/cd9eaa5fd589e7e8e785203c7dc4792e25f56436/CMakeLists.txt#L673

SCRIPT_PATH is the path to runFuzzTest.sh. Does this answer your question?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only in CMakeList.txt? No existing codebuild job is calling it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only ran in CMake after we migrated away from make. codebuild jobs will indirectly call it through cmake.

CORPUS_UPLOAD_LOC=$4
ARTIFACT_UPLOAD_LOC=$5
MIN_TEST_PER_SEC="1000"
MIN_FEATURES_COVERED="100"

Expand All @@ -47,8 +48,8 @@ UBSAN_OPTIONS+="print_stacktrace=1"
NUM_CPU_THREADS=$(nproc)
LIBFUZZER_ARGS+="-timeout=5 -max_len=4096 -print_final_stats=1 -jobs=${NUM_CPU_THREADS} -workers=${NUM_CPU_THREADS} -max_total_time=${FUZZ_TIMEOUT_SEC}"

TEST_SPECIFIC_OVERRIDES="${PWD}/LD_PRELOAD/${TEST_NAME}_overrides.so"
GLOBAL_OVERRIDES="${PWD}/LD_PRELOAD/global_overrides.so"
TEST_SPECIFIC_OVERRIDES="${BUILD_DIR_PATH}/lib/lib${TEST_NAME}_overrides.so"
GLOBAL_OVERRIDES="${BUILD_DIR_PATH}/lib/libglobal_overrides.so"

FUZZCOV_SOURCES="${S2N_ROOT}/api ${S2N_ROOT}/bin ${S2N_ROOT}/crypto ${S2N_ROOT}/error ${S2N_ROOT}/stuffer ${S2N_ROOT}/tls ${S2N_ROOT}/utils"

Expand Down Expand Up @@ -104,9 +105,13 @@ fi
if [[ "$FUZZ_COVERAGE" == "true" ]]; then
mkdir -p "./profiles/${TEST_NAME}"
rm -f ./profiles/${TEST_NAME}/*.profraw
LLVM_PROFILE_FILE="./profiles/${TEST_NAME}/${TEST_NAME}.%p.profraw" ./${TEST_NAME} ${LIBFUZZER_ARGS} ${TEMP_CORPUS_DIR} > ${TEST_NAME}_output.txt 2>&1 || ACTUAL_TEST_FAILURE=1
LLVM_PROFILE_FILE="./profiles/${TEST_NAME}/${TEST_NAME}.%p.profraw" \
${BUILD_DIR_PATH}/bin/${TEST_NAME} ${LIBFUZZER_ARGS} ${TEMP_CORPUS_DIR} \
> ${TEST_NAME}_output.txt 2>&1 || ACTUAL_TEST_FAILURE=1
else
env LD_PRELOAD="$LD_PRELOAD_" ./${TEST_NAME} ${LIBFUZZER_ARGS} ${TEMP_CORPUS_DIR} > ${TEST_NAME}_output.txt 2>&1 || ACTUAL_TEST_FAILURE=1
env LD_PRELOAD="$LD_PRELOAD_" \
${BUILD_DIR_PATH}/bin/${TEST_NAME} ${LIBFUZZER_ARGS} ${TEMP_CORPUS_DIR} \
> ${TEST_NAME}_output.txt 2>&1 || ACTUAL_TEST_FAILURE=1
fi

TEST_INFO=$(
Expand Down Expand Up @@ -171,7 +176,8 @@ then
else
# TEMP_CORPUS_DIR may contain many new inputs that only covers a small set of new branches.
# Instead of copying all new inputs to the corpus directory, only copy back minimum number of new inputs that reach new branches.
./${TEST_NAME} -merge=1 "./corpus/${TEST_NAME}" "${TEMP_CORPUS_DIR}" > ${TEST_NAME}_results.txt 2>&1
${BUILD_DIR_PATH}/bin/${TEST_NAME} -merge=1 "./corpus/${TEST_NAME}" "${TEMP_CORPUS_DIR}" \
> ${TEST_NAME}_results.txt 2>&1

# Print number of new files and branches found in new Inputs (if any)
RESULTS=`grep -Eo "[0-9]+ new files .*$" ${TEST_NAME}_results.txt | tail -1`
Expand Down
Loading