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 07:21:52 UTC

[GitHub] [james-project] andrevka opened a new pull request #307: James 3502, add ssl support

andrevka opened a new pull request #307:
URL: https://github.com/apache/james-project/pull/307


   https://issues.apache.org/jira/browse/JAMES-3502?filter=-2


----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582740183



##########
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:
       removed, applied checkstyle




----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582706344



##########
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:
       fixed https://github.com/apache/james-project/pull/307/commits/3c0c552a7fe1918e07d9fe305ede2d1552e95813

##########
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:
       fixed https://github.com/apache/james-project/pull/307/commits/3c0c552a7fe1918e07d9fe305ede2d1552e95813




----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582740002



##########
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:
       https://github.com/apache/james-project/pull/307/commits/62bd3eedb8553408878ee74e59425ae7e1dabee5




----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582652137



##########
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:
       client certificate authentication?

##########
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:
       client certificate authentication?




----------------------------------------------------------------
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


[GitHub] [james-project] chibenwa commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582654819



##########
File path: backends-common/rabbitmq/pom.xml
##########
@@ -93,6 +93,16 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-pool2</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <version>4.5.13</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpcore</artifactId>
+            <version>4.4.5</version>
+        </dependency>

Review comment:
       Can you quickly explain why we need these dependencies? At first glance it seems unrelated to AMQP...




----------------------------------------------------------------
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


[GitHub] [james-project] Arsnael commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r584437284



##########
File path: backends-common/rabbitmq/pom.xml
##########
@@ -93,6 +93,16 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-pool2</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <version>4.5.13</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpcore</artifactId>
+            <version>4.4.5</version>

Review comment:
       I can see we have an httpcore artifact declared in `james-server-jmap` module as well, in a different version (4.4.14). Would be good to try centralizing the use of the artifact in the root pom and bump it here for rabbitmq maybe?




----------------------------------------------------------------
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


[GitHub] [james-project] Arsnael commented on pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #307:
URL: https://github.com/apache/james-project/pull/307#issuecomment-789492413


   Merged


----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582656420



##########
File path: backends-common/rabbitmq/pom.xml
##########
@@ -93,6 +93,16 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-pool2</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <version>4.5.13</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpcore</artifactId>
+            <version>4.4.5</version>
+        </dependency>

Review comment:
       httpcore, has useful class SSLContextBuilder
   httpclient, is needed for configuring mangement api client for org.apache.http.conn.ssl.DefaultHostnameVerifier




----------------------------------------------------------------
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


[GitHub] [james-project] chibenwa commented on pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #307:
URL: https://github.com/apache/james-project/pull/307#issuecomment-787570537


   Would other contributors like to have a look at this work?
   
   It is :green_apple: .


----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582694019



##########
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:
       SUBJECT_ALTERNATIVE_NAME_VERIFIER ?




----------------------------------------------------------------
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


[GitHub] [james-project] andrevka commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582694019



##########
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:
       what do you think of SUBJECT_ALTERNATIVE_NAME_VERIFIER ?




----------------------------------------------------------------
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


[GitHub] [james-project] chibenwa commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582656089



##########
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:
       Sorry, I missread. That's ok.




----------------------------------------------------------------
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


[GitHub] [james-project] chibenwa commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
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


[GitHub] [james-project] Arsnael closed pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
Arsnael closed pull request #307:
URL: https://github.com/apache/james-project/pull/307


   


----------------------------------------------------------------
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


[GitHub] [james-project] Arsnael commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r584433891



##########
File path: backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/RabbitMQConfiguration.java
##########
@@ -197,11 +471,16 @@ public static RabbitMQConfiguration from(Configuration configuration) {
         Preconditions.checkState(!Strings.isNullOrEmpty(managementUriAsString), "You need to specify the management URI of RabbitMQ");
         URI managementUri = checkURI(managementUriAsString);
 
+        Boolean useSsl = configuration.getBoolean(USE_SSL, false);
+
         ManagementCredentials managementCredentials = ManagementCredentials.from(configuration);
         return builder()
             .amqpUri(amqpUri)
             .managementUri(managementUri)
             .managementCredentials(managementCredentials)
+            .useSsl(useSsl)
+            .sslConfiguration(sslTrustConfiguration(configuration))
+            .sslConfiguration(sslTrustConfiguration(configuration))

Review comment:
       duplicated line here

##########
File path: backends-common/rabbitmq/pom.xml
##########
@@ -93,6 +93,16 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-pool2</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <version>4.5.13</version>

Review comment:
       We already have that artifact with the same version declared in the root pom file, so you can omit the version here

##########
File path: backends-common/rabbitmq/pom.xml
##########
@@ -93,6 +93,16 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-pool2</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <version>4.5.13</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpcore</artifactId>
+            <version>4.4.5</version>

Review comment:
       I can see we have an httpcore artifact declared in `james-server-jmap` module as well, in a different version (4.4.14). Would be good to try centralizing the use of the artifact in the root pom and bump it for jmap




----------------------------------------------------------------
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


[GitHub] [james-project] chibenwa commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582655740



##########
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:
       Sorry, I missread.




----------------------------------------------------------------
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


[GitHub] [james-project] chibenwa commented on a change in pull request #307: James 3502, add ssl support

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #307:
URL: https://github.com/apache/james-project/pull/307#discussion_r582655740



##########
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:
       Sorry, I missread. That's ok.




----------------------------------------------------------------
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