Merge branch 'cassandra-4.1' into cassandra-5.0

apache · Aug 28, 2023 · a9606c6 · a9606c6
2 parents d2f8e8e + b47bee4
commit a9606c6
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 22 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,6 +1,7 @@
 5.0-alpha2
  * Forbid SAI indexes with analysis options on primary key columns (CASSANDRA-18782)
 Merged from 4.1:
+ * Allow empty keystore_password in encryption_options (CASSANDRA-18778)
 Merged from 4.0:
 Merged from 3.11:
 Merged from 3.0:

diff --git a/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java b/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java
@@ -128,12 +128,11 @@ public synchronized void initHotReloading()
      *
      * @param isOutboundKeystore {@code true} for the {@code outbound_keystore_password};{@code false} otherwise
      * @param password           value
-     * @throws IllegalArgumentException if the {@code password} is empty as per the definition of {@link StringUtils#isEmpty(CharSequence)}
+     * @throws IllegalArgumentException if the {@code password} is null
      */
     protected void validatePassword(boolean isOutboundKeystore, String password)
     {
-        boolean keystorePasswordEmpty = StringUtils.isEmpty(password);
-        if (keystorePasswordEmpty)
+        if (password == null)
         {
             String keyName = isOutboundKeystore ? "outbound_" : "";
             final String msg = String.format("'%skeystore_password' must be specified", keyName);
@@ -205,14 +204,15 @@ private KeyManagerFactory getKeyManagerFactory(final FileBasedStoreContext conte
             final String algorithm = this.algorithm == null ? KeyManagerFactory.getDefaultAlgorithm() : this.algorithm;
             KeyManagerFactory kmf = KeyManagerFactory.getInstance(algorithm);
             KeyStore ks = KeyStore.getInstance(store_type);
-            ks.load(ksf, context.password.toCharArray());
+            final char[] password = context.password.toCharArray();
+            ks.load(ksf, password);
 
             if (!context.checkedExpiry)
             {
                 checkExpiredCerts(ks);
                 context.checkedExpiry = true;
             }
-            kmf.init(ks, context.password.toCharArray());
+            kmf.init(ks, password);
             return kmf;
         }
         catch (Exception e)

diff --git a/test/conf/cassandra_ssl_test_nopassword.keystore b/test/conf/cassandra_ssl_test_nopassword.keystore
diff --git a/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java b/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java
@@ -90,37 +90,34 @@ public void testHappyPath() throws SSLException
     }
 
     /**
-     * Tests for empty {@code keystore_password} and empty {@code outbound_keystore_password} configurations.
+     * Tests that empty {@code keystore_password} and {@code outbound_keystore_password} is allowed.
      */
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testEmptyKeystorePasswords() throws SSLException
     {
-        EncryptionOptions.ServerEncryptionOptions localEncryptionOptions = encryptionOptions.withKeyStorePassword(null).withOutboundKeystorePassword(null);
+        EncryptionOptions.ServerEncryptionOptions localEncryptionOptions = encryptionOptions
+                                                                           .withKeyStorePassword("")
+                                                                           .withKeyStore("test/conf/cassandra_ssl_test_nopassword.keystore")
+                                                                           .withOutboundKeystorePassword("")
+                                                                           .withOutboundKeystore("test/conf/cassandra_ssl_test_nopassword.keystore");
 
         Assert.assertEquals("org.apache.cassandra.security.FileBasedSslContextFactoryTest$TestFileBasedSSLContextFactory",
                             localEncryptionOptions.ssl_context_factory.class_name);
-        Assert.assertNull("keystore_password must be null", localEncryptionOptions.keystore_password);
-        Assert.assertNull("outbound_keystore_password must be null", localEncryptionOptions.outbound_keystore_password);
+        Assert.assertEquals("keystore_password must be empty", "", localEncryptionOptions.keystore_password);
+        Assert.assertEquals("outbound_keystore_password must empty", "", localEncryptionOptions.outbound_keystore_password);
 
         TestFileBasedSSLContextFactory sslContextFactory =
         (TestFileBasedSSLContextFactory) localEncryptionOptions.sslContextFactoryInstance;
-        try
-        {
-            sslContextFactory.buildKeyManagerFactory();
-            sslContextFactory.buildTrustManagerFactory();
-        }
-        catch (Exception e)
-        {
-            Assert.assertEquals("'keystore_password' must be specified", e.getMessage());
-            throw e;
-        }
+
+        sslContextFactory.buildKeyManagerFactory();
+        sslContextFactory.buildTrustManagerFactory();
     }
 
     /**
-     * Tests for the empty password for the {@code keystore} used for the client communication.
+     * Tests that an absent keystore_password for the {@code keystore} is disallowed.
      */
     @Test(expected = IllegalArgumentException.class)
-    public void testEmptyKeystorePassword() throws SSLException
+    public void testNullKeystorePasswordDisallowed() throws SSLException
     {
         EncryptionOptions.ServerEncryptionOptions localEncryptionOptions = encryptionOptions.withKeyStorePassword(null);