Skip to content

chore: [wip] PQC POC 2#13203

Draft
diegomarquezp wants to merge 10 commits into
mainfrom
chore/pqc-poc-2
Draft

chore: [wip] PQC POC 2#13203
diegomarquezp wants to merge 10 commits into
mainfrom
chore/pqc-poc-2

Conversation

@diegomarquezp
Copy link
Copy Markdown
Contributor

No description provided.

TAG=agy
CONV=0ade5891-3c8d-4e27-a240-b1a8cd6a0b0c
Copy link
Copy Markdown
Contributor

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

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 introduces Post-Quantum Cryptography (PQC) support across the Java SDK by integrating Bouncy Castle and configuring PQC-hardened SSL contexts for gRPC and HTTP/JSON transports. Key changes include the addition of a pqc-test module, reflection-based configuration for Netty channel builders, and updates to OAuth2Utils and InstantiatingHttpJsonChannelProvider. Feedback identifies issues with potential initialization errors in OAuth2Utils, the use of a SNAPSHOT dependency version, and performance overhead from repeated reflection lookups. Suggestions were also provided for improving exception handling, ensuring logic consistency in transport creation, and cleaning up test provider registrations.

Comment thread google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/OAuth2Utils.java Outdated
Comment thread sdk-platform-java/gapic-generator-java-pom-parent/pom.xml
…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
@diegomarquezp
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

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

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 implements Post-Quantum Cryptography (PQC) support by hardening HTTP and gRPC transports with Bouncy Castle JSSE. Key changes include updating OAuth2Utils to use a PQC-hardened transport, adding Bouncy Castle dependencies to GAX modules, and introducing reflection-based configuration for gRPC channels to support hybrid PQC key exchange. A new integration test module, pqc-test, is also introduced. Feedback highlights the use of a -SNAPSHOT dependency version in the parent POM, code duplication and brittle string-based class checks in the gRPC reflection logic, and unused imports.

<javax.annotation-api.version>1.3.2</javax.annotation-api.version>
<grpc.version>1.81.0</grpc.version>
<google.http-client.version>2.1.0</google.http-client.version>
<google.http-client.version>2.1.1-SNAPSHOT</google.http-client.version>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Updating google.http-client.version to a -SNAPSHOT version in a parent POM is generally discouraged for release-track projects. This can lead to unstable builds and dependency resolution issues in environments without access to the specific snapshot repository. If this is necessary for the POC, please ensure it is reverted or replaced with a stable version before merging.

Comment on lines +930 to +965
private ManagedChannelBuilder<?> applyPqcConfiguration(ManagedChannelBuilder<?> builder) {
// Configure the PQ and classical hybrid named groups:
// 1. X25519MLKEM768 (codepoint 4588): Hybrid classical (X25519) + post-quantum (ML-KEM-768) key exchange.
// Provides defense-in-depth: if ML-KEM is compromised, security reverts to classical strength of X25519.
// 2. MLKEM768 (codepoint 1896): Pure post-quantum key exchange using ML-KEM-768.
// 3. X25519 (codepoint 29): Classical elliptic curve Diffie-Hellman key exchange, used as a fallback.
String[] hybridGroups = new String[] {"X25519MLKEM768", "MLKEM768", "X25519"};
String builderClassName = builder.getClass().getName();
boolean isShaded = "io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder".equals(builderClassName);
boolean isUnshaded = "io.grpc.netty.NettyChannelBuilder".equals(builderClassName);

if (isShaded && OpenSslReflectionHolder.SHADED_AVAILABLE) {
try {
Object sslContextBuilder = OpenSslReflectionHolder.SHADED_FOR_CLIENT.invoke(null);
OpenSslReflectionHolder.SHADED_OPTION_METHOD.invoke(
sslContextBuilder, OpenSslReflectionHolder.SHADED_GROUPS_OPTION, (Object) hybridGroups);
Object sslContext = OpenSslReflectionHolder.SHADED_BUILD_METHOD.invoke(sslContextBuilder);
OpenSslReflectionHolder.SHADED_SSL_CONTEXT_METHOD.invoke(builder, sslContext);
return builder;
} catch (java.lang.reflect.InvocationTargetException | IllegalAccessException | RuntimeException e) {
LOG.log(Level.WARNING, "Failed to configure shaded PQC transport fallback", e);
}
} else if (isUnshaded && OpenSslReflectionHolder.UNSHADED_AVAILABLE) {
try {
Object sslContextBuilder = OpenSslReflectionHolder.UNSHADED_FOR_CLIENT.invoke(null);
OpenSslReflectionHolder.UNSHADED_OPTION_METHOD.invoke(
sslContextBuilder, OpenSslReflectionHolder.UNSHADED_GROUPS_OPTION, (Object) hybridGroups);
Object sslContext = OpenSslReflectionHolder.UNSHADED_BUILD_METHOD.invoke(sslContextBuilder);
OpenSslReflectionHolder.UNSHADED_SSL_CONTEXT_METHOD.invoke(builder, sslContext);
return builder;
} catch (java.lang.reflect.InvocationTargetException | IllegalAccessException | RuntimeException e) {
LOG.log(Level.WARNING, "Failed to configure unshaded PQC transport fallback", e);
}
}
return builder;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The applyPqcConfiguration method contains significant code duplication between the shaded and unshaded Netty configuration paths. Additionally, checking the builder class name via string comparison (lines 938-939) is brittle and may fail if the class is renamed or wrapped.

Consider refactoring OpenSslReflectionHolder to store two instances of a helper class (e.g., NettyReflectionMetadata) that encapsulates the reflected methods and classes for each variant. You can then use Class.isInstance() for a more robust type check and a single helper method to apply the configuration.

References
  1. If code is duplicated and needs to be shared, move it to a separate helper/utility class.

Comment on lines +45 to +46
import javax.net.ssl.SSLContext;
import java.security.NoSuchAlgorithmException;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The imports for javax.net.ssl.SSLContext and java.security.NoSuchAlgorithmException are unused in this file and should be removed.

ks.load(PqcTestServer.class.getResourceAsStream("/pqctest.p12"), "password".toCharArray());

// Build a custom HttpTransport explicitly trusting the self-signed certificate keystore.
final com.google.api.client.http.HttpTransport httpTransport = new com.google.api.client.http.javanet.NetHttpTransport.Builder()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this should be enabled by default via pom.xml in this test

…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
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