[GitHub] [cassandra] dineshjoshi commented on a change in pull request #763: CASSANDRA-16144 TLS connections to the storage port on a node without server encryption configured causes java.io.IOException accessing missing keystore

[GitBox <gi...@apache.org>]

dineshjoshi commented on a change in pull request #763:
URL: https://github.com/apache/cassandra/pull/763#discussion_r498988230



##########
File path: src/java/org/apache/cassandra/config/EncryptionOptions.java
##########
@@ -27,6 +28,23 @@
 
 public class EncryptionOptions
 {
+    public enum TlsEncryptionPolicy
+    {
+        unencrypted("unencrypted"), optional("optionally encrypted"), encrypted("encrypted");

Review comment:
       Usual convention is to use upper case identifiers for enum elements.

##########
File path: src/java/org/apache/cassandra/net/SocketFactory.java
##########
@@ -228,39 +228,41 @@ static SslHandler newSslHandler(Channel channel, SslContext sslContext, @Nullabl
         return sslHandler;
     }
 
-    static String encryptionLogStatement(EncryptionOptions options)
+    /**
+     * Summarizes the intended encryption options, suitable for logging. Once a connection is established, use
+     * {@link SocketFactory#encryptionConnectionSummary} below.
+     * @param options options to summarize
+     * @return description of encryption options
+     */
+    static String encryptionOptionsSummary(EncryptionOptions options)

Review comment:
       Would it be better to simply change this to:
   ```java
   if (options == null)
       return "disabled";
   
   String encryptionType = SSLFactory.openSslIsAvailable() ? "openssl" : "jdk";
   return options.tlsEncryptionPolicy().description() + '(' + encryptionType + ')';
   ```

##########
File path: src/java/org/apache/cassandra/net/InboundConnectionSettings.java
##########
@@ -83,8 +83,8 @@ public boolean authenticate(InetAddress address, int port)
 
     public String toString()
     {
-        return format("address: (%s), nic: %s, encryption: %s",
-                      bindAddress, FBUtilities.getNetworkInterface(bindAddress.address), SocketFactory.encryptionLogStatement(null, encryption));
+        return format("address: (%s), nic: %s, %s",

Review comment:
       Could we add back `encryption: ` label?

##########
File path: src/java/org/apache/cassandra/transport/Server.java
##########
@@ -139,18 +139,19 @@ public synchronized void start()
         if (workerGroup != null)
             bootstrap = bootstrap.group(workerGroup);
 
+        final EncryptionOptions clientEnc = DatabaseDescriptor.getNativeProtocolEncryptionOptions();
+
         if (this.useSSL)
         {
-            final EncryptionOptions clientEnc = DatabaseDescriptor.getNativeProtocolEncryptionOptions();
-
-            if (clientEnc.optional)
+            if (clientEnc.tlsEncryptionPolicy() == EncryptionOptions.TlsEncryptionPolicy.optional)
             {
-                logger.info("Enabling optionally encrypted CQL connections between client and server");
+                logger.debug("Enabling optionally encrypted CQL connections between client and server");
                 bootstrap.childHandler(new OptionalSecureInitializer(this, clientEnc));
             }
             else
             {
-                logger.info("Enabling encrypted CQL connections between client and server");
+                logger.debug("Enabling encrypted CQL connections between client and server");
+                assert clientEnc.tlsEncryptionPolicy() == EncryptionOptions.TlsEncryptionPolicy.encrypted : "Unexpected policy: " + clientEnc.tlsEncryptionPolicy();

Review comment:
       Is `clientEnc.tlsEncryptionPolicy()` guaranteed to be non-null?




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org