From b47bee42d3b15020fbae72b173e873fa57c8e0c8 Mon Sep 17 00:00:00 2001 From: Andy Tolbert <6889771+tolbertam@users.noreply.github.com> Date: Fri, 18 Aug 2023 09:33:57 -0500 Subject: [PATCH] Allow empty keystore_password in encryption_options patch by Andy Tolbert; reviewed by Jon Meredith and Stefan Miklosovic for CASSANDRA-18778 --- CHANGES.txt | 1 + .../security/FileBasedSslContextFactory.java | 10 +++---- .../cassandra_ssl_test_nopassword.keystore | Bin 0 -> 2517 bytes .../FileBasedSslContextFactoryTest.java | 27 +++++++----------- 4 files changed, 17 insertions(+), 21 deletions(-) create mode 100644 test/conf/cassandra_ssl_test_nopassword.keystore 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 0000000000000000000000000000000000000000..8778a3876b272b6f6b801d458c5155c2f58701a7 GIT binary patch literal 2517 zcmY+^cQhM{9tUudNE5qlVy{p|t=Oy7s!b7lR7-s|q&LlUA}?MK^GCjLI&rD5myR3M#+cs8(y zrZ9>vPMw`nHxLH1;`!FXs#E;H96LmE1FZ zB=wb_IKC7bgV)I2ptB*t`SAB-B2es$?+hLM6D#}zk0O4KxB4s(9q4S|RozsV&A|oj z*0C+SPljmsDsp65?_850uSQT?d14T?j{=N3lVKqqJ8sd8yIbpz{Q-~eS7{SPljllb zmo6bFEI}JgYVo&PPkKE zHgO){gTe=>y-17Zi?$DPI^QE{8b9-nS%XW7H!;-U6BecLvW({Miv1Tv2?y zK}B;{Iuh?1c9HkUibC@B?hDIQ&nxf%uvdySH^)t)X}7gL*}TlZPG4g33;z9SoF}c4 zSUjd))gFItsl=#ciznnB-+u}29X3lBf-5!ZxBxdA6qTJ#t_jBx-$V+w>dka)lr#5_ zbBtX)_b9cX~pScZC#afQ(`hvsj(1%70iO>gr3*osAsd-^s4zmVXczssP3wq9d(@W*7&8VHyS%3(KH(W14KTWMjC{sk+_U+UCvD~-G8{C1yEnQvULf{ z`u`~c{H4frVFmYi0}=B}5fB>G<80tz);bHt#vfTIeiJlqnU1E=yJD?v%*E1+vZKYP zM*CK{LfE)Zj=VGl3+5RdXbf$w5SC(6e)F(zrD#0rH3ajSDd?o5X}C>6tE!qLu9QI` zEi_cI{JrR@D;}P$2l3vz&TcqTQy}0Ov)^ZS!Lfx$Iotnr7ruHb<&|~Cg10IO>t|Q) z-=8Q(E3%QyvzC~bA3ia`COXi2-b|mcO8q3o^;Ku@oi6qutNZ9I79sNFx93fM{hQ){ZJS^FghxZ1qyFIZeT^W6 zex>LilExDPrkgE#d$Wf?Y+*QR!W^-&u9!YeH~<$eXAn#EXk*P3Z0kee#=0ThCCc7* z1`grXbsih=tlea7s$9=PJQ8tr^(cYCZwUEcHRZop< z2tld7F-!68gGfhBTSPS# zzM(E#HJ$)eq`;x|YeC}nY()u#FWbSAX@`Cyt%n1i{kA4B93&R zx;=-FlvNy`o5|{iRap&&yKXNlF1_#?UL7OVA&-+b_M_(-#AaiGRh2R$N5x5FrV zRy6Q+(^|bhmpmVY#XUvL&LkE0+8VgJ}!~JfBPbi&m(YP>8E3L z66%Fw!@VEPI>yz6%Ao4bkJ=tQR^87U;(pwiXk_#ltZ}Wx2k{43jUV9}0OI=q!^wpH ziSCY;@{WPUWcii)5B3iGnkLY!Ea}JOG%&)V+|#uBM7BprHmgu{Uu`Mqu>QkWkI7~@ z0$wxVT-y14=|LaCAabg=lc;JkcJSKU@Gx7gASIlBSHn!U*!oMCUp*_X3z6nzJ}eb> zx~j*$rO&Nd!wJ7oBK&aJHcuj0`V$`)tR@pQ3`djkSs*ga>4%bSx(hzd6W-q!y?@+7 z_D}x_Yy~$HFKHs1T46?wL<@S{;O-9%DBnu)970Wk@DtM({>^V(USxt@4ehgEXw z^Ml%sDb& zrRD9GTtCk2QrOMkoU126#FE6&iQ;>+mA7R=y?ZcJ-yb2I5p zC8OR)tC#8Q6O{VJl?o*x%6Momts&-b1_a4Zh-o<;_37axh zNzoSMJGh-WGP4^?h4jLS(6EpyR3|ze6t!dOsNH0Q@o~7hUaSqI5w;7I5=098S4SvZ ziUkD2N@@#^y+9QC&5NWeCKSfSFxK2gN+A(QD40f+5lGEJLj{6iZe(7~=q%TzH@FVT Z%eH^z{{BuZ2r?fS^-dxwi}RO~{{r3Ku`2)o literal 0 HcmV?d00001 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);