Skip to content

ROX-33133: Remove hardcoded key-exchange in gRPC #2991

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.

Sign up for GitHub

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

Draft
vladbologa wants to merge 1 commit into
base: master
Choose a base branch
from
+21 −0
Draft

ROX-33133: Remove hardcoded key-exchange in gRPC #2991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions builder/install/40-grpc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ export CXXFLAGS="-Wno-error=class-memaccess -Wno-ignored-qualifiers -Wno-stringo
cd third_party/grpc

cp NOTICE.txt "${LICENSE_DIR}/grpc-${GRPC_REVISION}"

# ROX-33133: Remove hardcoded P-256 curve from gRPC (OpenSSL 3.x only) to allow
# OpenSSL to use system crypto-policies defaults, enabling post-quantum key
# exchange (ML-KEM). See: https://github.com/grpc/grpc/issues/23083
patch -p1 < ../../builder/install/grpc-pq-curves.patch

mkdir -p cmake/build
cd cmake/build
cmake \
Expand Down
15 changes: 15 additions & 0 deletions builder/install/grpc-pq-curves.patch
View file
Open in desktop
Copy link
Collaborator

@Molter73 Molter73 Feb 27, 2026

Choose a reason for hiding this comment

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

I'm a bit concerned about carrying a patch for this ourselves. I can already tell the line has moved on the master branch of grpc, so when we decide to update the dependency, this patch will need to be regenerated.

Is there any way we could directly upstream something instead of working around it ourselves?

Copy link
Contributor Author

@vladbologa vladbologa Feb 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'm just trying to get ML-KEM to work for now.

As for the upstreaming, I don't know. They had a ticket and they didn't fix it. It would be best to fix it upstream, but I'm not sure that can be done in time for 4.11.

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
--- a/src/core/tsi/ssl_transport_security.cc
+++ b/src/core/tsi/ssl_transport_security.cc
@@ -819,12 +819,6 @@ static tsi_result populate_ssl_context(
}
SSL_CTX_set_options(context, SSL_OP_SINGLE_ECDH_USE);
EC_KEY_free(ecdh);
-#else
- if (!SSL_CTX_set1_groups(context, kSslEcCurveNames, 1)) {
- LOG(ERROR) << "Could not set ephemeral ECDH key.";
- return TSI_INTERNAL_ERROR;
- }
- SSL_CTX_set_options(context, SSL_OP_SINGLE_ECDH_USE);
#endif
}
return TSI_OK;
Loading