You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "jyothsnakonisa (via GitHub)" <gi...@apache.org> on 2023/08/30 23:18:08 UTC
[GitHub] [cassandra] jyothsnakonisa opened a new pull request, #2652: CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode
jyothsnakonisa opened a new pull request, #2652:
URL: https://github.com/apache/cassandra/pull/2652
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode [cassandra]
Posted by "pmcfadin (via GitHub)" <gi...@apache.org>.
pmcfadin closed pull request #2652: CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode
URL: https://github.com/apache/cassandra/pull/2652
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode [cassandra]
Posted by "jyothsnakonisa (via GitHub)" <gi...@apache.org>.
jyothsnakonisa commented on code in PR #2652:
URL: https://github.com/apache/cassandra/pull/2652#discussion_r1431734568
##########
src/java/org/apache/cassandra/config/EncryptionOptions.java:
##########
@@ -67,6 +69,42 @@ public String description()
}
}
+ public enum ClientAuth
+ {
+ REQUIRED("true"),
+ NOT_REQUIRED("false"),
+ OPTIONAL("optional");
+ private final String value;
+ private static final Map<String, ClientAuth> VALUES = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
Review Comment:
Reverted it back
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode [cassandra]
Posted by "pmcfadin (via GitHub)" <gi...@apache.org>.
pmcfadin commented on PR #2652:
URL: https://github.com/apache/cassandra/pull/2652#issuecomment-4227140675
Closing as [CASSANDRA-18811](https://issues.apache.org/jira/browse/CASSANDRA-18811) has been resolved as Fixed — if this PR contains changes not captured in the merged fix, please reopen.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode [cassandra]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #2652:
URL: https://github.com/apache/cassandra/pull/2652#discussion_r1430914474
##########
src/java/org/apache/cassandra/config/EncryptionOptions.java:
##########
@@ -67,6 +69,42 @@ public String description()
}
}
+ public enum ClientAuth
+ {
+ REQUIRED("true"),
+ NOT_REQUIRED("false"),
+ OPTIONAL("optional");
+ private final String value;
+ private static final Map<String, ClientAuth> VALUES = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
Review Comment:
giving this a second thought. It looks like the values are always lowercased, so I think we should go back to the original HashMap implementation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18811 Set right client auth for creating SSL context in mTLS optional mode [cassandra]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #2652:
URL: https://github.com/apache/cassandra/pull/2652#discussion_r1430796979
##########
src/java/org/apache/cassandra/config/EncryptionOptions.java:
##########
@@ -67,6 +68,42 @@ public String description()
}
}
+ public enum ClientAuth
+ {
+ REQUIRED("true"),
+ NOT_REQUIRED("false"),
+ OPTIONAL("optional");
+ private final String value;
+ private static Map<String, ClientAuth> values = new HashMap<>();
+ static
+ {
+ for(final ClientAuth clientAuth: ClientAuth.values())
+ {
+ values.put(clientAuth.value, clientAuth);
+ values.put(clientAuth.name().toLowerCase(), clientAuth);
+ }
+ }
+
+ ClientAuth(String value)
+ {
+ this.value = value;
+ }
+
+ public static ClientAuth from(String value)
+ {
+ if(values.containsKey(value.toLowerCase()))
+ {
+ return values.get(value.toLowerCase());
+ };
Review Comment:
dangling semicolon
```suggestion
}
```
##########
src/java/org/apache/cassandra/config/EncryptionOptions.java:
##########
@@ -67,6 +68,42 @@ public String description()
}
}
+ public enum ClientAuth
+ {
+ REQUIRED("true"),
+ NOT_REQUIRED("false"),
+ OPTIONAL("optional");
+ private final String value;
+ private static Map<String, ClientAuth> values = new HashMap<>();
+ static
+ {
+ for(final ClientAuth clientAuth: ClientAuth.values())
+ {
+ values.put(clientAuth.value, clientAuth);
+ values.put(clientAuth.name().toLowerCase(), clientAuth);
+ }
+ }
+
+ ClientAuth(String value)
+ {
+ this.value = value;
+ }
+
+ public static ClientAuth from(String value)
+ {
+ if(values.containsKey(value.toLowerCase()))
Review Comment:
NIT spacing
```suggestion
if (values.containsKey(value.toLowerCase()))
```
##########
src/java/org/apache/cassandra/config/EncryptionOptions.java:
##########
@@ -67,6 +68,42 @@ public String description()
}
}
+ public enum ClientAuth
+ {
+ REQUIRED("true"),
+ NOT_REQUIRED("false"),
+ OPTIONAL("optional");
+ private final String value;
+ private static Map<String, ClientAuth> values = new HashMap<>();
+ static
+ {
+ for(final ClientAuth clientAuth: ClientAuth.values())
Review Comment:
NIT: Usage of final is discouraged in C* code style
```suggestion
for (ClientAuth clientAuth : ClientAuth.values())
```
##########
src/java/org/apache/cassandra/config/EncryptionOptions.java:
##########
@@ -67,6 +68,42 @@ public String description()
}
}
+ public enum ClientAuth
+ {
+ REQUIRED("true"),
+ NOT_REQUIRED("false"),
+ OPTIONAL("optional");
+ private final String value;
+ private static Map<String, ClientAuth> values = new HashMap<>();
Review Comment:
I would prefer a case insensitive treemap here, given that we are dealing with user input, so both "Required" and "REQUIRED" would be valid configuration options. Also, I think it is preferred to use final and I would uppercase this field to denote it is a static final field
```suggestion
private static final Map<String, ClientAuth> VALUES = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
```
##########
src/java/org/apache/cassandra/auth/MutualTlsWithPasswordFallbackAuthenticator.java:
##########
@@ -50,4 +55,15 @@ public SaslNegotiator newSaslNegotiator(InetAddress clientAddress, Certificate[]
}
return mutualTlsAuthenticator.newSaslNegotiator(clientAddress, certificates);
}
+
+ @Override
+ public void validateConfiguration() throws ConfigurationException
+ {
+ Config config = DatabaseDescriptor.getRawConfig();
+ if(config.client_encryption_options.getClientAuth() == EncryptionOptions.ClientAuth.NOT_REQUIRED)
Review Comment:
NIT:
```suggestion
if (config.client_encryption_options.getClientAuth() == EncryptionOptions.ClientAuth.NOT_REQUIRED)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org