-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
Changes from 14 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
4d9147c
7311e42
13dfd95
22470c1
e24bde5
69f69e6
a0d7b5f
a2efcda
38afe99
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,83 @@ 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.52.1] | ||
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 | ||
# TODO [childw] do we need to re-specify the env vars here after L330's configuration? | ||
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 }} | ||
# TODO [childw] can we use env.* instead of env vars here? | ||
- 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 | ||
|
@@ -620,7 +697,8 @@ jobs: | |
- build-windows-msi | ||
- build-macos | ||
- build-ubuntu | ||
- build-ubuntu-ssltests | ||
- build-ubuntu-ssltests-awslc | ||
- build-ubuntu-ssltests-openssl | ||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- build-wasi | ||
- test-hypothesis | ||
- build-asan | ||
|
@@ -635,7 +713,8 @@ jobs: | |
with: | ||
allowed-failures: >- | ||
build-windows-msi, | ||
build-ubuntu-ssltests, | ||
build-ubuntu-ssltests-awslc | ||
build-ubuntu-ssltests-openssl | ||
test-hypothesis, | ||
cifuzz, | ||
allowed-skips: >- | ||
|
@@ -653,7 +732,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 @@ | ||
Add a new GitHub CI job to test python's ssl module with AWS-LC as the backing cryptography and TLS library. | ||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
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 crypto libraries | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
The script | ||||||
|
||||||
(1) downloads OpenSSL / LibreSSL tar bundle | ||||||
(1) downloads OpenSSL / LibreSSL / AWS-LC tar bundle | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
(2) extracts it to ./src | ||||||
(3) compiles OpenSSL / LibreSSL | ||||||
(4) installs OpenSSL / LibreSSL into ../multissl/$LIB/$VERSION/ | ||||||
(3) compiles OpenSSL / LibreSSL / AWS-LC | ||||||
(4) installs the crypto library into ../multissl/$LIB/$VERSION/ | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
(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.52.1", | ||||||
] | ||||||
|
||||||
# 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 crypto libraries" | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
"versions." | ||||||
) | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
|
@@ -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) | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
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" | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
"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) | ||||||
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.
Suggested change
Alternatively, it's probably time that we could remove 2.7 from the list of versions we should stay compatible with. 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.
That's certainly a call for the maintainers, but I'm happy to help implement your decision. No concerns from AWS-LC's perspective as the earliest version we test against is 3.9. 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. @zware -- the 2.7 compat issues didn't show up in this PR's CI. is there some other slack/zulip/etc. channel where i can monitor secondary CI builds to get ahead of these issues during review? 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 personally have been ready to drop 2.7 compatibility everywhere for over a decade now :). I have no evidence that anyone is still actually using 2.7 with this script; that note is 8 years old, from back when we were still maintaining each of the mentioned versions. |
||||||
|
||||||
|
||||||
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) | ||||||
|
@@ -513,6 +554,14 @@ def main(): | |||||
build.install() | ||||||
builds.append(build) | ||||||
|
||||||
for version in args.awslc: | ||||||
WillChilds-Klein marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
build = BuildAWSLC( | ||||||
version, | ||||||
args | ||||||
) | ||||||
build.install() | ||||||
builds.append(build) | ||||||
|
||||||
if args.steps in {'modules', 'tests'}: | ||||||
for build in builds: | ||||||
try: | ||||||
|
@@ -539,7 +588,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.
I'd suggest for the initial version, keep as similar to the OpenSSL job/workflow, and then perhaps update both at once afterwards?
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.
Fair enough. I'll remove the TODOs. Perhaps we can leave this comment unresolved as a reminder for me to clean up both (if tenable) if/after this PR has been merged.