Skip to content

Conversation

bwesterb
Copy link
Member

@bwesterb bwesterb commented Oct 2, 2025

The big diff is misleading. Applying each patch to the base 478b28ab12f and comparing them, we see:

git range-diff 478b28ab12f2001a03261624261fd041f5439706..adcd4022f75953605a9bf9f6a4a45c0b4fd8ed94 478b28ab12f2001a03261624261fd041f5439706..6f1b1e1f451e61cd2bda0922eecaa8387397ac5a
1:  adcd4022f ! 1:  6f1b1e1f4 Add additional post-quantum key agreements
    @@ Commit message
     
         This patch adds:
     
    -    1. Support for MLKEM768X25519 under the codepoint 0x11ec. The version
    -       of BoringSSL we patch against did not support it yet.
    +    1. Support for X25519MLKEM768 under the codepoint 0x11ec. The version
    +       of BoringSSL we patch against did not support it yet. Like recent
    +       upstream, enable by default.
     
         2. Supports for P256Kyber768Draft00 under 0xfe32, which we temporarily
            need for compliance reasons.  (Note that this is not the codepoint
    @@ ssl/extensions.cc: static bool tls1_check_duplicate_extensions(const CBS *cbs) {
            return true;
          default:
            return false;
    +@@ ssl/extensions.cc: bool ssl_client_hello_get_extension(const SSL_CLIENT_HELLO *client_hello,
    + }
    + 
    + static const uint16_t kDefaultGroups[] = {
    ++    SSL_GROUP_X25519_MLKEM768,
    +     SSL_GROUP_X25519,
    +     SSL_GROUP_SECP256R1,
    +     SSL_GROUP_SECP384R1,
     
      ## ssl/ssl_key_share.cc ##
     @@

The big diff is misleading. Applying each patch to the base 478b28ab12f
and comparing them, we see:

git range-diff 478b28ab12f2001a03261624261fd041f5439706..adcd4022f75953605a9bf9f6a4a45c0b4fd8ed94 478b28ab12f2001a03261624261fd041f5439706..6f1b1e1f451e61cd2bda0922eecaa8387397ac5a
1:  adcd4022f ! 1:  6f1b1e1f4 Add additional post-quantum key agreements
    @@ Commit message

         This patch adds:

    -    1. Support for MLKEM768X25519 under the codepoint 0x11ec. The version
    -       of BoringSSL we patch against did not support it yet.
    +    1. Support for X25519MLKEM768 under the codepoint 0x11ec. The version
    +       of BoringSSL we patch against did not support it yet. Like recent
    +       upstream, enable by default.

         2. Supports for P256Kyber768Draft00 under 0xfe32, which we temporarily
            need for compliance reasons.  (Note that this is not the codepoint
    @@ ssl/extensions.cc: static bool tls1_check_duplicate_extensions(const CBS *cbs) {
            return true;
          default:
            return false;
    +@@ ssl/extensions.cc: bool ssl_client_hello_get_extension(const SSL_CLIENT_HELLO *client_hello,
    + }
    +
    + static const uint16_t kDefaultGroups[] = {
    ++    SSL_GROUP_X25519_MLKEM768,
    +     SSL_GROUP_X25519,
    +     SSL_GROUP_SECP256R1,
    +     SSL_GROUP_SECP384R1,

      ## ssl/ssl_key_share.cc ##
     @@
@bwesterb
Copy link
Member Author

bwesterb commented Oct 2, 2025

Side-note: given we enable PQ by default after #397, it might be more convenient to move the submodule to a fork with the patch already applied.

@cjpatton cjpatton added the v5 label Oct 2, 2025
@cjpatton
Copy link
Collaborator

cjpatton commented Oct 2, 2025

The big diff is misleading.

I confirm that the diff is indeed small. In fact, all it does is add X25519MLKEM768 as the most preferred group in the defaults.

Queestion: Should we also add P256Kyber768Draft00 to the default groups, with low preference? FIPS users will override the defaults anyway, but if a FIPS client wants to talk to a non-FIPS server, we can save them an HRR by supporting P256Kyber on the server side.

@cjpatton
Copy link
Collaborator

cjpatton commented Oct 2, 2025

ote: given we enable PQ by default after #397, it might be more convenient to move the submodule to a fork with the patch already applied.

I think it's useful to keep as a patch. Once we're ready to upgrade to a newer version of boring, it'll be more clear what work needs to be done to maintain the features implemented by the patch.

@bwesterb
Copy link
Member Author

bwesterb commented Oct 3, 2025

Queestion: Should we also add P256Kyber768Draft00 to the default groups, with low preference?

Done that.

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.

2 participants