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

tests: allow TLS1.2 with RSA-PSS certs in integ tests #4949

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Dec 4, 2024

Release Summary:

Resolved issues:

tests #4927

Description of changes:

Our integration tests currently skip any combination of parameters that would negotiate RSA-PSS-PSS with TLS1.2. Now that we support RSA-PSS-PSS with TLS1.2, this change removes those restrictions.

Callouts

I'm sorry for the hacky string comparisons, but that's unfortunately how the integ tests currently work ;_;

Testing:

It's hard to confirm what tests we run, so I dumped the output of a successful test_s2n_client_signature_algorithms run into a file and searched it. It looks like:

...
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-ECDHE-RSA-AES128-GCM-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-ECDHE-RSA-AES256-GCM-SHA384]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-ECDHE-RSA-CHACHA20-POLY1305]
...

The commands I ran, and the results:

$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | wc -l
68

$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | grep TLS1.2 | wc -l
56

$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | grep TLS1.2 | grep s2n_client | wc -l
28

$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | grep TLS1.2 | grep s2n_server | wc -l
28

$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | sed 's/^.*-OpenSSL-\(.*\)\]/\1/' | sort -u
DHE-RSA-AES128-GCM-SHA256
DHE-RSA-AES128-SHA
DHE-RSA-AES128-SHA256
DHE-RSA-AES256-GCM-SHA384
DHE-RSA-AES256-SHA
DHE-RSA-AES256-SHA256
DHE-RSA-CHACHA20-POLY1305
ECDHE-RSA-AES128-GCM-SHA256
ECDHE-RSA-AES128-SHA
ECDHE-RSA-AES128-SHA256
ECDHE-RSA-AES256-GCM-SHA384
ECDHE-RSA-AES256-SHA
ECDHE-RSA-AES256-SHA384
ECDHE-RSA-CHACHA20-POLY1305
TLS_AES_128_GCM_SHA256
TLS_AES_256_GCM_SHA384
TLS_CHACHA20_POLY1305_SHA256

That looks correct to me, given the full set of ciphers:

$ cat signature_algorithms.out | grep ": PASSED " | grep "OpenSSL" | sed 's/^.*-OpenSSL-\(.*\)\]/\1/' | sort -u
AES128-SHA
AES128-SHA256
AES256-SHA
AES256-SHA256
DHE-RSA-AES128-GCM-SHA256
DHE-RSA-AES128-SHA
DHE-RSA-AES128-SHA256
DHE-RSA-AES256-GCM-SHA384
DHE-RSA-AES256-SHA
DHE-RSA-AES256-SHA256
DHE-RSA-CHACHA20-POLY1305
ECDHE-ECDSA-AES128-GCM-SHA256
ECDHE-ECDSA-AES128-SHA
ECDHE-ECDSA-AES128-SHA256
ECDHE-ECDSA-AES256-GCM-SHA384
ECDHE-ECDSA-AES256-SHA
ECDHE-ECDSA-AES256-SHA384
ECDHE-ECDSA-CHACHA20-POLY1305
ECDHE-RSA-AES128-GCM-SHA256
ECDHE-RSA-AES128-SHA
ECDHE-RSA-AES128-SHA256
ECDHE-RSA-AES256-GCM-SHA384
ECDHE-RSA-AES256-SHA
ECDHE-RSA-AES256-SHA384
ECDHE-RSA-CHACHA20-POLY1305
TLS_AES_128_GCM_SHA256
TLS_AES_256_GCM_SHA384
TLS_CHACHA20_POLY1305_SHA256

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@lrstewart lrstewart marked this pull request as ready for review December 6, 2024 01:52
@lrstewart lrstewart requested review from jmayclin and jouho December 6, 2024 01:52
Comment on lines 118 to 122
if self.algorithm == 'RSAPSS':
if cipher.algorithm != 'RSA':
return False
if 'ECDHE' in cipher.name:
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is the "hacky string comparisons", but I'm trying to understand the context here. Is RSA-PSS only compatible with ECDHE key exchange?

Copy link
Contributor Author

@lrstewart lrstewart Dec 9, 2024

Choose a reason for hiding this comment

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

RSA-PSS certs can't be used with RSA key exchange (that's what Sam was asking about here). That's because RSA-PSS certs can only be used for signing, not encryption, and RSA key exchange involves encryption. RSA-PSS certs are basically just RSA certs with safer usage rules.

But as I look at this again, checking for "ECDHE" is probably too restrictive. I'll relax it to "DHE". I can't go all the way to just checking for no "RSA", because 1) legacy cipher suites assume RSA without putting it in their names 2) "RSA" could also refer to the auth method instead of the kex method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, and I updated the testing results in the description. And here's the result of only non-ECDHE:

$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | grep TLS1.2 | grep -v ECDHE | wc -l
28
$ cat signature_algorithms.out | grep ": PASSED " | grep pss_pss | grep TLS1.2 | grep -v ECDHE
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-GCM-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-GCM-SHA384]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-CHACHA20-POLY1305]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-GCM-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-GCM-SHA384]
293: PASSED test_signature_algorithms.py::test_s2n_server_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-CHACHA20-POLY1305]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-GCM-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-GCM-SHA384]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-CHACHA20-POLY1305]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES128-GCM-SHA256]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-AES256-GCM-SHA384]
293: PASSED test_signature_algorithms.py::test_s2n_client_signature_algorithms[no-client-auth-rsa_pss_pss_sha256-RSA_PSS_2048_SHA256-TLS1.2-S2N-OpenSSL-DHE-RSA-CHACHA20-POLY1305]

Copy link
Contributor

Choose a reason for hiding this comment

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

Very interesting. Thank you for explaining!

@lrstewart lrstewart requested a review from jouho December 9, 2024 22:56
tests/integrationv2/common.py Show resolved Hide resolved
@lrstewart lrstewart enabled auto-merge (squash) December 10, 2024 00:56
@lrstewart lrstewart merged commit 3c4ea72 into aws:main Dec 10, 2024
39 checks passed
@lrstewart lrstewart deleted the rsapss_integ branch December 10, 2024 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants