Build set of TLS versions from S2Av2's GetTlsConfigResp.

rmehta19 · rmehta19 · commit da330cd1f7c4 · 2024-09-09T13:01:12.000-07:00
diff --git a/s2a/src/main/java/io/grpc/s2a/handshaker/ProtoUtil.java b/s2a/src/main/java/io/grpc/s2a/handshaker/ProtoUtil.java
@@ -16,6 +16,8 @@
 
 package io.grpc.s2a.handshaker;
 
+import com.google.common.collect.ImmutableSet;
+
 /** Converts proto messages to Netty strings. */
 final class ProtoUtil {
   /**
@@ -68,5 +70,27 @@ static String convertTlsProtocolVersion(TLSVersion tlsVersion) {
     }
   }
 
+  /**
+   * Builds a set of strings representing all {@link TLSVersion}s between {@code minTlsVersion} and
+   * {@code maxTlsVersion}.
+   */
+  static ImmutableSet<String> buildTlsProtocolVersionSet(
+      TLSVersion minTlsVersion, TLSVersion maxTlsVersion) {
+    ImmutableSet.Builder<String> tlsVersions = ImmutableSet.<String>builder();
+    for (TLSVersion tlsVersion : TLSVersion.values()) {
+      int versionNumber;
+      try {
+        versionNumber = tlsVersion.getNumber();
+      } catch (IllegalArgumentException e) {
+        continue;
+      }
+      if (versionNumber < minTlsVersion.getNumber() || versionNumber > maxTlsVersion.getNumber()) {
+        continue;
+      }
+      tlsVersions.add(convertTlsProtocolVersion(tlsVersion));
+    }
+    return tlsVersions.build();
+  }
+
   private ProtoUtil() {}
 }
diff --git a/s2a/src/main/java/io/grpc/s2a/handshaker/SslContextFactory.java b/s2a/src/main/java/io/grpc/s2a/handshaker/SslContextFactory.java
@@ -19,7 +19,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import io.grpc.netty.GrpcSslContexts;
 import io.grpc.s2a.handshaker.S2AIdentity;
 import io.netty.handler.ssl.OpenSslContextOption;
@@ -138,14 +138,13 @@ private static void configureSslContextWithClientTlsConfiguration(
           NoSuchAlgorithmException,
           UnrecoverableKeyException {
     sslContextBuilder.keyManager(createKeylessManager(clientTlsConfiguration));
-    sslContextBuilder.protocols(
-        ProtoUtil.convertTlsProtocolVersion(clientTlsConfiguration.getMinTlsVersion()),
-        ProtoUtil.convertTlsProtocolVersion(clientTlsConfiguration.getMaxTlsVersion()));
-    ImmutableList.Builder<String> ciphersuites = ImmutableList.<String>builder();
-    for (int i = 0; i < clientTlsConfiguration.getCiphersuitesCount(); ++i) {
-      ciphersuites.add(ProtoUtil.convertCiphersuite(clientTlsConfiguration.getCiphersuites(i)));
+    ImmutableSet<String> tlsVersions =
+        ProtoUtil.buildTlsProtocolVersionSet(
+            clientTlsConfiguration.getMinTlsVersion(), clientTlsConfiguration.getMaxTlsVersion());
+    if (tlsVersions.isEmpty()) {
+      throw new S2AConnectionException("Set of TLS versions received from S2A server is empty.");
     }
-    sslContextBuilder.ciphers(ciphersuites.build());
+    sslContextBuilder.protocols(tlsVersions);
   }
 
   private static KeyManager createKeylessManager(
diff --git a/s2a/src/test/java/io/grpc/s2a/handshaker/FakeWriter.java b/s2a/src/test/java/io/grpc/s2a/handshaker/FakeWriter.java
@@ -16,6 +16,7 @@
 
 package io.grpc.s2a.handshaker;
 
+import static io.grpc.s2a.handshaker.TLSVersion.TLS_VERSION_1_2;
 import static io.grpc.s2a.handshaker.TLSVersion.TLS_VERSION_1_3;
 
 import com.google.common.collect.ImmutableMap;
@@ -39,7 +40,8 @@ enum Behavior {
     EMPTY_RESPONSE,
     ERROR_STATUS,
     ERROR_RESPONSE,
-    COMPLETE_STATUS
+    COMPLETE_STATUS,
+    BAD_TLS_VERSION_RESPONSE,
   }
 
   enum VerificationResult {
@@ -213,6 +215,20 @@ public void onNext(SessionReq sessionReq) {
       case COMPLETE_STATUS:
         reader.onCompleted();
         break;
+      case BAD_TLS_VERSION_RESPONSE:
+        reader.onNext(
+            SessionResp.newBuilder()
+                .setGetTlsConfigurationResp(
+                    GetTlsConfigurationResp.newBuilder()
+                        .setClientTlsConfiguration(
+                            GetTlsConfigurationResp.ClientTlsConfiguration.newBuilder()
+                                .addCertificateChain(LEAF_CERT)
+                                .addCertificateChain(INTERMEDIATE_CERT_2)
+                                .addCertificateChain(INTERMEDIATE_CERT_1)
+                                .setMinTlsVersion(TLS_VERSION_1_3)
+                                .setMaxTlsVersion(TLS_VERSION_1_2)))
+                .build());
+        break;
       default:
         reader.onNext(handleResponse(sessionReq));
     }
diff --git a/s2a/src/test/java/io/grpc/s2a/handshaker/ProtoUtilTest.java b/s2a/src/test/java/io/grpc/s2a/handshaker/ProtoUtilTest.java
@@ -18,6 +18,7 @@
 
 import static org.junit.Assert.assertThrows;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.truth.Expect;
 import org.junit.Rule;
 import org.junit.Test;
@@ -92,4 +93,39 @@ public void convertTlsProtocolVersion_withUnknownTlsVersion_fails() {
             () -> ProtoUtil.convertTlsProtocolVersion(TLSVersion.TLS_VERSION_UNSPECIFIED));
     expect.that(expected).hasMessageThat().isEqualTo("TLS version 0 is not supported.");
   }
+
+  @Test
+  public void buildTlsProtocolVersionSet_success() {
+    expect
+        .that(
+            ProtoUtil.buildTlsProtocolVersionSet(
+                TLSVersion.TLS_VERSION_1_0, TLSVersion.TLS_VERSION_1_3))
+        .isEqualTo(ImmutableSet.of("TLSv1", "TLSv1.1", "TLSv1.2", "TLSv1.3"));
+    expect
+        .that(
+            ProtoUtil.buildTlsProtocolVersionSet(
+                TLSVersion.TLS_VERSION_1_2, TLSVersion.TLS_VERSION_1_2))
+        .isEqualTo(ImmutableSet.of("TLSv1.2"));
+    expect
+        .that(
+            ProtoUtil.buildTlsProtocolVersionSet(
+                TLSVersion.TLS_VERSION_1_3, TLSVersion.TLS_VERSION_1_3))
+        .isEqualTo(ImmutableSet.of("TLSv1.3"));
+    expect
+        .that(
+            ProtoUtil.buildTlsProtocolVersionSet(
+                TLSVersion.TLS_VERSION_1_3, TLSVersion.TLS_VERSION_1_2))
+        .isEmpty();
+  }
+
+  @Test
+  public void buildTlsProtocolVersionSet_failure() {
+    AssertionError expected =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                ProtoUtil.buildTlsProtocolVersionSet(
+                    TLSVersion.TLS_VERSION_UNSPECIFIED, TLSVersion.TLS_VERSION_1_3));
+    expect.that(expected).hasMessageThat().isEqualTo("TLS version 0 is not supported.");
+  }
 }
diff --git a/s2a/src/test/java/io/grpc/s2a/handshaker/SslContextFactoryTest.java b/s2a/src/test/java/io/grpc/s2a/handshaker/SslContextFactoryTest.java
@@ -58,12 +58,6 @@ public void createForClient_returnsValidSslContext() throws Exception {
     expect.that(sslContext.sessionTimeout()).isEqualTo(300);
     expect.that(sslContext.isClient()).isTrue();
     expect.that(sslContext.applicationProtocolNegotiator().protocols()).containsExactly("h2");
-    expect
-        .that(sslContext.cipherSuites())
-        .containsExactly(
-            "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
-            "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
-            "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256");
     SSLSessionContext sslSessionContext = sslContext.sessionContext();
     if (sslSessionContext instanceof OpenSslSessionContext) {
       OpenSslSessionContext openSslSessionContext = (OpenSslSessionContext) sslSessionContext;
@@ -82,12 +76,6 @@ public void createForClient_withLocalIdentity_returnsValidSslContext() throws Ex
     expect.that(sslContext.sessionTimeout()).isEqualTo(300);
     expect.that(sslContext.isClient()).isTrue();
     expect.that(sslContext.applicationProtocolNegotiator().protocols()).containsExactly("h2");
-    expect
-        .that(sslContext.cipherSuites())
-        .containsExactly(
-            "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
-            "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
-            "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256");
     SSLSessionContext sslSessionContext = sslContext.sessionContext();
     if (sslSessionContext instanceof OpenSslSessionContext) {
       OpenSslSessionContext openSslSessionContext = (OpenSslSessionContext) sslSessionContext;
@@ -141,6 +129,22 @@ public void createForClient_getsErrorFromServer_throwsError() throws Exception {
         .contains("Failed to get client TLS configuration from S2A.");
   }
 
+  @Test
+  public void createForClient_getsBadTlsVersionsFromServer_throwsError() throws Exception {
+    writer.setBehavior(FakeWriter.Behavior.BAD_TLS_VERSION_RESPONSE);
+
+    S2AConnectionException expected =
+        assertThrows(
+            S2AConnectionException.class,
+            () ->
+                SslContextFactory.createForClient(
+                    stub, FAKE_TARGET_NAME, /* localIdentity= */ Optional.empty()));
+
+    assertThat(expected)
+        .hasMessageThat()
+        .contains("Set of TLS versions received from S2A server is empty.");
+  }
+
   @Test
   public void createForClient_nullStub_throwsError() throws Exception {
     writer.sendUnexpectedResponse();
