feat(rest): add PQC curves to REST client#16233
Conversation
There was a problem hiding this comment.
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.
| 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; | ||
| } |
There was a problem hiding this comment.
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
- 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 Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
Optionally enable PQC curves for REST client
Explicitly configure
CURLOPT_SSL_EC_CURVESin the libcurl backend toprefer
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_CURVESfor compatibility with older libcurlheaders. 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