Upgrade Netty to 4.1.135

S2ATrustManager now needs to implement X509ExtendedTrustManager because
Netty is now doing hostname verification itself if the trust manager is
not extended (netty/netty@09e72c4fd8). S2A
didn't need to be extended because it received the desired hostname to
verify from its constructor, so doesn't need to look it up in
SSLSocket/SSLEngine.

NettyClientTransportTest is seeing a different failure now because the
server is no longer failing the RPC before sending the response headers
(netty/netty@b831454889).

Author: ejona86

MODULE.bazel

@@ -25,20 +25,20 @@ IO_GRPC_GRPC_JAVA_ARTIFACTS = [
     "dev.cel:common:0.12.0",
     "com.squareup.okhttp:okhttp:2.7.5",
     "com.squareup.okio:okio:2.10.0",  # 3.0+ needs swapping to -jvm; need work to avoid flag-day
-    "io.netty:netty-buffer:4.1.133.Final",
-    "io.netty:netty-codec-http2:4.1.133.Final",
-    "io.netty:netty-codec-http:4.1.133.Final",
-    "io.netty:netty-codec-socks:4.1.133.Final",
-    "io.netty:netty-codec:4.1.133.Final",
-    "io.netty:netty-common:4.1.133.Final",
-    "io.netty:netty-handler-proxy:4.1.133.Final",
-    "io.netty:netty-handler:4.1.133.Final",
-    "io.netty:netty-resolver:4.1.133.Final",
+    "io.netty:netty-buffer:4.1.135.Final",
+    "io.netty:netty-codec-http2:4.1.135.Final",
+    "io.netty:netty-codec-http:4.1.135.Final",
+    "io.netty:netty-codec-socks:4.1.135.Final",
+    "io.netty:netty-codec:4.1.135.Final",
+    "io.netty:netty-common:4.1.135.Final",
+    "io.netty:netty-handler-proxy:4.1.135.Final",
+    "io.netty:netty-handler:4.1.135.Final",
+    "io.netty:netty-resolver:4.1.135.Final",
     "io.netty:netty-tcnative-boringssl-static:2.0.75.Final",
     "io.netty:netty-tcnative-classes:2.0.75.Final",
-    "io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.133.Final",
-    "io.netty:netty-transport-native-unix-common:4.1.133.Final",
-    "io.netty:netty-transport:4.1.133.Final",
+    "io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.135.Final",
+    "io.netty:netty-transport-native-unix-common:4.1.135.Final",
+    "io.netty:netty-transport:4.1.135.Final",
     "io.opencensus:opencensus-api:0.31.0",
     "io.opencensus:opencensus-contrib-grpc-metrics:0.31.0",
     "io.perfmark:perfmark-api:0.27.0",
@@ -65,6 +65,7 @@ android_sdk_repository_extension = use_extension(
     "android_sdk_repository_extension",
 )
 use_repo(android_sdk_repository_extension, "androidsdk")
+
 register_toolchains("@androidsdk//:sdk-toolchain", "@androidsdk//:all")
 
 maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
@@ -83,6 +84,8 @@ use_repo(maven, "maven")
 grpc_android_maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven", dev_dependency = True)
 grpc_android_maven.install(
     name = "grpc_android_maven",
+    # Set this explicitly since the default guess is incorrect under Bzlmod.
+    aar_import_bzl_label = "@rules_android//rules:rules.bzl",
     artifacts = [
         "androidx.annotation:annotation:1.6.0",
         "androidx.annotation:annotation-jvm:1.6.0",
@@ -96,8 +99,6 @@ grpc_android_maven.install(
     strict_visibility = True,
     # For Bazel 8+ compatibility.
     use_starlark_android_rules = True,
-    # Set this explicitly since the default guess is incorrect under Bzlmod.
-    aar_import_bzl_label = "@rules_android//rules:rules.bzl",
 )
 use_repo(grpc_android_maven, "grpc_android_maven")
 

gradle/libs.versions.toml

@@ -104,17 +104,17 @@ mockito-android = "org.mockito:mockito-android:4.4.0"
 mockito-core = "org.mockito:mockito-core:4.4.0"
 # Need to decide when we require users to absorb the breaking changes in 4.2
 # checkForUpdates: netty-codec-http2:4.1.+
-netty-codec-http2 = "io.netty:netty-codec-http2:4.1.133.Final"
+netty-codec-http2 = "io.netty:netty-codec-http2:4.1.135.Final"
 # checkForUpdates: netty-handler-proxy:4.1.+
-netty-handler-proxy = "io.netty:netty-handler-proxy:4.1.133.Final"
+netty-handler-proxy = "io.netty:netty-handler-proxy:4.1.135.Final"
 # Keep the following references of tcnative version in sync whenever it's updated:
 #   SECURITY.md
 netty-tcnative = "io.netty:netty-tcnative-boringssl-static:2.0.75.Final"
 netty-tcnative-classes = "io.netty:netty-tcnative-classes:2.0.75.Final"
 # checkForUpdates: netty-transport-epoll:4.1.+
-netty-transport-epoll = "io.netty:netty-transport-native-epoll:4.1.133.Final"
+netty-transport-epoll = "io.netty:netty-transport-native-epoll:4.1.135.Final"
 # checkForUpdates: netty-unix-common:4.1.+
-netty-unix-common = "io.netty:netty-transport-native-unix-common:4.1.133.Final"
+netty-unix-common = "io.netty:netty-transport-native-unix-common:4.1.135.Final"
 okhttp = "com.squareup.okhttp:okhttp:2.7.5"
 # okio 3.5+ uses Kotlin 1.9+ which requires Android Gradle Plugin 9+
 # checkForUpdates: okio:3.4.+

netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java

@@ -98,6 +98,7 @@
 import io.netty.channel.socket.SocketChannelConfig;
 import io.netty.channel.socket.nio.NioServerSocketChannel;
 import io.netty.channel.socket.nio.NioSocketChannel;
+import io.netty.handler.codec.http2.Http2Exception;
 import io.netty.handler.codec.http2.StreamBufferingEncoder;
 import io.netty.handler.ssl.ClientAuth;
 import io.netty.handler.ssl.SslContext;
@@ -610,8 +611,12 @@ public void channelFactoryShouldNNotSetSocketOptionKeepAlive() throws Exception
   public void maxHeaderListSizeShouldBeEnforcedOnClient() throws Exception {
     startServer();
 
+    // We want the header list size limit to be half-way close to the RPC's header size, otherwise
+    // Netty will kill the connection instead of just the stream. While we can kill the connection,
+    // we want to make sure there's a more graceful failure first. We don't want a cliff where if an
+    // app adds one byte suddenly RPCs kill the connection.
     NettyClientTransport transport =
-        newTransport(newNegotiator(), DEFAULT_MAX_MESSAGE_SIZE, 1, null, true);
+        newTransport(newNegotiator(), DEFAULT_MAX_MESSAGE_SIZE, 75, null, true);
     callMeMaybe(transport.start(clientTransportListener));
     verify(clientTransportListener, timeout(5000)).transportReady();
 
@@ -621,11 +626,17 @@ public void maxHeaderListSizeShouldBeEnforcedOnClient() throws Exception {
       fail("The stream should have been failed due to client received header exceeds header list"
           + " size limit!");
     } catch (Exception e) {
-      Throwable rootCause = getRootCause(e);
-      Status status = ((StatusException) rootCause).getStatus();
+      Status status = ((StatusException) e.getCause()).getStatus();
       assertEquals(Status.Code.INTERNAL, status.getCode());
-      assertEquals("RST_STREAM closed stream. HTTP/2 error code: PROTOCOL_ERROR",
-          status.getDescription());
+      if (status.getCause() instanceof Http2Exception.StreamException) {
+        // Netty 4.1.135+ stream-level error that is generated by the client.
+        assertThat(status.getCause()).hasMessageThat()
+            .contains("Header size exceeded max allowed size");
+      } else {
+        // Older Netty failed the stream on the the server-side.
+        assertEquals("RST_STREAM closed stream. HTTP/2 error code: PROTOCOL_ERROR",
+            status.getDescription());
+      }
     }
   }
 

repositories.bzl

@@ -30,20 +30,20 @@ IO_GRPC_GRPC_JAVA_ARTIFACTS = [
     "dev.cel:common:0.12.0",
     "com.squareup.okhttp:okhttp:2.7.5",
     "com.squareup.okio:okio:2.10.0",  # 3.0+ needs swapping to -jvm; need work to avoid flag-day
-    "io.netty:netty-buffer:4.1.133.Final",
-    "io.netty:netty-codec-http2:4.1.133.Final",
-    "io.netty:netty-codec-http:4.1.133.Final",
-    "io.netty:netty-codec-socks:4.1.133.Final",
-    "io.netty:netty-codec:4.1.133.Final",
-    "io.netty:netty-common:4.1.133.Final",
-    "io.netty:netty-handler-proxy:4.1.133.Final",
-    "io.netty:netty-handler:4.1.133.Final",
-    "io.netty:netty-resolver:4.1.133.Final",
+    "io.netty:netty-buffer:4.1.135.Final",
+    "io.netty:netty-codec-http2:4.1.135.Final",
+    "io.netty:netty-codec-http:4.1.135.Final",
+    "io.netty:netty-codec-socks:4.1.135.Final",
+    "io.netty:netty-codec:4.1.135.Final",
+    "io.netty:netty-common:4.1.135.Final",
+    "io.netty:netty-handler-proxy:4.1.135.Final",
+    "io.netty:netty-handler:4.1.135.Final",
+    "io.netty:netty-resolver:4.1.135.Final",
     "io.netty:netty-tcnative-boringssl-static:2.0.75.Final",
     "io.netty:netty-tcnative-classes:2.0.75.Final",
-    "io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.133.Final",
-    "io.netty:netty-transport-native-unix-common:4.1.133.Final",
-    "io.netty:netty-transport:4.1.133.Final",
+    "io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.135.Final",
+    "io.netty:netty-transport-native-unix-common:4.1.135.Final",
+    "io.netty:netty-transport:4.1.135.Final",
     "io.opencensus:opencensus-api:0.31.0",
     "io.opencensus:opencensus-contrib-grpc-metrics:0.31.0",
     "io.perfmark:perfmark-api:0.27.0",

s2a/src/main/java/io/grpc/s2a/internal/handshaker/S2ATrustManager.java

@@ -28,17 +28,19 @@
 import com.google.s2a.proto.v2.ValidatePeerCertificateChainResp;
 import io.grpc.s2a.internal.handshaker.S2AIdentity;
 import java.io.IOException;
+import java.net.Socket;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.util.Optional;
 import javax.annotation.concurrent.NotThreadSafe;
-import javax.net.ssl.X509TrustManager;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** Offloads verification of the peer certificate chain to S2A. */
 @NotThreadSafe
-final class S2ATrustManager implements X509TrustManager {
+final class S2ATrustManager extends X509ExtendedTrustManager {
   private final Optional<S2AIdentity> localIdentity;
   private final S2AStub stub;
   private final String hostname;
@@ -71,6 +73,18 @@ public void checkClientTrusted(X509Certificate[] chain, String authType)
     checkPeerTrusted(chain, /* isCheckingClientCertificateChain= */ true);
   }
 
+  @Override
+  public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
+      throws CertificateException {
+    checkClientTrusted(chain, authType);
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
+      throws CertificateException {
+    checkClientTrusted(chain, authType);
+  }
+
   /**
    * Validates the given certificate chain provided by the peer.
    *
@@ -86,6 +100,18 @@ public void checkServerTrusted(X509Certificate[] chain, String authType)
     checkPeerTrusted(chain, /* isCheckingClientCertificateChain= */ false);
   }
 
+  @Override
+  public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
+      throws CertificateException {
+    checkServerTrusted(chain, authType);
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
+      throws CertificateException {
+    checkServerTrusted(chain, authType);
+  }
+
   /**
    * Returns null because the accepted issuers are held in S2A and this class receives decision made
    * from S2A on the fly about which to use to verify a given chain.
@@ -156,4 +182,4 @@ private static ImmutableList<ByteString> certificateChainToDerChain(X509Certific
     }
     return derChain.build();
   }
-}
+}