-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
gh-135401: Test AWS-LC as a cryptography library in CI #135402
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
base: main
Are you sure you want to change the base?
Changes from all commits
57209fe
8fb1016
67fd836
4f0928b
b65d662
6791473
269dc10
cd74e2b
24fbecf
fa08737
3f3a70b
7d37e6a
6eb1190
8f4a0eb
7ebee26
840923d
3850ba0
be1b72c
c655484
99df7d5
8f95caa
31506be
4312b5a
f4968da
3134a9e
f8fde35
eb11bca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -260,7 +260,7 @@ jobs: | |
free-threading: ${{ matrix.free-threading }} | ||
os: ${{ matrix.os }} | ||
|
||
build-ubuntu-ssltests: | ||
build-ubuntu-ssltests-openssl: | ||
name: 'Ubuntu SSL tests with OpenSSL' | ||
runs-on: ${{ matrix.os }} | ||
timeout-minutes: 60 | ||
|
@@ -322,6 +322,81 @@ jobs: | |
- name: SSL tests | ||
run: ./python Lib/test/ssltests.py | ||
|
||
build-ubuntu-ssltests-awslc: | ||
name: 'Ubuntu SSL tests with AWS-LC' | ||
runs-on: ${{ matrix.os }} | ||
timeout-minutes: 60 | ||
needs: build-context | ||
if: needs.build-context.outputs.run-tests == 'true' | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
os: [ubuntu-24.04] | ||
awslc_ver: [1.55.0] | ||
env: | ||
AWSLC_VER: ${{ matrix.awslc_ver}} | ||
MULTISSL_DIR: ${{ github.workspace }}/multissl | ||
OPENSSL_DIR: ${{ github.workspace }}/multissl/aws-lc/${{ matrix.awslc_ver }} | ||
LD_LIBRARY_PATH: ${{ github.workspace }}/multissl/aws-lc/${{ matrix.awslc_ver }}/lib | ||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
persist-credentials: false | ||
- name: Runner image version | ||
run: echo "IMAGE_OS_VERSION=${ImageOS}-${ImageVersion}" >> "$GITHUB_ENV" | ||
- name: Restore config.cache | ||
uses: actions/cache@v4 | ||
with: | ||
path: config.cache | ||
key: ${{ github.job }}-${{ env.IMAGE_OS_VERSION }}-${{ needs.build-context.outputs.config-hash }} | ||
- name: Register gcc problem matcher | ||
run: echo "::add-matcher::.github/problem-matchers/gcc.json" | ||
- name: Install dependencies | ||
run: sudo ./.github/workflows/posix-deps-apt.sh | ||
- name: Configure SSL lib env vars | ||
run: | | ||
echo "MULTISSL_DIR=${GITHUB_WORKSPACE}/multissl" >> "$GITHUB_ENV" | ||
echo "OPENSSL_DIR=${GITHUB_WORKSPACE}/multissl/aws-lc/${AWSLC_VER}" >> "$GITHUB_ENV" | ||
echo "LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/multissl/aws-lc/${AWSLC_VER}/lib" >> "$GITHUB_ENV" | ||
- name: 'Restore AWS-LC build' | ||
id: cache-aws-lc | ||
uses: actions/cache@v4 | ||
with: | ||
path: ./multissl/aws-lc/${{ matrix.awslc_ver }} | ||
key: ${{ matrix.os }}-multissl-aws-lc-${{ matrix.awslc_ver }} | ||
- name: Install AWS-LC | ||
if: steps.cache-aws-lc.outputs.cache-hit != 'true' | ||
run: | | ||
python3 Tools/ssl/multissltests.py \ | ||
--steps=library \ | ||
--base-directory "$MULTISSL_DIR" \ | ||
--awslc ${{ matrix.awslc_ver }} \ | ||
--system Linux | ||
- name: Add ccache to PATH | ||
run: | | ||
echo "PATH=/usr/lib/ccache:$PATH" >> "$GITHUB_ENV" | ||
- name: Configure ccache action | ||
uses: hendrikmuhs/[email protected] | ||
with: | ||
save: false | ||
- name: Configure CPython | ||
run: | | ||
./configure CFLAGS="-fdiagnostics-format=json" \ | ||
--config-cache \ | ||
--enable-slower-safety \ | ||
--with-pydebug \ | ||
--with-openssl="$OPENSSL_DIR" \ | ||
--with-builtin-hashlib-hashes=blake2 \ | ||
--with-ssl-default-suites=openssl | ||
- name: Build CPython | ||
run: make -j | ||
- name: Display build info | ||
run: make pythoninfo | ||
- name: Verify python is linked to AWS-LC | ||
run: ./python -c 'import ssl; print(ssl.OPENSSL_VERSION)' | grep AWS-LC | ||
- name: SSL tests | ||
run: ./python Lib/test/ssltests.py | ||
|
||
build-wasi: | ||
name: 'WASI' | ||
needs: build-context | ||
|
@@ -628,7 +703,8 @@ jobs: | |
- build-windows-msi | ||
- build-macos | ||
- build-ubuntu | ||
- build-ubuntu-ssltests | ||
- build-ubuntu-ssltests-awslc | ||
- build-ubuntu-ssltests-openssl | ||
- build-wasi | ||
- test-hypothesis | ||
- build-asan | ||
|
@@ -643,7 +719,8 @@ jobs: | |
with: | ||
allowed-failures: >- | ||
build-windows-msi, | ||
build-ubuntu-ssltests, | ||
build-ubuntu-ssltests-awslc, | ||
build-ubuntu-ssltests-openssl, | ||
test-hypothesis, | ||
cifuzz, | ||
allowed-skips: >- | ||
|
@@ -661,7 +738,8 @@ jobs: | |
check-generated-files, | ||
build-macos, | ||
build-ubuntu, | ||
build-ubuntu-ssltests, | ||
build-ubuntu-ssltests-awslc, | ||
build-ubuntu-ssltests-openssl, | ||
build-wasi, | ||
test-hypothesis, | ||
build-asan, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ apt-get -yq install \ | |
build-essential \ | ||
pkg-config \ | ||
ccache \ | ||
cmake \ | ||
gdb \ | ||
lcov \ | ||
libb2-dev \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Add a new GitHub CI job to test the :py:mod:`ssl` module with AWS-LC__ as the backing cryptography and TLS library. | ||
|
||
__ https://github.com/aws/aws-lc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use an inline link instead of an anonymous one. This can be achieved as follows IIRC (check if this is correct) : `text <link>`_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
#!./python | ||
"""Run Python tests against multiple installations of OpenSSL and LibreSSL | ||
"""Run Python tests against multiple installations of cryptography libraries | ||
|
||
The script | ||
|
||
(1) downloads OpenSSL / LibreSSL tar bundle | ||
(1) downloads the tar bundle | ||
(2) extracts it to ./src | ||
(3) compiles OpenSSL / LibreSSL | ||
(4) installs OpenSSL / LibreSSL into ../multissl/$LIB/$VERSION/ | ||
(3) compiles the relevant library | ||
(4) installs that library into ../multissl/$LIB/$VERSION/ | ||
(5) forces a recompilation of Python modules using the | ||
header and library files from ../multissl/$LIB/$VERSION/ | ||
(6) runs Python's test suite | ||
|
@@ -61,6 +61,10 @@ | |
LIBRESSL_RECENT_VERSIONS = [ | ||
] | ||
|
||
AWSLC_RECENT_VERSIONS = [ | ||
"1.55.0", | ||
] | ||
|
||
# store files in ../multissl | ||
HERE = os.path.dirname(os.path.abspath(__file__)) | ||
PYTHONROOT = os.path.abspath(os.path.join(HERE, '..', '..')) | ||
|
@@ -70,7 +74,7 @@ | |
parser = argparse.ArgumentParser( | ||
prog='multissl', | ||
description=( | ||
"Run CPython tests with multiple OpenSSL and LibreSSL " | ||
"Run CPython tests with multiple cryptography libraries" | ||
"versions." | ||
) | ||
) | ||
|
@@ -102,6 +106,14 @@ | |
"OpenSSL and LibreSSL versions are given." | ||
).format(LIBRESSL_RECENT_VERSIONS, LIBRESSL_OLD_VERSIONS) | ||
) | ||
parser.add_argument( | ||
'--awslc', | ||
nargs='+', | ||
default=(), | ||
help=( | ||
"AWS-LC versions, defaults to '{}'." | ||
).format(AWSLC_RECENT_VERSIONS) | ||
) | ||
parser.add_argument( | ||
'--tests', | ||
nargs='*', | ||
|
@@ -111,7 +123,7 @@ | |
parser.add_argument( | ||
'--base-directory', | ||
default=MULTISSL_DIR, | ||
help="Base directory for OpenSSL / LibreSSL sources and builds." | ||
help="Base directory for crypto library sources and builds." | ||
) | ||
parser.add_argument( | ||
'--no-network', | ||
|
@@ -124,8 +136,8 @@ | |
choices=['library', 'modules', 'tests'], | ||
default='tests', | ||
help=( | ||
"Which steps to perform. 'library' downloads and compiles OpenSSL " | ||
"or LibreSSL. 'module' also compiles Python modules. 'tests' builds " | ||
"Which steps to perform. 'library' downloads and compiles a crypto" | ||
"library. 'module' also compiles Python modules. 'tests' builds " | ||
"all and runs the test suite." | ||
) | ||
) | ||
|
@@ -453,6 +465,34 @@ class BuildLibreSSL(AbstractBuilder): | |
build_template = "libressl-{}" | ||
|
||
|
||
class BuildAWSLC(AbstractBuilder): | ||
library = "AWS-LC" | ||
url_templates = ( | ||
"https://github.com/aws/aws-lc/archive/refs/tags/v{v}.tar.gz", | ||
) | ||
src_template = "aws-lc-{}.tar.gz" | ||
build_template = "aws-lc-{}" | ||
|
||
def _build_src(self, config_args=()): | ||
cwd = self.build_dir | ||
log.info("Running build in {}".format(cwd)) | ||
env = os.environ.copy() | ||
env["LD_RUN_PATH"] = self.lib_dir # set rpath | ||
if self.system: | ||
env['SYSTEM'] = self.system | ||
cmd = [ | ||
"cmake", | ||
"-DCMAKE_BUILD_TYPE=RelWithDebInfo", | ||
"-DCMAKE_PREFIX_PATH={}".format(self.install_dir), | ||
"-DCMAKE_INSTALL_PREFIX={}".format(self.install_dir), | ||
"-DBUILD_SHARED_LIBS=ON", | ||
"-DBUILD_TESTING=OFF", | ||
"-DFIPS=OFF", | ||
] | ||
self._subprocess_call(cmd, cwd=cwd, env=env) | ||
self._subprocess_call(["make", f"-j{self.jobs}"], cwd=cwd, env=env) | ||
|
||
|
||
def configure_make(): | ||
if not os.path.isfile('Makefile'): | ||
log.info('Running ./configure') | ||
|
@@ -467,9 +507,10 @@ def configure_make(): | |
|
||
def main(): | ||
args = parser.parse_args() | ||
if not args.openssl and not args.libressl: | ||
if not args.openssl and not args.libressl and not args.awslc: | ||
args.openssl = list(OPENSSL_RECENT_VERSIONS) | ||
args.libressl = list(LIBRESSL_RECENT_VERSIONS) | ||
args.awslc = list(AWSLC_RECENT_VERSIONS) | ||
Comment on lines
+510
to
+513
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like the rigidity of this as it scales poorly when we introduce more implementations (I don't see BoringSSL for instance). But let's keep this as is and I'll refactor this script in a separate PR. |
||
if not args.disable_ancient: | ||
args.openssl.extend(OPENSSL_OLD_VERSIONS) | ||
args.libressl.extend(LIBRESSL_OLD_VERSIONS) | ||
|
@@ -496,22 +537,15 @@ def main(): | |
|
||
# download and register builder | ||
builds = [] | ||
|
||
for version in args.openssl: | ||
build = BuildOpenSSL( | ||
version, | ||
args | ||
) | ||
build.install() | ||
builds.append(build) | ||
|
||
for version in args.libressl: | ||
build = BuildLibreSSL( | ||
version, | ||
args | ||
) | ||
build.install() | ||
builds.append(build) | ||
for build_class, versions in [ | ||
(BuildOpenSSL, args.openssl), | ||
(BuildLibreSSL, args.libressl), | ||
(BuildAWSLC, args.awslc), | ||
]: | ||
for version in versions: | ||
build = build_class(version, args) | ||
build.install() | ||
builds.append(build) | ||
|
||
if args.steps in {'modules', 'tests'}: | ||
for build in builds: | ||
|
@@ -539,7 +573,7 @@ def main(): | |
else: | ||
print('Executed all SSL tests.') | ||
|
||
print('OpenSSL / LibreSSL versions:') | ||
print('OpenSSL / LibreSSL / AWS-LC versions:') | ||
for build in builds: | ||
print(" * {0.library} {0.version}".format(build)) | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ISTM we should include the new AWS-LC variant in each of these three places as well. Being included in
allowed-failures
andallowed-skips
it can't block a merge, but it will ensure that there's a result before a PR is merged.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. For
allowed-failures
andallowed-skips
, I agree. But what aboutall-required-green
? It looks like that job is "required" to pass, and will fail if any of its members do not pass.So, it looks like including
-awslc
there would effectively make it a "required" check. I'm open to this, but @gpshead expressed a preference for making this test non-required at first, which seems reasonable.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see!
needs
here does not mean "needs to have succeeded", just "needs to have completed". From GitHub's docs:Your proposal makes sense. I'll add
-awslc
in those 3 places.