You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/02/25 08:54:55 UTC
[GitHub] [james-project] chibenwa commented on a change in pull request #307: James 3502, add ssl support
chibenwa commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582646724
##########
File path: backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/RabbitMQConfiguration.java
##########
@@ -18,18 +18,278 @@
****************************************************************/
package org.apache.james.backends.rabbitmq;
+import java.io.File;
import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
+import java.util.stream.Stream;
import org.apache.commons.configuration2.Configuration;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
+import static org.apache.james.backends.rabbitmq.RabbitMQConfiguration.SSLConfiguration.*;
+import static org.apache.james.backends.rabbitmq.RabbitMQConfiguration.SSLConfiguration.SSLValidationStrategy.*;
+
public class RabbitMQConfiguration {
+ public static class SSLConfiguration {
+
+ public enum SSLValidationStrategy {
+ DEFAULT,
+ IGNORE,
+ OVERRIDE;
+
+ static SSLValidationStrategy from(String rawValue) {
+ Preconditions.checkNotNull(rawValue);
+
+ return Stream.of(values())
+ .filter(strategy -> strategy.name().equalsIgnoreCase(rawValue))
+ .findAny()
+ .orElseThrow(() -> new IllegalArgumentException(String.format("invalid strategy '%s'", rawValue)));
+
+ }
+ }
+
+ public enum HostNameVerifier {
+ DEFAULT,
Review comment:
DEFAUT does not describe a behaviour. Can we find terms to express what this does?
##########
File path: backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/RabbitMQConfiguration.java
##########
@@ -18,18 +18,278 @@
****************************************************************/
package org.apache.james.backends.rabbitmq;
+import java.io.File;
import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
+import java.util.stream.Stream;
import org.apache.commons.configuration2.Configuration;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
+import static org.apache.james.backends.rabbitmq.RabbitMQConfiguration.SSLConfiguration.*;
+import static org.apache.james.backends.rabbitmq.RabbitMQConfiguration.SSLConfiguration.SSLValidationStrategy.*;
+
public class RabbitMQConfiguration {
+ public static class SSLConfiguration {
+
+ public enum SSLValidationStrategy {
+ DEFAULT,
+ IGNORE,
+ OVERRIDE;
+
+ static SSLValidationStrategy from(String rawValue) {
+ Preconditions.checkNotNull(rawValue);
+
+ return Stream.of(values())
+ .filter(strategy -> strategy.name().equalsIgnoreCase(rawValue))
+ .findAny()
+ .orElseThrow(() -> new IllegalArgumentException(String.format("invalid strategy '%s'", rawValue)));
+
+ }
+ }
+
+ public enum HostNameVerifier {
+ DEFAULT,
+ ACCEPT_ANY_HOSTNAME;
+
+ static HostNameVerifier from(String rawValue) {
+ Preconditions.checkNotNull(rawValue);
+
+ return Stream.of(values())
+ .filter(verifier -> verifier.name().equalsIgnoreCase(rawValue))
+ .findAny()
+ .orElseThrow(() -> new IllegalArgumentException(String.format("invalid HostNameVerifier '%s'", rawValue)));
+
+ }
+ }
+
+ public static class SSLKeyStore {
+
+ public static SSLKeyStore of(String filePath, String password) {
+ return new SSLKeyStore(filePath, password);
+ }
+
+ private final File file;
+ private final char[] password;
+
+ private SSLKeyStore(String filePath, String password) {
+ Preconditions.checkNotNull(filePath, "%s cannot be null when %s is specified",
+ SSL_KEY_STORE_PATH, SSL_KEY_STORE_PASSWORD);
+ Preconditions.checkNotNull(password,
+ "%s cannot be null when %s is specified",
+ SSL_KEY_STORE_PASSWORD, SSL_KEY_STORE_PATH);
+ Preconditions.checkArgument(Files.exists(Paths.get(filePath)),
+ "the file '%s' from property '%s' doesn't exist", filePath, SSL_KEY_STORE_PATH);
+
+ this.file = new File(filePath);
+ this.password = password.toCharArray();
+ }
+
+ public File getFile() {
+ return file;
+ }
+
+ public char[] getPassword() {
+ return password;
+ }
+
+ @Override
+ public final boolean equals(Object o) {
+ if (o instanceof SSLKeyStore) {
+ SSLKeyStore that = (SSLKeyStore) o;
+
+ return Objects.equals(this.file, that.file)
+ && Arrays.equals(this.password, that.password);
+ }
+ return false;
+ }
+
+ @Override
+ public final int hashCode() {
+ return Objects.hash(file, Arrays.hashCode(password));
+ }
+ }
+
+
+ public static class SSLTrustStore {
+
+ public static SSLTrustStore of(String filePath, String password) {
+ return new SSLTrustStore(filePath, password);
+ }
+
+ private final File file;
+ private final char[] password;
+
+ private SSLTrustStore(String filePath, String password) {
+ Preconditions.checkNotNull(filePath, "%s cannot be null when %s is specified",
+ SSL_TRUST_STORE_PATH, SSL_TRUST_STORE_PASSWORD);
+ Preconditions.checkNotNull(password,
+ "%s cannot be null when %s is specified",
+ SSL_TRUST_STORE_PASSWORD, SSL_TRUST_STORE_PATH);
+ Preconditions.checkArgument(Files.exists(Paths.get(filePath)),
+ "the file '%s' from property '%s' doesn't exist", filePath, SSL_TRUST_STORE_PATH);
+
+ this.file = new File(filePath);
+ this.password = password.toCharArray();
+ }
+
+ public File getFile() {
+ return file;
+ }
+
+ public char[] getPassword() {
+ return password;
+ }
+
+ @Override
+ public final boolean equals(Object o) {
+ if (o instanceof SSLTrustStore) {
+ SSLTrustStore that = (SSLTrustStore) o;
+
+ return Objects.equals(this.file, that.file)
+ && Arrays.equals(this.password, that.password);
+ }
+ return false;
+ }
+
+ @Override
+ public final int hashCode() {
+ return Objects.hash(file, Arrays.hashCode(password));
+ }
+ }
+
+ static class Builder {
+
+ interface RequireSSLStrategyTrustStore {
Review comment:
Use `@FunctionalInterface` annotation where relevant.
##########
File path: docs/modules/servers/pages/distributed/configure/rabbitmq.adoc
##########
@@ -46,6 +46,39 @@ Optional integer, defaults to 50
| Configure the size of the channel pool.
Optional integer, defaults to 3
+
+
+| ssl.enabled
+| Is using ssl enabled
+Optional boolean, defaults to false
+
+| ssl.management.enabled
+| Is using ssl on management api enabled
+Optional boolean, defaults to false
+
+| ssl.validation.strategy
+| Configure the validation strategy used for rabbitmq connections. Possible values are default, ignore and override.
+Optional string, defaults to default
+
+| ssl.truststore
+| Points to the truststore (PKCS12) used for verifying rabbitmq connection. If configured then "ssl.truststore.password" must also be configured,
+Optional string, defaults to systemwide truststore. "ssl.validation.strategy: override" must be configured if you want to use this
+
+| ssl.truststore.password
+| Configure the truststore password. If configured then "ssl.truststore" must also be configured,
+Optional string, defaults to empty string. "ssl.validation.strategy: override" must be configured if you want to use this
+
+| ssl.hostname.verifier
+| Configure host name verification. Possible options are default and accept_any_hostname
+Optional string, defaults to default
+
+| ssl.keystore
+| Points to the keystore(PKCS12) used for verifying rabbitmq connection. If configured then "ssl.keystore.password" must also be configured,
+Optional string, defaults to empty string
+
+| ssl.keystore.password
+| Configure the keystore password. If configured then "ssl.keystore" must also be configured,
+Optional string, defaults to empty string
Review comment:
Is there some cases when it should be specified?
##########
File path: docs/modules/servers/pages/distributed/configure/rabbitmq.adoc
##########
@@ -46,6 +46,39 @@ Optional integer, defaults to 50
| Configure the size of the channel pool.
Optional integer, defaults to 3
+
+
+| ssl.enabled
+| Is using ssl enabled
+Optional boolean, defaults to false
+
+| ssl.management.enabled
+| Is using ssl on management api enabled
+Optional boolean, defaults to false
+
+| ssl.validation.strategy
+| Configure the validation strategy used for rabbitmq connections. Possible values are default, ignore and override.
+Optional string, defaults to default
+
+| ssl.truststore
+| Points to the truststore (PKCS12) used for verifying rabbitmq connection. If configured then "ssl.truststore.password" must also be configured,
+Optional string, defaults to systemwide truststore. "ssl.validation.strategy: override" must be configured if you want to use this
+
+| ssl.truststore.password
+| Configure the truststore password. If configured then "ssl.truststore" must also be configured,
+Optional string, defaults to empty string. "ssl.validation.strategy: override" must be configured if you want to use this
+
+| ssl.hostname.verifier
+| Configure host name verification. Possible options are default and accept_any_hostname
+Optional string, defaults to default
+
+| ssl.keystore
+| Points to the keystore(PKCS12) used for verifying rabbitmq connection. If configured then "ssl.keystore.password" must also be configured,
+Optional string, defaults to empty string
Review comment:
Is there some cases when it should be specified?
##########
File path: backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/RabbitMQConnectionFactory.java
##########
@@ -54,18 +69,85 @@ private ConnectionFactory from(RabbitMQConfiguration rabbitMQConfiguration) {
connectionFactory.setUsername(rabbitMQConfiguration.getManagementCredentials().getUser());
connectionFactory.setPassword(String.valueOf(rabbitMQConfiguration.getManagementCredentials().getPassword()));
+ if (configuration.useSsl()) setupSslConfiguration(connectionFactory);
Review comment:
You miss a `{}` block after this if, the code style will not like it ;-)
##########
File path: docs/modules/servers/pages/distributed/configure/rabbitmq.adoc
##########
@@ -46,6 +46,39 @@ Optional integer, defaults to 50
| Configure the size of the channel pool.
Optional integer, defaults to 3
+
+
+| ssl.enabled
+| Is using ssl enabled
+Optional boolean, defaults to false
+
+| ssl.management.enabled
+| Is using ssl on management api enabled
+Optional boolean, defaults to false
+
+| ssl.validation.strategy
+| Configure the validation strategy used for rabbitmq connections. Possible values are default, ignore and override.
+Optional string, defaults to default
Review comment:
What does default do?
##########
File path: backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/RabbitMQConfiguration.java
##########
@@ -18,18 +18,278 @@
****************************************************************/
package org.apache.james.backends.rabbitmq;
+import java.io.File;
import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
+import java.util.stream.Stream;
import org.apache.commons.configuration2.Configuration;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
+import static org.apache.james.backends.rabbitmq.RabbitMQConfiguration.SSLConfiguration.*;
+import static org.apache.james.backends.rabbitmq.RabbitMQConfiguration.SSLConfiguration.SSLValidationStrategy.*;
Review comment:
Please avoid wildcards, this will likely not pass the checkstyle.
##########
File path: docs/modules/servers/pages/distributed/configure/rabbitmq.adoc
##########
@@ -46,6 +46,39 @@ Optional integer, defaults to 50
| Configure the size of the channel pool.
Optional integer, defaults to 3
+
+
+| ssl.enabled
+| Is using ssl enabled
+Optional boolean, defaults to false
+
+| ssl.management.enabled
+| Is using ssl on management api enabled
+Optional boolean, defaults to false
+
+| ssl.validation.strategy
+| Configure the validation strategy used for rabbitmq connections. Possible values are default, ignore and override.
+Optional string, defaults to default
+
+| ssl.truststore
+| Points to the truststore (PKCS12) used for verifying rabbitmq connection. If configured then "ssl.truststore.password" must also be configured,
+Optional string, defaults to systemwide truststore. "ssl.validation.strategy: override" must be configured if you want to use this
+
+| ssl.truststore.password
+| Configure the truststore password. If configured then "ssl.truststore" must also be configured,
+Optional string, defaults to empty string. "ssl.validation.strategy: override" must be configured if you want to use this
+
+| ssl.hostname.verifier
+| Configure host name verification. Possible options are default and accept_any_hostname
+Optional string, defaults to default
Review comment:
What does default do?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org