Skip to content

feat(rest): add PQC curves to REST client#16233

Draft
hongalex wants to merge 1 commit into
googleapis:mainfrom
hongalex:feat-support-pqc
Draft

feat(rest): add PQC curves to REST client#16233
hongalex wants to merge 1 commit into
googleapis:mainfrom
hongalex:feat-support-pqc

Conversation

@hongalex

@hongalex hongalex commented Jul 2, 2026

Copy link
Copy Markdown
Member

Optionally enable PQC curves for REST client

Explicitly configure CURLOPT_SSL_EC_CURVES in the libcurl backend to
prefer X25519MLKEM768 (the hybrid post-quantum key exchange group),
followed by classical curves (X25519, P-256, P-384).

This enables Post-Quantum Cryptography (PQC) compliance for C++ REST
clients when connecting to servers that support it (such as Showcase
running on Go 1.24+ or Google Cloud services supporting PQC).

We use CURLOPT_SSL_EC_CURVES for compatibility with older libcurl
headers. If setting the option fails, we log an INFO message and
continue with libcurl/SSL library defaults, ensuring no regression
on older platforms.

TAG=agy
CONV=a6dbbcf8-b85b-4925-b9cf-7e1227fee2c0

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds a call to configure SSL elliptic curves using CURLOPT_SSL_EC_CURVES in CurlImpl::MakeRequestImpl. The review feedback correctly points out that reusing the status variable for an option whose failure is ignored is risky, as it leaves status in a non-OK state temporarily. It is recommended to use a separate local variable for this check to prevent accidental propagation of the error state.

Comment on lines +604 to +607
status = handle_.SetOption(CURLOPT_SSL_EC_CURVES, "X25519MLKEM768:X25519:P-256:P-384");
if (!status.ok()) {
GCP_LOG(INFO) << "CURLOPT_SSL_EC_CURVES failed: " << status;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Reusing the status variable for an option whose failure is explicitly ignored is risky. If CURLOPT_SSL_EC_CURVES fails, status is left in a non-OK state until it is overwritten by the next SetOption call. If the code is refactored, reordered, or if another check is inserted in between, this temporary error state could be accidentally propagated or returned.

Using a separate local variable (e.g., ec_status) avoids polluting the main status variable and makes the intent to ignore the failure much clearer and safer.

  auto ec_status = handle_.SetOption(CURLOPT_SSL_EC_CURVES, "X25519MLKEM768:X25519:P-256:P-384");
  if (!ec_status.ok()) {
    GCP_LOG(INFO) << "CURLOPT_SSL_EC_CURVES failed: " << ec_status;
  }
References
  1. Prefer defensive code, such as explicit ok() checks, even if they seem redundant based on the current implementation of a framework, as the framework's contract may change in the future.

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.70%. Comparing base (a0ad519) to head (02fc095).
⚠️ Report is 59 commits behind head on main.

Files with missing lines Patch % Lines
google/cloud/internal/curl_impl.cc 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #16233   +/-   ##
=======================================
  Coverage   92.70%   92.70%           
=======================================
  Files        2356     2356           
  Lines      220047   220050    +3     
=======================================
+ Hits       203998   204008   +10     
+ Misses      16049    16042    -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant