chore: POC for PQC in gax-grpc#13335
Conversation
TAG=agy CONV=0ade5891-3c8d-4e27-a240-b1a8cd6a0b0c
…ranch chore/pqc-poc-2 TAG=agy CONV=0ade5891-3c8d-4e27-a240-b1a8cd6a0b0c
TAG=agy CONV=0ade5891-3c8d-4e27-a240-b1a8cd6a0b0c
This commit addresses the review feedback by implementing: 1. Graceful fallback to default NetHttpTransport with WARNING logging in OAuth2Utils static initializer. 2. warning-level exception logging in InstantiatingGrpcChannelProvider. 3. Caching of shaded/unshaded gRPC Netty OpenSSL reflection lookups inside thread-safe static OpenSslReflectionHolder to remove runtime overhead. 4. Reverting createHttpTransport in InstantiatingHttpJsonChannelProvider to return null when mTLS is not active, as default transport is already PQC-hardened. 5. Clean unregistration of BC and BCJSSE security providers in integration test server/client teardown. TAG=agy
…e-cloud-java into chore/pqc-poc-2
…t delegating wrapper
…uncy Castle JSSE in PQC zero-config connectivity integration tests and add extensive documentation comments - Configures Bouncy Castle server-scoped system properties fallback to enforce ML-KEM-768 on Java 17. - Keeps compile-safe Java 20 reflection for JRE 20+ runtimes. - Adds extremely detailed comments describing provider, keystore, managers, server configurators, and netty gRPC secure socket pipelines. TAG=agy CONV=5d96c302-27fd-438a-ad0e-ffd6d64e61cb
This reverts commit f0e9e46.
…ate in pqc-tests workflow
| @@ -70,7 +70,6 @@ | |||
| <dependency> | |||
| <groupId>io.grpc</groupId> | |||
| <artifactId>grpc-netty-shaded</artifactId> | |||
| <scope>runtime</scope> | |||
There was a problem hiding this comment.
this must be evaluated
There was a problem hiding this comment.
Code Review
This pull request introduces a new pqc-test module to validate Post-Quantum Cryptography (PQC) transport enforcement in the Google Cloud Java SDK, including a test server (PqcTestServer) and connectivity tests (PqcConnectivityTest). It also updates InstantiatingGrpcChannelProvider to configure PQC on gRPC channels. The review feedback highlights two resource leak opportunities: first, a potential leak of the background server process in PqcConnectivityTest if initialization fails before @AfterAll can run; second, a potential leak of partially started servers in PqcTestServer.main if startup fails, along with a recommendation to avoid swallowing InterruptedException.
| java.io.BufferedReader reader = | ||
| new java.io.BufferedReader( | ||
| new java.io.InputStreamReader( | ||
| serverProcess.getInputStream(), java.nio.charset.StandardCharsets.UTF_8)); | ||
|
|
||
| String line; | ||
| boolean grpcPqcFound = false; | ||
| boolean grpcClassicalFound = false; | ||
|
|
||
| // Wait for the server process to output its HTTP and gRPC ports | ||
| long startTime = System.currentTimeMillis(); | ||
| while ((line = reader.readLine()) != null) { | ||
| System.out.println("[SERVER-OUT] " + line); | ||
| if (line.startsWith("GRPC_PQC_PORT: ")) { | ||
| grpcPqcPort = Integer.parseInt(line.substring(15).trim()); | ||
| grpcPqcFound = true; | ||
| } else if (line.startsWith("GRPC_CLASSICAL_PORT: ")) { | ||
| grpcClassicalPort = Integer.parseInt(line.substring(21).trim()); | ||
| grpcClassicalFound = true; | ||
| } | ||
|
|
||
| if (grpcPqcFound && grpcClassicalFound) { | ||
| break; | ||
| } | ||
|
|
||
| // Ephemeral port detection timeout (10 seconds) to fail-fast on server startup | ||
| // errors | ||
| if (System.currentTimeMillis() - startTime > 10000) { | ||
| throw new RuntimeException( | ||
| "Timeout waiting for PqcTestServer ephemeral ports to be printed!"); | ||
| } | ||
| } | ||
|
|
||
| if (!grpcPqcFound || !grpcClassicalFound) { | ||
| throw new RuntimeException("PqcTestServer failed to initialize ephemeral ports!"); | ||
| } | ||
|
|
||
| // Start a background thread to continuously drain the server's stdout | ||
| Thread drainThread = | ||
| new Thread( | ||
| () -> { | ||
| try { | ||
| String l; | ||
| while ((l = reader.readLine()) != null) { | ||
| System.out.println("[SERVER-OUT] " + l); | ||
| } | ||
| } catch (java.io.IOException e) { | ||
| // Ignore stream closed | ||
| } | ||
| }); | ||
| drainThread.setDaemon(true); | ||
| drainThread.start(); |
There was a problem hiding this comment.
If an exception or timeout occurs during the initialization phase in @BeforeAll (for example, if the server fails to print the expected ports within 10 seconds), the background serverProcess will be left running. Since @AfterAll is not executed when @BeforeAll fails in JUnit 5, this results in a leaked background process. Wrap the initialization logic in a try-catch block and forcibly destroy the process if setup fails.
try {
// Read server's stdout to dynamically capture the allocated ephemeral ports
java.io.BufferedReader reader =
new java.io.BufferedReader(
new java.io.InputStreamReader(
serverProcess.getInputStream(), java.nio.charset.StandardCharsets.UTF_8));
String line;
boolean grpcPqcFound = false;
boolean grpcClassicalFound = false;
// Wait for the server process to output its HTTP and gRPC ports
long startTime = System.currentTimeMillis();
while ((line = reader.readLine()) != null) {
System.out.println("[SERVER-OUT] " + line);
if (line.startsWith("GRPC_PQC_PORT: ")) {
grpcPqcPort = Integer.parseInt(line.substring(15).trim());
grpcPqcFound = true;
} else if (line.startsWith("GRPC_CLASSICAL_PORT: ")) {
grpcClassicalPort = Integer.parseInt(line.substring(21).trim());
grpcClassicalFound = true;
}
if (grpcPqcFound && grpcClassicalFound) {
break;
}
// Ephemeral port detection timeout (10 seconds) to fail-fast on server startup
// errors
if (System.currentTimeMillis() - startTime > 10000) {
throw new RuntimeException(
"Timeout waiting for PqcTestServer ephemeral ports to be printed!");
}
}
if (!grpcPqcFound || !grpcClassicalFound) {
throw new RuntimeException("PqcTestServer failed to initialize ephemeral ports!");
}
// Start a background thread to continuously drain the server's stdout
Thread drainThread =
new Thread(
() -> {
try {
String l;
while ((l = reader.readLine()) != null) {
System.out.println("[SERVER-OUT] " + l);
}
} catch (java.io.IOException e) {
// Ignore stream closed
}
});
drainThread.setDaemon(true);
drainThread.start();
} catch (Throwable t) {
serverProcess.destroyForcibly();
throw t;
}| public static void main(String[] args) throws Exception { | ||
| PqcTestServer server = new PqcTestServer(); | ||
| server.start(); | ||
|
|
||
| System.out.println("HTTP_PQC_PORT: " + server.getHttpPqcPort()); | ||
| System.out.println("HTTP_CLASSICAL_PORT: " + server.getHttpClassicalPort()); | ||
| System.out.println("GRPC_PQC_PORT: " + server.getGrpcPqcPort()); | ||
| System.out.println("GRPC_CLASSICAL_PORT: " + server.getGrpcClassicalPort()); | ||
| System.out.flush(); | ||
|
|
||
| try { | ||
| while (System.in.read() != -1) { | ||
| Thread.sleep(1000); | ||
| } | ||
| } catch (Exception e) { | ||
| } finally { | ||
| server.stop(); | ||
| } | ||
| } |
There was a problem hiding this comment.
If server.start() throws an exception during startup (e.g., due to a port binding failure or keystore issue), the exception is thrown before entering the try-finally block. As a result, server.stop() is never called, leaving any partially started HTTP or gRPC servers running. Wrap the entire startup and execution in a single try-finally block to ensure server.stop() is always executed. Additionally, make sure that InterruptedException is not swallowed when catching exceptions; restore the thread's interrupted status appropriately.
public static void main(String[] args) throws Exception {
PqcTestServer server = new PqcTestServer();
try {
server.start();
System.out.println("HTTP_PQC_PORT: " + server.getHttpPqcPort());
System.out.println("HTTP_CLASSICAL_PORT: " + server.getHttpClassicalPort());
System.out.println("GRPC_PQC_PORT: " + server.getGrpcPqcPort());
System.out.println("GRPC_CLASSICAL_PORT: " + server.getGrpcClassicalPort());
System.out.flush();
while (System.in.read() != -1) {
Thread.sleep(1000);
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
System.err.println("Server interrupted: " + e.getMessage());
} catch (Exception e) {
e.printStackTrace();
} finally {
server.stop();
}
}References
- In Java, do not swallow InterruptedException. When catching it, restore the thread's interrupted status by calling Thread.currentThread().interrupt() and handle the interruption appropriately, such as by throwing a relevant exception (e.g., SpannerException) to signal that the operation cannot proceed.
No description provided.