diff --git a/CHANGES.txt b/CHANGES.txt index 6e5744abf7..21ca2abec7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1.4 + * Allow empty keystore_password in encryption_options (CASSANDRA-18778) * Skip ColumnFamilyStore#topPartitions initialization when client or tool mode (CASSANDRA-18697) Merged from 4.0: * Fix NTS log message when an unrecognized strategy option is passed (CASSANDRA-18679) diff --git a/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java b/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java index 9876eb4b89..3643ad67d1 100644 --- a/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java +++ b/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java @@ -123,12 +123,11 @@ abstract public class FileBasedSslContextFactory extends AbstractSslContextFacto * Validates the given keystore password. * * @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(String password) { - boolean keystorePasswordEmpty = StringUtils.isEmpty(password); - if (keystorePasswordEmpty) + if (password == null) { throw new IllegalArgumentException("'keystore_password' must be specified"); } @@ -155,13 +154,14 @@ abstract public class FileBasedSslContextFactory extends AbstractSslContextFacto final String algorithm = this.algorithm == null ? KeyManagerFactory.getDefaultAlgorithm() : this.algorithm; KeyManagerFactory kmf = KeyManagerFactory.getInstance(algorithm); KeyStore ks = KeyStore.getInstance(store_type); - ks.load(ksf, keystore_password.toCharArray()); + final char[] password = keystore_password.toCharArray(); + ks.load(ksf, password); if (!checkedExpiry) { checkExpiredCerts(ks); checkedExpiry = true; } - kmf.init(ks, keystore_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 new file mode 100644 index 0000000000..8778a3876b Binary files /dev/null and b/test/conf/cassandra_ssl_test_nopassword.keystore differ diff --git a/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java b/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java index be49d163eb..ff45361e27 100644 --- a/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java +++ b/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java @@ -81,36 +81,31 @@ public class FileBasedSslContextFactoryTest } /** - * Tests for empty {@code keystore_password} and empty {@code outbound_keystore_password} configurations. + * Tests that empty {@code keystore_password} is allowed. */ - @Test(expected = IllegalArgumentException.class) + @Test public void testEmptyKeystorePasswords() throws SSLException { - EncryptionOptions.ServerEncryptionOptions localEncryptionOptions = encryptionOptions.withKeyStorePassword(null); + EncryptionOptions.ServerEncryptionOptions localEncryptionOptions = encryptionOptions + .withKeyStorePassword("") + .withKeyStore("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.assertEquals("keystore_password must be empty", "", localEncryptionOptions.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);