CASSANDRA-18428: Adding equals/hashCode override for the ServerEncryptionOptions #2269

maulin-vasavada · 2023-04-11T00:11:14Z

Thanks for sending a pull request! Here are some tips if you're new here:

Ensure you have added or run the appropriate tests for your PR.
Be sure to keep the PR description updated to reflect all changes.
Write your PR title to summarize what this PR proposes.
If possible, provide a concise example to reproduce the issue for a faster review.
Read our contributor guidelines
If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

Mmuzaf · 2023-07-20T18:09:29Z

test/unit/org/apache/cassandra/config/EncryptionOptionsEqualityTest.java

@@ -139,4 +156,47 @@ public void testDifferentCustomSslContextFactoryParameters() {
        assertNotEquals(encryptionOptions1, encryptionOptions2);
        assertNotEquals(encryptionOptions1.hashCode(), encryptionOptions2.hashCode());
    }
+
+    @Test
+    public void testServerEncryptionOptions() {


nl for the left curly bracket.

Mmuzaf · 2023-07-20T18:09:42Z

test/unit/org/apache/cassandra/config/EncryptionOptionsEqualityTest.java

+    @Test
+    public void testServerEncryptionOptions() {
+        EncryptionOptions.ServerEncryptionOptions encryptionOptions1 = createServerEncryptionOptions();
+        EncryptionOptions.ServerEncryptionOptions encryptionOptions2 =createServerEncryptionOptions();


Suggested change

EncryptionOptions.ServerEncryptionOptions encryptionOptions2 =createServerEncryptionOptions();

EncryptionOptions.ServerEncryptionOptions encryptionOptions2 = createServerEncryptionOptions();

Mmuzaf · 2023-07-20T18:10:03Z

test/unit/org/apache/cassandra/config/EncryptionOptionsEqualityTest.java

+    }
+
+    @Test
+    public void testServerEncryptionOptionsMismatchForOutboundKeystore() {


nl for the left curly bracket.

Mmuzaf · 2023-07-20T18:10:08Z

test/unit/org/apache/cassandra/config/EncryptionOptionsEqualityTest.java

+    }
+
+    @Test
+    public void testServerEncryptionOptionsMismatchForInboundKeystore() {


nl for the left curly bracket.

Mmuzaf · 2023-07-20T18:11:06Z

src/java/org/apache/cassandra/config/EncryptionOptions.java

+            result += 31 * (outbound_keystore == null ? 0 : outbound_keystore.hashCode());
+            result += 31 * (outbound_keystore_password == null ? 0 : outbound_keystore_password.hashCode());
+            return result;
+        }


newline between methods.

Mmuzaf · 2023-07-20T18:11:36Z

src/java/org/apache/cassandra/config/EncryptionOptions.java

+                return true;
+            if (o == null || getClass() != o.getClass())
+                return false;
+            if (!super.equals(o)) {


I would remove the brackets at all here :-)

Mmuzaf · 2023-07-20T18:15:15Z

src/java/org/apache/cassandra/config/EncryptionOptions.java

@@ -627,8 +627,8 @@ public ServerEncryptionOptions(ParameterizedClass sslContextFactoryClass, String
                                       InternodeEncryption internode_encryption, boolean legacy_ssl_storage_port_enabled)
        {
            super(sslContextFactoryClass, keystore, keystore_password, truststore, truststore_password, cipher_suites,
-            protocol, accepted_protocols, algorithm, store_type, require_client_auth, require_endpoint_verification,
-            null, optional);
+                  protocol, accepted_protocols, algorithm, store_type, require_client_auth, require_endpoint_verification,


I would exclude this from the PR as the formatting has nothing to do with the patch itself. Just not to avoid drawing attention to it in simple patches.

Mmuzaf · 2023-07-20T18:16:33Z

src/java/org/apache/cassandra/config/EncryptionOptions.java

-                          + " its broadcast address in the handshake and bypass authentication. To ensure that mutual TLS"
-                          + " authentication is not bypassed, please set internode_encryption to 'all'. Continuing with"
-                          + " insecure configuration.");
+                            + " It is possible for an internode connection to pretend to be in the same rack/dc by spoofing"


The same as above - as the formatting is not related to the change itself it is worth excluding it from the PR to avoid drawing attention to these lines.

Mmuzaf · 2023-07-20T18:19:42Z

The code-style is here, just for reference:
https://cassandra.apache.org/_/development/code_style.html

Mmuzaf · 2023-07-20T18:23:04Z

test/unit/org/apache/cassandra/config/EncryptionOptionsEqualityTest.java

@@ -35,6 +35,23 @@
 */
 public class EncryptionOptionsEqualityTest
 {
+    private EncryptionOptions.ServerEncryptionOptions createServerEncryptionOptions()
+    {
+        EncryptionOptions.ServerEncryptionOptions encryptionOptions =


You can inline encryptionOptions for simplicity.

maulin-vasavada · 2023-07-20T20:34:23Z

Thanks @Mmuzaf for the review. I'll take a look at those soon.

maulin-vasavada · 2023-07-22T00:14:10Z

@Mmuzaf I have opened a new PR#2507 hence will close this. Can you please review that new PR? Thanks

Adding equals/hashCode override for the ServerEncryptionOptions

63d7283

Mmuzaf reviewed Jul 20, 2023

View reviewed changes

maulin-vasavada closed this Jul 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CASSANDRA-18428: Adding equals/hashCode override for the ServerEncryptionOptions #2269

CASSANDRA-18428: Adding equals/hashCode override for the ServerEncryptionOptions #2269

maulin-vasavada commented Apr 11, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf Jul 20, 2023

Mmuzaf commented Jul 20, 2023

Mmuzaf Jul 20, 2023

maulin-vasavada commented Jul 20, 2023

maulin-vasavada commented Jul 22, 2023

	EncryptionOptions.ServerEncryptionOptions encryptionOptions2 =createServerEncryptionOptions();
	EncryptionOptions.ServerEncryptionOptions encryptionOptions2 = createServerEncryptionOptions();

CASSANDRA-18428: Adding equals/hashCode override for the ServerEncryptionOptions #2269

CASSANDRA-18428: Adding equals/hashCode override for the ServerEncryptionOptions #2269

Conversation

maulin-vasavada commented Apr 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mmuzaf commented Jul 20, 2023

Choose a reason for hiding this comment

maulin-vasavada commented Jul 20, 2023

maulin-vasavada commented Jul 22, 2023