You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/07/22 10:32:32 UTC

[GitHub] [activemq-artemis] brusdev opened a new pull request #3667: ARTEMIS-3367 Set verifyHost true by default

brusdev opened a new pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667


   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682340204



##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java
##########
@@ -82,60 +85,18 @@ public CoreClientOverOneWaySSLTest(String storeProvider, String storeType, boole
       if (suffix.equalsIgnoreCase("PKCS12")) {
          suffix = "p12";
       }
-      SERVER_SIDE_KEYSTORE = "server-side-keystore." + suffix;
-      CLIENT_SIDE_TRUSTSTORE = "client-side-truststore." + suffix;
+      SERVER_SIDE_KEYSTORE = "server-keystore." + suffix;
+      CLIENT_SIDE_TRUSTSTORE = "server-ca-truststore." + suffix;

Review comment:
       I see, this could not be obvious but it should clarify what the test is doing:
   The `SERVER_SIDE_KEYSTORE` points to a keystore with the server key.
   The `CLIENT_SIDE_TRUSTSTORE` points to a truststore with the server ca certificate.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r675539301



##########
File path: .project
##########
@@ -14,4 +14,15 @@
 	<natures>
 		<nature>org.eclipse.m2e.core.maven2Nature</nature>
 	</natures>
+	<filteredResources>

Review comment:
       no, it wasn't on purpose. I'll remove it, thanks




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682340722



##########
File path: tests/security-resources/build.sh
##########
@@ -0,0 +1,156 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# The various SSL stores and certificates were created with the following commands:
+# Requires use of JDK 8+ keytool command.
+set -e
+
+KEY_PASS=securepass
+STORE_PASS=securepass
+CA_VALIDITY=365000
+VALIDITY=36500
+
+# Clean up existing files
+# -----------------------
+rm -f *.crt *.csr openssl-* *.jceks *.jks *.p12 *.pem
+
+# Create a key and self-signed certificate for the CA, to sign server certificate requests and use for trust:
+# ----------------------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias server-ca -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Server Certification Authority, OU=Artemis, O=ActiveMQ" -validity $CA_VALIDITY -ext bc:c=ca:true
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -exportcert -rfc > server-ca.crt
+openssl pkcs12 -in server-ca-keystore.p12 -nodes -nocerts -out server-ca.pem -password pass:$STORE_PASS
+
+# Create trust store with the server CA cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-ca-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -importkeystore -srckeystore server-ca-truststore.p12 -destkeystore server-ca-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore server-ca-truststore.p12 -destkeystore server-ca-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the server, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=sA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -alias server -certreq -file server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile server.csr -outfile server.crt -validity $VALIDITY -ext bc=ca:false -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server -file server.crt
+
+keytool -importkeystore -srckeystore server-keystore.p12 -destkeystore server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore server-keystore.p12 -destkeystore server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the other server, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias other-server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Other Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -alias other-server -certreq -file other-server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile other-server.csr -outfile other-server.crt -validity $VALIDITY -ext bc=ca:false -ext eku=sA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-server -file other-server.crt
+
+keytool -importkeystore -srckeystore other-server-keystore.p12 -destkeystore other-server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-server-keystore.p12 -destkeystore other-server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create trust store with the other server cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-server-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-server -file other-server.crt -noprompt
+keytool -importkeystore -srckeystore other-server-truststore.p12 -destkeystore other-server-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-server-truststore.p12 -destkeystore other-server-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create crl with the other server cert:
+# -------------------------------------------------------
+> openssl-database
+echo 00 > openssl-crlnumber
+openssl ca -config openssl.conf -revoke other-server.crt -keyfile server-ca.pem -cert server-ca.crt
+openssl ca -config openssl.conf -gencrl -keyfile server-ca.pem -cert server-ca.crt -out other-server-crl.pem -crldays $VALIDITY
+
+# Create a key pair for the broker with an unexpected hostname, and sign it with the CA:
+# --------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias unknown-server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Unknown Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=sA
+
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -alias unknown-server -certreq -file unknown-server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile unknown-server.csr -outfile unknown-server.crt -validity $VALIDITY -ext bc=ca:false -ext eku=sA
+
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias unknown-server -file unknown-server.crt
+
+keytool -importkeystore -srckeystore unknown-server-keystore.p12 -destkeystore unknown-server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore unknown-server-keystore.p12 -destkeystore unknown-server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key and self-signed certificate for the CA, to sign client certificate requests and use for trust:
+# ----------------------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias client-ca -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Client Certification Authority, OU=Artemis, O=ActiveMQ" -validity $CA_VALIDITY -ext bc:c=ca:true
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -exportcert -rfc > client-ca.crt
+openssl pkcs12 -in client-ca-keystore.p12 -nodes -nocerts -out client-ca.pem -password pass:$STORE_PASS
+
+# Create trust store with the client CA cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-ca-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -importkeystore -srckeystore client-ca-truststore.p12 -destkeystore client-ca-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore client-ca-truststore.p12 -destkeystore client-ca-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the client, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias client -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -alias client -certreq -file client.csr
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -gencert -rfc -infile client.csr -outfile client.crt -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client -file client.crt
+
+keytool -importkeystore -srckeystore client-keystore.p12 -destkeystore client-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore client-keystore.p12 -destkeystore client-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the other client, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias other-client -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Other Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -alias other-client -certreq -file other-client.csr
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -gencert -rfc -infile other-client.csr -outfile other-client.crt -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-client -file other-client.crt
+
+keytool -importkeystore -srckeystore other-client-keystore.p12 -destkeystore other-client-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-client-keystore.p12 -destkeystore other-client-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass

Review comment:
       The `CoreClientOverOneWaySSLTest` and `CoreClientOverTwoWaySSLTest` tests use most of them because they are parameterized tests. ATM only 2 truststores for the `other-client` and 2 truststores for the `unknown-client` are not used but we might need them in the future if they are used in a parametric test.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r681978852



##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpFailoverEndpointDiscoveryTest.java
##########
@@ -90,9 +93,9 @@ private JmsConnectionFactory getJmsConnectionFactory() {
       if (protocol == 0) {
          return new JmsConnectionFactory("failover:(amqp://localhost:61616)");
       } else {
-         String keystore = this.getClass().getClassLoader().getResource("client-side-keystore.jks").getFile();
-         String truststore = this.getClass().getClassLoader().getResource("client-side-truststore.jks").getFile();
-         return new JmsConnectionFactory("failover:(amqps://localhost:61616?transport.keyStoreLocation=" + keystore + "&transport.keyStorePassword=secureexample&transport.trustStoreLocation=" + truststore + "&transport.trustStorePassword=secureexample&transport.verifyHost=false)");
+         String keystore = this.getClass().getClassLoader().getResource("examples/features/standard/jmx-ssl/src/main/resources/activemq/server0/client-keystore.jks").getFile();
+         String truststore = this.getClass().getClassLoader().getResource("examples/features/standard/jmx-ssl/src/main/resources/activemq/server0/server-ca-truststore.jks").getFile();

Review comment:
       Why not reference the resources in `tests/security-resources` like the comment at the beginning of the class suggests? Are the example resources unique in some way?




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682339565



##########
File path: examples/features/standard/jmx-ssl/readme.md
##########
@@ -16,12 +16,54 @@ With these properties, ActiveMQ Artemis broker will be manageable remotely using
 
 The various keystore files are generated using the following commands:
 
-* `keytool -genkey -keystore server-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore server-side-keystore.jks -file server-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore client-side-truststore.jks -file server-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
-* `keytool -genkey -keystore client-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore client-side-keystore.jks -file client-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore server-side-truststore.jks -file client-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
+```shell
+#!/bin/bash
+set -e
+
+KEY_PASS=securepass
+STORE_PASS=securepass
+CA_VALIDITY=365000
+VALIDITY=36500

Review comment:
       I used a multiple of 365 because it is the number of **days in a year**, this was supposed to make it easier to evaluate how many years is the validity of the certificates.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r675536697



##########
File path: .project
##########
@@ -14,4 +14,15 @@
 	<natures>
 		<nature>org.eclipse.m2e.core.maven2Nature</nature>
 	</natures>
+	<filteredResources>

Review comment:
       why did you add this? was this on purpose?




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r681925548



##########
File path: examples/features/standard/jmx-ssl/readme.md
##########
@@ -16,12 +16,54 @@ With these properties, ActiveMQ Artemis broker will be manageable remotely using
 
 The various keystore files are generated using the following commands:
 
-* `keytool -genkey -keystore server-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore server-side-keystore.jks -file server-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore client-side-truststore.jks -file server-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
-* `keytool -genkey -keystore client-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore client-side-keystore.jks -file client-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore server-side-truststore.jks -file client-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
+```shell

Review comment:
       This is a bit verbose for an example readme, where creating these files either isnt the interesting bit and can be 'hidden away', or is the interesting bit and then having it on its own would be good to isolate it out. 
   
   I would create another file or script and reference it, indicate how to run the bits. Can be easier to run later if needing to regenerate things. E.g see https://github.com/apache/activemq-artemis/blob/2.17.0/examples/features/broker-connection/amqp-sending-overssl/readme.md
   
   
   Same would apply to other examples that follow (even if they had a smaller number of such commands in the readme already, but now have more).

##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java
##########
@@ -82,60 +85,18 @@ public CoreClientOverOneWaySSLTest(String storeProvider, String storeType, boole
       if (suffix.equalsIgnoreCase("PKCS12")) {
          suffix = "p12";
       }
-      SERVER_SIDE_KEYSTORE = "server-side-keystore." + suffix;
-      CLIENT_SIDE_TRUSTSTORE = "client-side-truststore." + suffix;
+      SERVER_SIDE_KEYSTORE = "server-keystore." + suffix;
+      CLIENT_SIDE_TRUSTSTORE = "server-ca-truststore." + suffix;

Review comment:
       Related to earlier comment, you then get somewhat less obvious things like this seemingly (but not actually) contradictory constant definition.

##########
File path: tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/netty/NettyConnectorTest.java
##########
@@ -56,10 +59,10 @@ public void setUp() throws Exception {
       Map<String, Object> params = new HashMap<>();
       params.put(TransportConstants.SSL_ENABLED_PROP_NAME, true);
       params.put(TransportConstants.SSL_PROVIDER, TransportConstants.OPENSSL_PROVIDER);
-      params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "openssl-server-side-keystore.jks");
-      params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "secureexample");
-      params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME,  "openssl-server-side-truststore.jks");
-      params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "secureexample");
+      params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "server-keystore.jks");
+      params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "securepass");
+      params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME,  "client-ca-truststore.jks");
+      params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "securepass");

Review comment:
       I'm not the biggest fan of the naming reversal that happened here.
   
   For the keystores, the 'client' and 'server' bits of the name still somewhat denotes where it is being used (as the old 'server side' desginator did before), and additionally what it contains. For the trust stores however, the 'client CA' and 'server CA' bits now denote only what it contains rather than where it is used (which the old 'client-side' designator did), with it then used in the 'opposite' place. This is a bit awkward, you get a bit of cognitive dissonance where it seems wrong to use the 'server keystore' and the 'client...truststore' files together on the server, and the 'client keystore' and 'server...truststore' files together on the client.
   

##########
File path: examples/features/standard/jmx-ssl/readme.md
##########
@@ -16,12 +16,54 @@ With these properties, ActiveMQ Artemis broker will be manageable remotely using
 
 The various keystore files are generated using the following commands:
 
-* `keytool -genkey -keystore server-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore server-side-keystore.jks -file server-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore client-side-truststore.jks -file server-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
-* `keytool -genkey -keystore client-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore client-side-keystore.jks -file client-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore server-side-truststore.jks -file client-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
+```shell
+#!/bin/bash
+set -e
+
+KEY_PASS=securepass
+STORE_PASS=securepass
+CA_VALIDITY=365000
+VALIDITY=36500

Review comment:
       Not sure there need to be 900 years difference between these two validities hehe. Validity is in days, so smaller values would seem doable and closer to 'representative use'. I use 9999 when I want to mean 'so long these wont expire before they are definitely not going to be used anymore'. For any such long value the algorithms will still be disabled and make us replace them before they expire.
   
   Using such a massive value, based around multiple of 365, kind of suggests it isnt in days but something else.

##########
File path: tests/security-resources/build.sh
##########
@@ -0,0 +1,156 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# The various SSL stores and certificates were created with the following commands:
+# Requires use of JDK 8+ keytool command.
+set -e
+
+KEY_PASS=securepass
+STORE_PASS=securepass
+CA_VALIDITY=365000
+VALIDITY=36500
+
+# Clean up existing files
+# -----------------------
+rm -f *.crt *.csr openssl-* *.jceks *.jks *.p12 *.pem
+
+# Create a key and self-signed certificate for the CA, to sign server certificate requests and use for trust:
+# ----------------------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias server-ca -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Server Certification Authority, OU=Artemis, O=ActiveMQ" -validity $CA_VALIDITY -ext bc:c=ca:true
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -exportcert -rfc > server-ca.crt
+openssl pkcs12 -in server-ca-keystore.p12 -nodes -nocerts -out server-ca.pem -password pass:$STORE_PASS
+
+# Create trust store with the server CA cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-ca-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -importkeystore -srckeystore server-ca-truststore.p12 -destkeystore server-ca-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore server-ca-truststore.p12 -destkeystore server-ca-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the server, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=sA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -alias server -certreq -file server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile server.csr -outfile server.crt -validity $VALIDITY -ext bc=ca:false -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server -file server.crt
+
+keytool -importkeystore -srckeystore server-keystore.p12 -destkeystore server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore server-keystore.p12 -destkeystore server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the other server, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias other-server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Other Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -alias other-server -certreq -file other-server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile other-server.csr -outfile other-server.crt -validity $VALIDITY -ext bc=ca:false -ext eku=sA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-server -file other-server.crt
+
+keytool -importkeystore -srckeystore other-server-keystore.p12 -destkeystore other-server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-server-keystore.p12 -destkeystore other-server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create trust store with the other server cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-server-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-server -file other-server.crt -noprompt
+keytool -importkeystore -srckeystore other-server-truststore.p12 -destkeystore other-server-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-server-truststore.p12 -destkeystore other-server-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create crl with the other server cert:
+# -------------------------------------------------------
+> openssl-database
+echo 00 > openssl-crlnumber
+openssl ca -config openssl.conf -revoke other-server.crt -keyfile server-ca.pem -cert server-ca.crt
+openssl ca -config openssl.conf -gencrl -keyfile server-ca.pem -cert server-ca.crt -out other-server-crl.pem -crldays $VALIDITY
+
+# Create a key pair for the broker with an unexpected hostname, and sign it with the CA:
+# --------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias unknown-server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Unknown Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=sA
+
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -alias unknown-server -certreq -file unknown-server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile unknown-server.csr -outfile unknown-server.crt -validity $VALIDITY -ext bc=ca:false -ext eku=sA
+
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias unknown-server -file unknown-server.crt
+
+keytool -importkeystore -srckeystore unknown-server-keystore.p12 -destkeystore unknown-server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore unknown-server-keystore.p12 -destkeystore unknown-server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key and self-signed certificate for the CA, to sign client certificate requests and use for trust:
+# ----------------------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias client-ca -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Client Certification Authority, OU=Artemis, O=ActiveMQ" -validity $CA_VALIDITY -ext bc:c=ca:true
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -exportcert -rfc > client-ca.crt
+openssl pkcs12 -in client-ca-keystore.p12 -nodes -nocerts -out client-ca.pem -password pass:$STORE_PASS
+
+# Create trust store with the client CA cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-ca-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -importkeystore -srckeystore client-ca-truststore.p12 -destkeystore client-ca-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore client-ca-truststore.p12 -destkeystore client-ca-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the client, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias client -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -alias client -certreq -file client.csr
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -gencert -rfc -infile client.csr -outfile client.crt -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client -file client.crt
+
+keytool -importkeystore -srckeystore client-keystore.p12 -destkeystore client-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore client-keystore.p12 -destkeystore client-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the other client, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias other-client -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Other Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -alias other-client -certreq -file other-client.csr
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -gencert -rfc -infile other-client.csr -outfile other-client.crt -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-client -file other-client.crt
+
+keytool -importkeystore -srckeystore other-client-keystore.p12 -destkeystore other-client-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-client-keystore.p12 -destkeystore other-client-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass

Review comment:
       Not really convinced 'other client'  (EDIT and now that I look, 'other server', 'unknown-client' and 'unknown-server') really needs to have a keystore and truststore of every type. Makes sense that the main set have the various different types to verify they all work (even though that has very little to do with the broker code in the end) but having them all for the various others seems like needless complexity.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682390004



##########
File path: examples/features/standard/jmx-ssl/readme.md
##########
@@ -16,12 +16,54 @@ With these properties, ActiveMQ Artemis broker will be manageable remotely using
 
 The various keystore files are generated using the following commands:
 
-* `keytool -genkey -keystore server-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore server-side-keystore.jks -file server-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore client-side-truststore.jks -file server-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
-* `keytool -genkey -keystore client-side-keystore.jks -storepass secureexample -keypass secureexample -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -keyalg RSA`
-* `keytool -export -keystore client-side-keystore.jks -file client-side-cert.cer -storepass secureexample`
-* `keytool -import -keystore server-side-truststore.jks -file client-side-cert.cer -storepass secureexample -keypass secureexample -noprompt`
+```shell
+#!/bin/bash
+set -e
+
+KEY_PASS=securepass
+STORE_PASS=securepass
+CA_VALIDITY=365000
+VALIDITY=36500

Review comment:
       I did get that, but I think the value is just unnecessarily large. So much so that people who dont know (and dont spot there is a 0 difference at the end, which I didnt initially) are likely to believe its something else.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682395429



##########
File path: tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/netty/NettyConnectorTest.java
##########
@@ -56,10 +59,10 @@ public void setUp() throws Exception {
       Map<String, Object> params = new HashMap<>();
       params.put(TransportConstants.SSL_ENABLED_PROP_NAME, true);
       params.put(TransportConstants.SSL_PROVIDER, TransportConstants.OPENSSL_PROVIDER);
-      params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "openssl-server-side-keystore.jks");
-      params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "secureexample");
-      params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME,  "openssl-server-side-truststore.jks");
-      params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "secureexample");
+      params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "server-keystore.jks");
+      params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "securepass");
+      params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME,  "client-ca-truststore.jks");
+      params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "securepass");

Review comment:
       The problem is one of them still reads like it mainly conveys the purpose rather than the content, which makes the other one seem like it does too, leading to the combination being confusing, more so when historically the name was historically where it was being used.
   
   I didnt particularly see need for the '-side' expansion in the old names, but together the old names were far clearer to me than the replacements. I fully expect people not copying an existing test to mess up using these, I know I likely will.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#issuecomment-885060227


   @brusdev the DO-NOT-MERGE-YET tag it now a moot thing. you can convert the PR as a draft.
   
   
   there is a link on the GitHub interface now:
   
   Still in progress? Convert to draft
   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit closed pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667


   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r681992361



##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpFailoverEndpointDiscoveryTest.java
##########
@@ -90,9 +93,9 @@ private JmsConnectionFactory getJmsConnectionFactory() {
       if (protocol == 0) {
          return new JmsConnectionFactory("failover:(amqp://localhost:61616)");
       } else {
-         String keystore = this.getClass().getClassLoader().getResource("client-side-keystore.jks").getFile();
-         String truststore = this.getClass().getClassLoader().getResource("client-side-truststore.jks").getFile();
-         return new JmsConnectionFactory("failover:(amqps://localhost:61616?transport.keyStoreLocation=" + keystore + "&transport.keyStorePassword=secureexample&transport.trustStoreLocation=" + truststore + "&transport.trustStorePassword=secureexample&transport.verifyHost=false)");
+         String keystore = this.getClass().getClassLoader().getResource("examples/features/standard/jmx-ssl/src/main/resources/activemq/server0/client-keystore.jks").getFile();
+         String truststore = this.getClass().getClassLoader().getResource("examples/features/standard/jmx-ssl/src/main/resources/activemq/server0/server-ca-truststore.jks").getFile();

Review comment:
       @jbertram thanks, it was due a refactoring issue of my IDE. I have just pushed a commit to fix.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682399848



##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java
##########
@@ -82,60 +85,18 @@ public CoreClientOverOneWaySSLTest(String storeProvider, String storeType, boole
       if (suffix.equalsIgnoreCase("PKCS12")) {
          suffix = "p12";
       }
-      SERVER_SIDE_KEYSTORE = "server-side-keystore." + suffix;
-      CLIENT_SIDE_TRUSTSTORE = "client-side-truststore." + suffix;
+      SERVER_SIDE_KEYSTORE = "server-keystore." + suffix;
+      CLIENT_SIDE_TRUSTSTORE = "server-ca-truststore." + suffix;

Review comment:
       I would hope the comments showed that I actually do understand what it is doing. I'm the one who argued that the tests should use CAs. I just think the different naming chosen adds scope for confusion, and is likely to trip people up who are used to the previous (and I would say more typical) naming format where the file name pointed to where it is used.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682406071



##########
File path: tests/security-resources/build.sh
##########
@@ -0,0 +1,156 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# The various SSL stores and certificates were created with the following commands:
+# Requires use of JDK 8+ keytool command.
+set -e
+
+KEY_PASS=securepass
+STORE_PASS=securepass
+CA_VALIDITY=365000
+VALIDITY=36500
+
+# Clean up existing files
+# -----------------------
+rm -f *.crt *.csr openssl-* *.jceks *.jks *.p12 *.pem
+
+# Create a key and self-signed certificate for the CA, to sign server certificate requests and use for trust:
+# ----------------------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias server-ca -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Server Certification Authority, OU=Artemis, O=ActiveMQ" -validity $CA_VALIDITY -ext bc:c=ca:true
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -exportcert -rfc > server-ca.crt
+openssl pkcs12 -in server-ca-keystore.p12 -nodes -nocerts -out server-ca.pem -password pass:$STORE_PASS
+
+# Create trust store with the server CA cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-ca-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -importkeystore -srckeystore server-ca-truststore.p12 -destkeystore server-ca-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore server-ca-truststore.p12 -destkeystore server-ca-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the server, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=sA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -alias server -certreq -file server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile server.csr -outfile server.crt -validity $VALIDITY -ext bc=ca:false -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server -file server.crt
+
+keytool -importkeystore -srckeystore server-keystore.p12 -destkeystore server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore server-keystore.p12 -destkeystore server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the other server, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias other-server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Other Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -alias other-server -certreq -file other-server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile other-server.csr -outfile other-server.crt -validity $VALIDITY -ext bc=ca:false -ext eku=sA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore other-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-server -file other-server.crt
+
+keytool -importkeystore -srckeystore other-server-keystore.p12 -destkeystore other-server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-server-keystore.p12 -destkeystore other-server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create trust store with the other server cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-server-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-server -file other-server.crt -noprompt
+keytool -importkeystore -srckeystore other-server-truststore.p12 -destkeystore other-server-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-server-truststore.p12 -destkeystore other-server-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create crl with the other server cert:
+# -------------------------------------------------------
+> openssl-database
+echo 00 > openssl-crlnumber
+openssl ca -config openssl.conf -revoke other-server.crt -keyfile server-ca.pem -cert server-ca.crt
+openssl ca -config openssl.conf -gencrl -keyfile server-ca.pem -cert server-ca.crt -out other-server-crl.pem -crldays $VALIDITY
+
+# Create a key pair for the broker with an unexpected hostname, and sign it with the CA:
+# --------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias unknown-server -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Unknown Server, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=sA
+
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -alias unknown-server -certreq -file unknown-server.csr
+keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -gencert -rfc -infile unknown-server.csr -outfile unknown-server.crt -validity $VALIDITY -ext bc=ca:false -ext eku=sA
+
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias server-ca -file server-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore unknown-server-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias unknown-server -file unknown-server.crt
+
+keytool -importkeystore -srckeystore unknown-server-keystore.p12 -destkeystore unknown-server-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore unknown-server-keystore.p12 -destkeystore unknown-server-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key and self-signed certificate for the CA, to sign client certificate requests and use for trust:
+# ----------------------------------------------------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias client-ca -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Client Certification Authority, OU=Artemis, O=ActiveMQ" -validity $CA_VALIDITY -ext bc:c=ca:true
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -exportcert -rfc > client-ca.crt
+openssl pkcs12 -in client-ca-keystore.p12 -nodes -nocerts -out client-ca.pem -password pass:$STORE_PASS
+
+# Create trust store with the client CA cert:
+# -------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-ca-truststore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -importkeystore -srckeystore client-ca-truststore.p12 -destkeystore client-ca-truststore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore client-ca-truststore.p12 -destkeystore client-ca-truststore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the client, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias client -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -alias client -certreq -file client.csr
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -gencert -rfc -infile client.csr -outfile client.crt -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client -file client.crt
+
+keytool -importkeystore -srckeystore client-keystore.p12 -destkeystore client-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore client-keystore.p12 -destkeystore client-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass
+
+# Create a key pair for the other client, and sign it with the CA:
+# ----------------------------------------------------------
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -alias other-client -genkey -keyalg "RSA" -keysize 2048 -dname "CN=ActiveMQ Artemis Other Client, OU=Artemis, O=ActiveMQ, L=AMQ, S=AMQ, C=AMQ" -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -alias other-client -certreq -file other-client.csr
+keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -gencert -rfc -infile other-client.csr -outfile other-client.crt -validity $VALIDITY -ext bc=ca:false -ext eku=cA -ext san=dns:localhost,ip:127.0.0.1
+
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias client-ca -file client-ca.crt -noprompt
+keytool -storetype pkcs12 -keystore other-client-keystore.p12 -storepass $STORE_PASS -keypass $KEY_PASS -importcert -alias other-client -file other-client.crt
+
+keytool -importkeystore -srckeystore other-client-keystore.p12 -destkeystore other-client-keystore.jceks -srcstoretype pkcs12 -deststoretype jceks -srcstorepass securepass -deststorepass securepass
+keytool -importkeystore -srckeystore other-client-keystore.p12 -destkeystore other-client-keystore.jks -srcstoretype pkcs12 -deststoretype jks -srcstorepass securepass -deststorepass securepass

Review comment:
       I would actually change the tests then, theres basically no need to run all those combinations. If the full suite wasted a little less time where easily possible (like here), it might not take so ridiculously long to run that important changes are made without running them (as clearly just happened on another PR).
   
   Files can be added later when they are actually needed, no need to keep around unused files. Though again, I see no reason the fully-unused ones would ever actually be needed, using them implies time wasting. However, being able to easily regenerate things when needed is part of the argument to use a script to create them (Though this case is so trivial you wouldnt even need to regenerate the rest, you could import the same keys to the new store with the command being added to the script) 




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3667: ARTEMIS-3367 Set verifyHost true by default

Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3667:
URL: https://github.com/apache/activemq-artemis/pull/3667#discussion_r682339973



##########
File path: tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/netty/NettyConnectorTest.java
##########
@@ -56,10 +59,10 @@ public void setUp() throws Exception {
       Map<String, Object> params = new HashMap<>();
       params.put(TransportConstants.SSL_ENABLED_PROP_NAME, true);
       params.put(TransportConstants.SSL_PROVIDER, TransportConstants.OPENSSL_PROVIDER);
-      params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "openssl-server-side-keystore.jks");
-      params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "secureexample");
-      params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME,  "openssl-server-side-truststore.jks");
-      params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "secureexample");
+      params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "server-keystore.jks");
+      params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "securepass");
+      params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME,  "client-ca-truststore.jks");
+      params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "securepass");

Review comment:
       I used names for security resources that denote their content and not the purpose to avoid confusion if they were to be used in different contexts.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org