You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "fapifta (via GitHub)" <gi...@apache.org> on 2026/07/10 10:16:00 UTC

[PR] RATIS-2598. TLS provider/protocol/cipher config is not applied to DataStream server/client. [ratis]

fapifta opened a new pull request, #1511:
URL: https://github.com/apache/ratis/pull/1511

   ## What changes were proposed in this pull request?
   
   The TLS provider, protocol list, and cipher suites configured on a TlsConf were applied on the gRPC transport but ignored on the Netty DataStream transport, so the DataStream server and client always used the provider defaults.
   
   This adds a small helper in NettyUtils that applies those settings when building the SslContext, and calls it for both the server and client. Ciphers are applied through SupportedCipherSuiteFilter (rather than the gRPC path's IdentityCipherSuiteFilter) so that an unsupported or misspelled cipher is dropped instead of crashing the server. Nothing changes when these settings are left unset.
   
   Generated-by: Claude Code (Claude Opus 4.8)
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/RATIS-2598
   
   ## How was this patch tested?
   
   Added three tests to TestTlsConfWithNetty covering the real NettyUtils code path: one checks that a configured cipher actually ends up on the SslContext, one checks that an unsupported cipher is filtered out instead of failing the build, and one runs a full TLSv1.2 handshake with an explicit protocol and cipher list. All five tests in the class pass, and checkstyle is clean on the two changed modules.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RATIS-2598. TLS provider/protocol/cipher config is not applied to DataStream server/client. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1511:
URL: https://github.com/apache/ratis/pull/1511#discussion_r3560896866


##########
ratis-common/src/main/java/org/apache/ratis/util/NettyUtils.java:
##########
@@ -185,6 +187,32 @@ static SslContextBuilder initSslContextBuilderForClient(TlsConf tlsConf) {
     if (tlsConf.isMutualTls()) {
       setKeyManager(b, tlsConf.getKeyManager());
     }
+    return configureSslContextBuilder(b, tlsConf);
+  }
+
+  /**
+   * Apply the {@link SslProvider}, enabled TLS protocols and cipher suites from the given
+   * {@link TlsConf} to the builder, so that the Netty DataStream transport honours the same
+   * configuration instead of falling back on the provider defaults.
+   *
+   * <p>Unlike the gRPC path ({@code GrpcUtil.configureSslContextBuilder}) this uses
+   * {@link SupportedCipherSuiteFilter}, which intersects the requested cipher suites with the
+   * ones actually supported by the negotiated provider/engine, rather than passing them through
+   * verbatim.
+   */
+  static SslContextBuilder configureSslContextBuilder(SslContextBuilder b, TlsConf tlsConf) {
+    final SslProvider sslProvider = tlsConf.getSslProvider();
+    if (sslProvider != null) {
+      b.sslProvider(sslProvider);
+    }

Review Comment:
   We should set also jsseProvider:
   ```java
       final String jsseProviderName = tlsConf.getJsseProviderName();
       if (jsseProviderName != null) {
         final Provider jsseProvider = Security.getProvider(jsseProviderName);
         b.sslContextProvider(jsseProvider);
       }
   ```



##########
ratis-common/src/main/java/org/apache/ratis/util/NettyUtils.java:
##########
@@ -185,6 +187,32 @@ static SslContextBuilder initSslContextBuilderForClient(TlsConf tlsConf) {
     if (tlsConf.isMutualTls()) {
       setKeyManager(b, tlsConf.getKeyManager());
     }
+    return configureSslContextBuilder(b, tlsConf);
+  }
+
+  /**
+   * Apply the {@link SslProvider}, enabled TLS protocols and cipher suites from the given
+   * {@link TlsConf} to the builder, so that the Netty DataStream transport honours the same
+   * configuration instead of falling back on the provider defaults.
+   *
+   * <p>Unlike the gRPC path ({@code GrpcUtil.configureSslContextBuilder}) this uses
+   * {@link SupportedCipherSuiteFilter}, which intersects the requested cipher suites with the
+   * ones actually supported by the negotiated provider/engine, rather than passing them through
+   * verbatim.
+   */
+  static SslContextBuilder configureSslContextBuilder(SslContextBuilder b, TlsConf tlsConf) {
+    final SslProvider sslProvider = tlsConf.getSslProvider();
+    if (sslProvider != null) {
+      b.sslProvider(sslProvider);
+    }
+    final List<String> protocols = tlsConf.getProtocols();
+    if (protocols != null && !protocols.isEmpty()) {
+      b.protocols(protocols.toArray(new String[0]));

Review Comment:
   toArray is not needed, i.e.
   ```java
         b.protocols(protocols);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RATIS-2598. TLS provider/protocol/cipher config is not applied to DataStream server/client. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1511:
URL: https://github.com/apache/ratis/pull/1511#discussion_r3560896866


##########
ratis-common/src/main/java/org/apache/ratis/util/NettyUtils.java:
##########
@@ -185,6 +187,32 @@ static SslContextBuilder initSslContextBuilderForClient(TlsConf tlsConf) {
     if (tlsConf.isMutualTls()) {
       setKeyManager(b, tlsConf.getKeyManager());
     }
+    return configureSslContextBuilder(b, tlsConf);
+  }
+
+  /**
+   * Apply the {@link SslProvider}, enabled TLS protocols and cipher suites from the given
+   * {@link TlsConf} to the builder, so that the Netty DataStream transport honours the same
+   * configuration instead of falling back on the provider defaults.
+   *
+   * <p>Unlike the gRPC path ({@code GrpcUtil.configureSslContextBuilder}) this uses
+   * {@link SupportedCipherSuiteFilter}, which intersects the requested cipher suites with the
+   * ones actually supported by the negotiated provider/engine, rather than passing them through
+   * verbatim.
+   */
+  static SslContextBuilder configureSslContextBuilder(SslContextBuilder b, TlsConf tlsConf) {
+    final SslProvider sslProvider = tlsConf.getSslProvider();
+    if (sslProvider != null) {
+      b.sslProvider(sslProvider);
+    }

Review Comment:
   We should set also jsseProvider:
   (Revised below)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RATIS-2598. TLS provider/protocol/cipher config is not applied to DataStream server/client. [ratis]

Posted by "fapifta (via GitHub)" <gi...@apache.org>.
fapifta commented on PR #1511:
URL: https://github.com/apache/ratis/pull/1511#issuecomment-4939294167

   Thank you for the review @szetszwo I have implemented the suggested changes, please take a look.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RATIS-2598. TLS provider/protocol/cipher config is not applied to DataStream server/client. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1511:
URL: https://github.com/apache/ratis/pull/1511#discussion_r3560941822


##########
ratis-common/src/main/java/org/apache/ratis/util/NettyUtils.java:
##########
@@ -185,6 +187,32 @@ static SslContextBuilder initSslContextBuilderForClient(TlsConf tlsConf) {
     if (tlsConf.isMutualTls()) {
       setKeyManager(b, tlsConf.getKeyManager());
     }
+    return configureSslContextBuilder(b, tlsConf);
+  }
+
+  /**
+   * Apply the {@link SslProvider}, enabled TLS protocols and cipher suites from the given
+   * {@link TlsConf} to the builder, so that the Netty DataStream transport honours the same
+   * configuration instead of falling back on the provider defaults.
+   *
+   * <p>Unlike the gRPC path ({@code GrpcUtil.configureSslContextBuilder}) this uses
+   * {@link SupportedCipherSuiteFilter}, which intersects the requested cipher suites with the
+   * ones actually supported by the negotiated provider/engine, rather than passing them through
+   * verbatim.
+   */
+  static SslContextBuilder configureSslContextBuilder(SslContextBuilder b, TlsConf tlsConf) {
+    final SslProvider sslProvider = tlsConf.getSslProvider();
+    if (sslProvider != null) {
+      b.sslProvider(sslProvider);
+    }

Review Comment:
   GrpcUtil has a getJsseProvider(..) method.  Let's move it to NettyUtils and reuse it, i.e.:
   ```diff
   @@ -185,9 +191,51 @@ public interface NettyUtils {
        if (tlsConf.isMutualTls()) {
          setKeyManager(b, tlsConf.getKeyManager());
        }
   +    return configureSslContextBuilder(b, tlsConf);
   +  }
   +
   +  /**
   +   * Apply the {@link SslProvider}, enabled TLS protocols and cipher suites from the given
   +   * {@link TlsConf} to the builder, so that the Netty DataStream transport honours the same
   +   * configuration instead of falling back on the provider defaults.
   +   *
   +   * <p>Unlike the gRPC path ({@code GrpcUtil.configureSslContextBuilder}) this uses
   +   * {@link SupportedCipherSuiteFilter}, which intersects the requested cipher suites with the
   +   * ones actually supported by the negotiated provider/engine, rather than passing them through
   +   * verbatim.
   +   */
   +  static SslContextBuilder configureSslContextBuilder(SslContextBuilder b, TlsConf tlsConf) {
   +    final SslProvider sslProvider = tlsConf.getSslProvider();
   +    if (sslProvider != null) {
   +      b.sslProvider(sslProvider);
   +    }
   +    final Provider jsseProvider = getJsseProvider(tlsConf);
   +    if (jsseProvider != null) {
   +      b.sslContextProvider(jsseProvider);
   +    }
   +    final List<String> protocols = tlsConf.getProtocols();
   +    if (protocols != null && !protocols.isEmpty()) {
   +      b.protocols(protocols);
   +    }
   +    final List<String> cipherSuites = tlsConf.getCipherSuites();
   +    if (cipherSuites != null && !cipherSuites.isEmpty()) {
   +      b.ciphers(cipherSuites, SupportedCipherSuiteFilter.INSTANCE);
   +    }
        return b;
      }
    
   +  static Provider getJsseProvider(TlsConf tlsConf) {
   +    final String providerName = tlsConf.getJsseProviderName();
   +    if (providerName == null || providerName.trim().isEmpty()) {
   +      return null;
   +    }
   +    final Provider namedProvider = Security.getProvider(providerName.trim());
   +    if (namedProvider == null) {
   +      throw new IllegalArgumentException("JSSE provider not found: " + providerName);
   +    }
   +    return namedProvider;
   +  }
   +
      static SslContext buildSslContextForClient(TlsConf tlsConf) {
        return buildSslContext("client", tlsConf, NettyUtils::initSslContextBuilderForClient);
      }
   diff --git a/ratis-grpc/src/main/java/org/apache/ratis/grpc/GrpcUtil.java b/ratis-grpc/src/main/java/org/apache/ratis/grpc/GrpcUtil.java
   index 36b8e9d65..e51d827ec 100644
   --- a/ratis-grpc/src/main/java/org/apache/ratis/grpc/GrpcUtil.java
   +++ b/ratis-grpc/src/main/java/org/apache/ratis/grpc/GrpcUtil.java
   @@ -58,6 +58,7 @@ import java.util.function.Function;
    import java.util.function.Supplier;
    
    import static org.apache.ratis.thirdparty.io.netty.handler.ssl.SslProvider.OPENSSL;
   +import static org.apache.ratis.util.NettyUtils.getJsseProvider;
    
    public interface GrpcUtil {
      Logger LOG = LoggerFactory.getLogger(GrpcUtil.class);
   @@ -357,18 +358,6 @@ public interface GrpcUtil {
        }
      }
    
   -  static Provider getJsseProvider(TlsConf tlsConf) {
   -    final String providerName = tlsConf.getJsseProviderName();
   -    if (providerName == null || providerName.trim().isEmpty()) {
   -      return null;
   -    }
   -    final Provider namedProvider = Security.getProvider(providerName.trim());
   -    if (namedProvider == null) {
   -      throw new IllegalArgumentException("JSSE provider not found: " + providerName);
   -    }
   -    return namedProvider;
   -  }
   -
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RATIS-2598. TLS provider/protocol/cipher config is not applied to DataStream server/client. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #1511:
URL: https://github.com/apache/ratis/pull/1511


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org