diff --git a/CHANGES.txt b/CHANGES.txt index 68515312fe..5e17fd8af5 100644 --- 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 index fdf669697c..39e584d257 100644 --- a/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java +++ b/src/java/org/apache/cassandra/security/FileBasedSslContextFactory.java @@ -128,12 +128,11 @@ public abstract class FileBasedSslContextFactory extends AbstractSslContextFacto * * @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 @@ public abstract 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, 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 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 fc84b3a00b..98e0342797 100644 --- a/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java +++ b/test/unit/org/apache/cassandra/security/FileBasedSslContextFactoryTest.java @@ -90,37 +90,34 @@ public class FileBasedSslContextFactoryTest } /** - * 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);