Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr merged PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407 -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1892078041 ## tests/security-resources/build.sh: ## @@ -175,6 +175,9 @@ openssl pkcs12 -in client-keystore.p12 -out client-key-cert.pem -nodes -password keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -exportcert -rfc > server-ca-cert.pem keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -exportcert -rfc > client-ca-cert.pem +## Combined ca-certs pem to verify loading of multiple certs +cat client-ca-cert.pem server-ca-cert.pem > client-and-server-ca-cert.pem Review Comment: Sure it can, I will do that right away -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr commented on code in PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1892061469 ## tests/security-resources/build.sh: ## @@ -175,6 +175,9 @@ openssl pkcs12 -in client-keystore.p12 -out client-key-cert.pem -nodes -password keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -exportcert -rfc > server-ca-cert.pem keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -exportcert -rfc > client-ca-cert.pem +## Combined ca-certs pem to verify loading of multiple certs +cat client-ca-cert.pem server-ca-cert.pem > client-and-server-ca-cert.pem Review Comment: Nitpick but can it be named client-and-server-ca-cert**s**.pem to make clearer later that its not a single shared cert. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1891722972
##
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/kubernetes/client/KubernetesClientImpl.java:
##
@@ -157,18 +161,12 @@ private SSLContext buildSSLContext() throws Exception {
logger.debug("Kubernetes CA certificate not found at: {}. Truststore
not configured", caPath);
return ctx;
}
- try (InputStream fis = new FileInputStream(certFile)) {
- KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());
- CertificateFactory certFactory =
CertificateFactory.getInstance("X.509");
- X509Certificate certificate = (X509Certificate)
certFactory.generateCertificate(fis);
- trustStore.load(null, null);
- trustStore.setCertificateEntry(certFile.getName(), certificate);
- TrustManagerFactory tmFactory = TrustManagerFactory
- .getInstance(TrustManagerFactory.getDefaultAlgorithm());
- tmFactory.init(trustStore);
-
- ctx.init(null, tmFactory.getTrustManagers(), new SecureRandom());
- }
+ KeyStore trustStore = SSLSupport.loadKeystore(null, "PEMCA", caPath,
null);
Review Comment:
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#issuecomment-2553566982 I have removed the cli command, it's greatly simplifying the PR, which is good. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1891687590
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ String kubeHost = input("--kube-host", "What is the cluster host?",
"api.crc.testing");
+ String kubeport = input("--kube-port", "What is the cluster port?",
"6443");
+ String tokenpath = input("--token-path", "What is the token path?",
"/var/run/secrets/kubernetes.io/serviceaccount/token");
+ String capath = input("--ca-path", "What is the ca path?",
"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt");
Review Comment:
Alright, leaving it out works for me! I'll remove it now.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gtully commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1891654528
##
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/kubernetes/client/KubernetesClientImpl.java:
##
@@ -157,18 +161,12 @@ private SSLContext buildSSLContext() throws Exception {
logger.debug("Kubernetes CA certificate not found at: {}. Truststore
not configured", caPath);
return ctx;
}
- try (InputStream fis = new FileInputStream(certFile)) {
- KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());
- CertificateFactory certFactory =
CertificateFactory.getInstance("X.509");
- X509Certificate certificate = (X509Certificate)
certFactory.generateCertificate(fis);
- trustStore.load(null, null);
- trustStore.setCertificateEntry(certFile.getName(), certificate);
- TrustManagerFactory tmFactory = TrustManagerFactory
- .getInstance(TrustManagerFactory.getDefaultAlgorithm());
- tmFactory.init(trustStore);
-
- ctx.init(null, tmFactory.getTrustManagers(), new SecureRandom());
- }
+ KeyStore trustStore = SSLSupport.loadKeystore(null, "PEMCA", caPath,
null);
Review Comment:
this looks good. I think it is the correct approach.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gtully commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1891653055
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ String kubeHost = input("--kube-host", "What is the cluster host?",
"api.crc.testing");
+ String kubeport = input("--kube-port", "What is the cluster port?",
"6443");
+ String tokenpath = input("--token-path", "What is the token path?",
"/var/run/secrets/kubernetes.io/serviceaccount/token");
+ String capath = input("--ca-path", "What is the ca path?",
"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt");
Review Comment:
I too am not convinced of the value of the cli bit. it is quite involved and
very specific. a more general approach would be through a jaas login check etc
but that would be more work. In general, access to the command line should be
very restricted so at best this is some sort of dev/debug tool.
personally I would leave it out.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1891504644
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @CommandLine.Option(names = "--kube-host", description = "hostname for
kubernetes api server")
+ protected String host;
+
+ @CommandLine.Option(names = "--kube-port", description = "port for
kubernetes api server")
+ protected String port;
+
+ @CommandLine.Option(names = "--token-path", description = "path to the
token to access the kubernetes api server with access to token reviews")
+ protected String tokenPath;
+
+ @CommandLine.Option(names = "--ca-path", description = "path to the
kubernetes api server trusted CAs")
+ protected String caPath;
+
+ @CommandLine.Option(names = "--token", description = "token to review")
+ protected String token;
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ if (host == null) {
+ host = input("--kube-host", "What is the cluster host?",
System.getProperty(KubernetesClientImpl.KUBERNETES_HOST));
+ }
+ if (port == null) {
+ port = input("--kube-port", "What is the cluster port?",
System.getProperty(KubernetesClientImpl.KUBERNETES_PORT));
+ }
+ if (tokenPath == null) {
+ tokenPath = input("--token-path", "What is the token path?",
System.getProperty(KubernetesClientImpl.KUBERNETES_TOKEN_PATH));
+ }
+ if (caPath == null) {
+ caPath = input("--ca-path", "What is the ca path?",
System.getProperty(KubernetesClientImpl.KUBERNETES_CA_PATH));
+ }
+ if (token == null) {
+ token = input("--token", "What is the token?", "");
+ }
Review Comment:
I've changed the behavior so that it takes the default values when needed.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1891328526
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @CommandLine.Option(names = "--kube-host", description = "hostname for
kubernetes api server")
+ protected String host;
+
+ @CommandLine.Option(names = "--kube-port", description = "port for
kubernetes api server")
+ protected String port;
+
+ @CommandLine.Option(names = "--token-path", description = "path to the
token to access the kubernetes api server with access to token reviews")
+ protected String tokenPath;
+
+ @CommandLine.Option(names = "--ca-path", description = "path to the
kubernetes api server trusted CAs")
+ protected String caPath;
+
+ @CommandLine.Option(names = "--token", description = "token to review")
+ protected String token;
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ if (host == null) {
+ host = input("--kube-host", "What is the cluster host?",
System.getProperty(KubernetesClientImpl.KUBERNETES_HOST));
+ }
+ if (port == null) {
+ port = input("--kube-port", "What is the cluster port?",
System.getProperty(KubernetesClientImpl.KUBERNETES_PORT));
+ }
+ if (tokenPath == null) {
+ tokenPath = input("--token-path", "What is the token path?",
System.getProperty(KubernetesClientImpl.KUBERNETES_TOKEN_PATH));
+ }
+ if (caPath == null) {
+ caPath = input("--ca-path", "What is the ca path?",
System.getProperty(KubernetesClientImpl.KUBERNETES_CA_PATH));
+ }
+ if (token == null) {
+ token = input("--token", "What is the token?", "");
+ }
Review Comment:
> I actually only meant ask for the token by input if not specified via
option. The rest all have defaults right? How likely is it someone will
actually be changing the value, i.e should they have to manually
provide/confirm all of them if not set by option, or are those underlying
defaults likely to be used and so be useful here if not specified?
It had value for debugging, since we could also tap into another
endpoint/port combo. But yea, getting the default value for the parameter makes
sense. I'll be in favor of letting the user able to override the value if
needed but only require the token as you suggest.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1890639827
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @CommandLine.Option(names = "--kube-host", description = "hostname for
kubernetes api server")
+ protected String host;
+
+ @CommandLine.Option(names = "--kube-port", description = "port for
kubernetes api server")
+ protected String port;
+
+ @CommandLine.Option(names = "--token-path", description = "path to the
token to access the kubernetes api server with access to token reviews")
+ protected String tokenPath;
+
+ @CommandLine.Option(names = "--ca-path", description = "path to the
kubernetes api server trusted CAs")
+ protected String caPath;
+
+ @CommandLine.Option(names = "--token", description = "token to review")
+ protected String token;
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ if (host == null) {
+ host = input("--kube-host", "What is the cluster host?",
System.getProperty(KubernetesClientImpl.KUBERNETES_HOST));
+ }
+ if (port == null) {
+ port = input("--kube-port", "What is the cluster port?",
System.getProperty(KubernetesClientImpl.KUBERNETES_PORT));
+ }
+ if (tokenPath == null) {
+ tokenPath = input("--token-path", "What is the token path?",
System.getProperty(KubernetesClientImpl.KUBERNETES_TOKEN_PATH));
+ }
+ if (caPath == null) {
+ caPath = input("--ca-path", "What is the ca path?",
System.getProperty(KubernetesClientImpl.KUBERNETES_CA_PATH));
+ }
+ if (token == null) {
+ token = input("--token", "What is the token?", "");
+ }
Review Comment:
I actually only meant ask for the token by input if not specified via
option. The rest all have defaults right? How likely is it someone will
actually be changing the value, i.e should they have to manually
provide/confirm all of them if not set by option, or are those underlying
defaults likely to be used and so be useful here if not specified?
Also, this only pulls the system property as its own 'local default', but
the underlying client checks the env variables too, so seems like this should
also check both? Should it perhaps just call the same getParam method?
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1890627019
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/Kubernetes.java:
##
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands.tools;
+
+import org.apache.activemq.artemis.cli.commands.HelpAction;
+import org.apache.activemq.artemis.cli.commands.TokenReview;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+@Command(name = "kube", description = "use 'help kube' for sub commands list",
subcommands = {TokenReview.class})
+public class Kubernetes implements Runnable {
+
+ CommandLine commandLine;
+
+ public Kubernetes(CommandLine commandLine) {
+ this.commandLine = commandLine;
+ }
+
+ @Override
+ public void run() {
+ HelpAction.help(commandLine, "data");
Review Comment:
Should this be "kube" to match the group name, rather than "data"?
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr commented on code in PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1890265382 ## tests/security-resources/client-and-server-ca-cert.pem: ## @@ -0,0 +1,42 @@ +-BEGIN CERTIFICATE- Review Comment: Since adding this file, which is only copying/using existing certs, is the only real change needed beyond the build.sh script...I think we should just avoid regenerating the whole set of files this time to minimise the overall change and make more obvious the effect of the actual change, i.e you can just revert the changes to all the other security-resources files except those two. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr commented on code in PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1890260472 ## tests/security-resources/build.sh: ## @@ -174,6 +174,7 @@ echo source.cert=classpath:unknown-server-cert.pem >> unknown-server-keystore.pe openssl pkcs12 -in client-keystore.p12 -out client-key-cert.pem -nodes -password pass:$STORE_PASS keytool -storetype pkcs12 -keystore server-ca-keystore.p12 -storepass $STORE_PASS -alias server-ca -exportcert -rfc > server-ca-cert.pem keytool -storetype pkcs12 -keystore client-ca-keystore.p12 -storepass $STORE_PASS -alias client-ca -exportcert -rfc > client-ca-cert.pem +cat client-ca-cert.pem server-ca-cert.pem > client-and-server-ca-cert.pem Review Comment: Would put this in its own section, its really a distinct usage from the bits in the section above (which probably should not be grouped together to begin with as they are for different things) ```suggestion ## Combined ca-certs pem to verify loading of multiple certs cat client-ca-cert.pem server-ca-cert.pem > client-and-server-ca-cert.pem ``` ## tests/security-resources/client-and-server-ca-cert.pem: ## @@ -0,0 +1,42 @@ +-BEGIN CERTIFICATE- Review Comment: Since adding this file, which is only copying/using existing certs, is the only reason change needed beyond the build.sh script...I think we should just avoid regneerating the whole set of files this time to minimise the overall change and make more obvious the effect of them, i.e you can just revert the changes to all the other security-resources files except those two. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1890230376
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ String kubeHost = input("--kube-host", "What is the cluster host?",
"api.crc.testing");
+ String kubeport = input("--kube-port", "What is the cluster port?",
"6443");
+ String tokenpath = input("--token-path", "What is the token path?",
"/var/run/secrets/kubernetes.io/serviceaccount/token");
+ String capath = input("--ca-path", "What is the ca path?",
"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt");
Review Comment:
I wouldn't consider the CLI command a must have, but I figured that it would
be nice to have an easy way to determine if an openshift config is correctly
working or not. I'm not against removing it if you think it shouldn't be there.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r112326
##
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/kubernetes/client/KubernetesClientImpl.java:
##
@@ -160,9 +163,13 @@ private SSLContext buildSSLContext() throws Exception {
try (InputStream fis = new FileInputStream(certFile)) {
KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());
Review Comment:
I'm trying that out, but without much success actually. It doesn't seem to
be able to load the `ca.crt` file properly.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt commented on code in PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r111420 ## artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java: ## @@ -0,0 +1,69 @@ +/* + * 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. + */ + +package org.apache.activemq.artemis.cli.commands; + +import org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient; +import org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl; +import picocli.CommandLine; + +@CommandLine.Command(name = "token-review", description = "Perform a kubernetes token review") Review Comment: Good idea, I'll do this. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gemmellr commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1888718616
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ String kubeHost = input("--kube-host", "What is the cluster host?",
"api.crc.testing");
+ String kubeport = input("--kube-port", "What is the cluster port?",
"6443");
+ String tokenpath = input("--token-path", "What is the token path?",
"/var/run/secrets/kubernetes.io/serviceaccount/token");
+ String capath = input("--ca-path", "What is the ca path?",
"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt");
+ String prevKSH = System.getProperty("KUBERNETES_SERVICE_HOST", kubeHost);
+ String prevKSP = System.getProperty("KUBERNETES_SERVICE_PORT", kubeport);
+ String prevTP = System.getProperty("KUBERNETES_TOKEN_PATH", tokenpath);
+ String prevCP = System.getProperty("KUBERNETES_CA_PATH", capath);
+ System.setProperty("KUBERNETES_SERVICE_HOST", kubeHost);
+ System.setProperty("KUBERNETES_SERVICE_PORT", kubeport);
+ System.setProperty("KUBERNETES_TOKEN_PATH", tokenpath);
+ System.setProperty("KUBERNETES_CA_PATH", capath);
+ String token = input("--token", "What is the token?",
"sha256~Cg_LIHObUgJHvF_Ziv9k9eHZQ3_UUNihRD9vA96hQKU");
Review Comment:
Not sure I'd put in a default token
##
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient;
+import
org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "token-review", description = "Perform a
kubernetes token review")
+public class TokenReview extends InputAbstract {
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ context.out.println();
+ String kubeHost = input("--kube-host", "What is the cluster host?",
"api.crc.testing");
+ String kubeport = input("--kube-port", "What is the cluster port?",
"6443");
+ String tokenpath = input("--token-path", "What is the token path?",
"/var/run/secrets/kubernetes.io/serviceaccount/token");
+ String capath = input("--ca-path", "What is the ca path?",
"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt");
+ String prevKSH = System.getProperty("KUBERNETES_SERVICE_HOST", kubeHost);
+ String prevKSP = System.getProperty("KUBERNETES_SERVICE_PORT", kubeport);
+ String prevTP = System.getProperty("KUBERNETES_TOKEN_PATH", t
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
gtully commented on code in PR #5407:
URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1888715162
##
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/kubernetes/client/KubernetesClientImpl.java:
##
@@ -160,9 +163,13 @@ private SSLContext buildSSLContext() throws Exception {
try (InputStream fis = new FileInputStream(certFile)) {
KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());
Review Comment:
I think it would make sense to lean on the PEM keyStoreType support, and
make use of SSLSupport utility calss to load a trust store. In that way, this
ssl client will behave like the others in the broker.
When this client was introduced there was no support for PEM in the broker,
but now there is.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
clebertsuconic commented on code in PR #5407: URL: https://github.com/apache/activemq-artemis/pull/5407#discussion_r1888626764 ## artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/TokenReview.java: ## @@ -0,0 +1,69 @@ +/* + * 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. + */ + +package org.apache.activemq.artemis.cli.commands; + +import org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClient; +import org.apache.activemq.artemis.spi.core.security.jaas.kubernetes.client.KubernetesClientImpl; +import picocli.CommandLine; + +@CommandLine.Command(name = "token-review", description = "Perform a kubernetes token review") Review Comment: just an idea: @lavocatt if we are adding tools to be used in kubernetes, I would aggregate them in a group... like we do with data ./artemis data print I would aggregate anything related to kubernetes as something like: ./artemis kube token-review unless we are 100% we wouldn't get other kube things? -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
[PR] [ARTEMIS-5219] load all certificates [activemq-artemis]
lavocatt opened a new pull request, #5407: URL: https://github.com/apache/activemq-artemis/pull/5407 In the JAAS login module for Kubernetes, the current implementation is loading only a single CA from the bundle that is given by OpenShift, which in turn leads to an issue where the broker don't find a proper PKIX path. This is fixing the issue by loading every certificates from the bundle. The commit also brings in a new command line that can be used to test if pod is well configured to perform token reviews or not. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
