Skip to content

Commit

Permalink
Run Valgrind SCA across GCC/clang and -O1/-O2/-O3/-Os
Browse files Browse the repository at this point in the history
This aims to increase the signal of secret-dependent execution gained
from the CT::poison() helpers by running these Valgrind-based tests
across a range of compilers and optimization flags.

For instance randombit#4107 would have been detectable using this matrix.
  • Loading branch information
reneme committed Oct 31, 2024
1 parent 5e1fdc4 commit cdd6fc3
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 15 deletions.
35 changes: 32 additions & 3 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,35 @@ jobs:

valgrind:
name: "valgrind"
strategy:
fail-fast: false

# Targets:
# - valgrind-full: all tests on valgrind, aiming to catch memory bugs
# - valgrind: reduced set of tests on valgrind, aiming to catch memory bugs
# - valgrind-ct-full: all tests on valgrind, aiming to catch secret-dependent execution issues
# - valgrind-ct: reduced set of tests on valgrind, aiming to catch secret-dependent execution issues

matrix:
# Run a matrix of compiler and optimization flag combinations to maximize
# the signal of secret-dependent execution issues introduced by compilers.
compiler: ["clang", "gcc"]
cxxflags: ["-O1", "-O2", "-O3"]
target: ["valgrind-ct-full"]

include:
- compiler: clang
cxxflags: "" # default compilation flags
target: "valgrind-full" # memory bug detection
- compiler: clang
cxxflags: "-Os"
# Clang's -Os generated binary is fast enough to run the full test suite.
target: "valgrind-ct-full"
- compiler: gcc
cxxflags: "-Os"
# GCC with -Os generates a much slower binary, that won't finish
# before timing out on GH Actions, so we run a reduced set of tests.
target: "valgrind-ct"

runs-on: ubuntu-24.04

Expand All @@ -158,11 +187,11 @@ jobs:
- name: Setup Build Agent
uses: ./.github/actions/setup-build-agent
with:
target: valgrind-full
cache-key: linux-x86_64-valgrind-full
target: ${{ matrix.target }}
cache-key: linux-x86_64-${{ matrix.compiler }}-${{ matrix.target }}-${{ matrix.cxxflags }}

- name: Valgrind Checks
run: python3 ./src/scripts/ci_build.py --cc=clang --make-tool=make valgrind-full
run: python3 ./src/scripts/ci_build.py --make-tool=make --cc=${{ matrix.compiler }} --custom-optimization-flags="${{ matrix.cxxflags }}" ${{ matrix.target }}

hybrid_tls_interop:
name: "PQ/T TLS 1.3"
Expand Down
2 changes: 1 addition & 1 deletion src/scripts/ci/setup_gh_actions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ if type -p "apt-get"; then
sudo apt-get -qq update
sudo apt-get -qq install ccache libbz2-dev liblzma-dev libsqlite3-dev

if [ "$TARGET" = "valgrind" ] || [ "$TARGET" = "valgrind-full" ]; then
if [ "$TARGET" = "valgrind" ] || [ "$TARGET" = "valgrind-full" ] || [ "$TARGET" = "valgrind-ct-full" ] || [ "$TARGET" = "valgrind-ct" ]; then
# (l)ist mode (avoiding https://github.com/actions/runner-images/issues/9996)
sudo NEEDRESTART_MODE=l apt-get -qq install valgrind
build_and_install_jitterentropy
Expand Down
36 changes: 25 additions & 11 deletions src/scripts/ci_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ def known_targets():
'static',
'valgrind',
'valgrind-full',
'valgrind-ct',
'valgrind-ct-full',
]

def is_running_in_github_actions():
Expand Down Expand Up @@ -123,12 +125,12 @@ def build_targets(target, target_os):
yield 'bogo_shim'
if target in ['examples', 'amalgamation']:
yield 'examples'
if target in ['valgrind', 'valgrind-full']:
if target in ['valgrind', 'valgrind-full', 'valgrind-ct', 'valgrind-ct-full']:
yield 'ct_selftest'

def determine_flags(target, target_os, target_cpu, target_cc, cc_bin, ccache,
root_dir, build_dir, test_results_dir, pkcs11_lib, use_gdb,
disable_werror, extra_cxxflags, disabled_tests):
disable_werror, extra_cxxflags, custom_optimization_flags, disabled_tests):

"""
Return the configure.py flags as well as make/test running prefixes
Expand Down Expand Up @@ -198,6 +200,11 @@ def sanitize_kv(some_string):
if target_cpu is not None:
flags += ['--cpu=%s' % (target_cpu)]

if custom_optimization_flags and any(flag for flag in custom_optimization_flags):
flags += ['--no-optimizations']
for flag in custom_optimization_flags:
flags += ['--extra-cxxflags=%s' % (flag)]

for flag in extra_cxxflags:
flags += ['--extra-cxxflags=%s' % (flag)]

Expand Down Expand Up @@ -247,23 +254,28 @@ def sanitize_kv(some_string):
if target in ['sde']:
test_prefix = ['sde', '-future', '--']

if target in ['valgrind', 'valgrind-full']:
if target in ['valgrind', 'valgrind-full', 'valgrind-ct', 'valgrind-ct-full']:
flags += ['--with-valgrind']

test_prefix = ['valgrind',
'-v',
'--error-exitcode=9',
'--leak-check=full',
'--show-reachable=yes',
'--track-origins=yes']
'--error-exitcode=9']

# For finding memory bugs, we're enabling more features that add runtime
# overhead which we don't need for the secret-dependent execution checks
# that 'valgrind-ct' and 'valgrind-ct-full' are aiming for.
if target not in ['valgrind-ct', 'valgrind-ct-full']:
test_prefix += ['--leak-check=full',
'--show-reachable=yes',
'--track-origins=yes']

build_config = os.path.join(build_dir, 'build', 'build_config.json')
pretest_cmd = ['python3', os.path.join(root_dir, 'src', 'ct_selftest', 'ct_selftest.py'), "--build-config-path=%s" % build_config, os.path.join(build_dir, 'botan_ct_selftest')]

# valgrind is single threaded anyway
test_cmd += ['--test-threads=1']

if target != 'valgrind-full':
if target not in ['valgrind-full', 'valgrind-ct-full']:
# valgrind is slow, so some tests only run in the nightly check
slow_tests = [
'argon2', 'bcrypt', 'bcrypt_pbkdf', 'compression_tests', 'cryptobox',
Expand Down Expand Up @@ -301,7 +313,7 @@ def sanitize_kv(some_string):
else:
flags += ['--enable-sanitizers=address']

if target in ['valgrind', 'valgrind-full', 'sanitizer', 'fuzzers']:
if target in ['valgrind', 'valgrind-full', 'valgrind-ct', 'valgrind-ct-full', 'sanitizer', 'fuzzers']:
flags += ['--disable-modules=locking_allocator']

if target == 'emscripten':
Expand Down Expand Up @@ -610,6 +622,8 @@ def parse_args(args):

parser.add_option('--extra-cxxflags', metavar='FLAGS', default=[], action='append',
help='Specify extra build flags')
parser.add_option('--custom-optimization-flags', metavar='FLAGS', default=[], action='append',
help='Specify custom optimization flags (disables all default optimizations)')

parser.add_option('--disabled-tests', metavar='DISABLED_TESTS', default=[], action='append',
help='Comma separated list of tests that should not be run')
Expand Down Expand Up @@ -781,7 +795,7 @@ def main(args=None):
target, options.os, options.cpu, options.cc, options.cc_bin,
options.compiler_cache, root_dir, build_dir, options.test_results_dir,
options.pkcs11_lib, options.use_gdb, options.disable_werror,
options.extra_cxxflags, options.disabled_tests)
options.extra_cxxflags, options.custom_optimization_flags, options.disabled_tests)

make_tool, make_opts = validate_make_tool(options.make_tool, options.build_jobs)

Expand Down Expand Up @@ -809,7 +823,7 @@ def main(args=None):
if target in ['examples', 'amalgamation']:
make_targets += ['examples']

if target in ['valgrind', 'valgrind-full']:
if target in ['valgrind', 'valgrind-full', 'valgrind-ct', 'valgrind-ct-full']:
make_targets += ['ct_selftest']

if target in ['coverage', 'sanitizer'] and options.os not in ['windows']:
Expand Down

0 comments on commit cdd6fc3

Please sign in to comment.