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

Support draft-kwiatkowski-tls-ecdhe-mlkem #2622

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

dcooper16
Copy link
Contributor

Describe your changes

This commit adds support for the three code points in draft-kwiatkowski-tls-ecdhe-mlkem.

What is your pull request about?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Typo fix
  • Documentation update
  • Update of other files

If it's a code change please check the boxes which are applicable

  • For the main program: My edits contain no tabs and the indentation is five spaces
  • I've read CONTRIBUTING.md and Coding_Convention.md
  • I have tested this fix against >=2 hosts and I couldn't spot a problem
  • I have tested this new feature against >=2 hosts which show this feature and >=2 host which does not (in order to avoid side effects) . I couldn't spot a problem
  • For the new feature I have made corresponding changes to the documentation and / or to help()
  • If it's a bigger change: I added myself to CREDITS.md (alphabetical order) and the change to CHANGELOG.md

@drwetter
Copy link
Collaborator

FYI: I reran the Ci jobs after stopping the previous one which ran after 4 hours

testssl.sh Show resolved Hide resolved
@drwetter
Copy link
Collaborator

whohoo ! 👍

cloudflare.com and google.com miss to show any TLS 1.3 sig algorithms with this patch . For Geert's site it's fine.

image

@drwetter
Copy link
Collaborator

drwetter commented Jan 18, 2025

Correction: that seems to be a MacOS thing. On Debian it works like expected. 🧐 .Not MacOS, for some reason using the supplied openssl is fine . So ./testssl.sh --openssl=/usr/bin/openssl -f google.com misses the TLS 1.3 sig algs too on Debian (12) with this patch.

…ber768d00

This commit adds support for the three code points in draft-kwiatkowski-tls-ecdhe-mlkem and the code point 0x6399 from draft-tls-westerbaan-xyber768d00. The group 0x6399 uses a pre-standard version of Kyber and is considered obsolete.
@dcooper16 dcooper16 force-pushed the draft-kwiatkowski-tls-ecdhe-mlkem branch from e548760 to 11d7979 Compare January 21, 2025 17:02
@dcooper16
Copy link
Contributor Author

Thanks for catching the mistake. I was adding the new PQC groups to the ClientHello whenever OpenSSL supported X25519 and X448, which was a mistake. The result was that when searching for supported signature algorithms, a PQC group was being selected, the response could not be decrypted, and the signature algorithm could not be determined.

I may not have caught this because I was getting the same results when using --openssl=/usr/bin/openssl with and without this change. The reason was that the current testssl.sh was not finding any TLS 1.3 signature algorithms when using --openssl=/usr/bin/openssl. I found the reason for that and submitted #2625 to fix that.

@drwetter drwetter merged commit 17f2a5d into testssl:3.2 Jan 22, 2025
2 checks passed
@drwetter
Copy link
Collaborator

Thanks a lot, David! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants