[
https://issues.apache.org/jira/browse/ARTEMIS-3106?focusedWorklogId=559950&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-559950
]
ASF GitHub Bot logged work on ARTEMIS-3106:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 02/Mar/21 13:24
Start Date: 02/Mar/21 13:24
Worklog Time Spent: 10m
Work Description: gemmellr commented on a change in pull request #3470:
URL: https://github.com/apache/activemq-artemis/pull/3470#discussion_r585432662
##########
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SCRAMServerSASLFactory.java
##########
@@ -0,0 +1,244 @@
+/*
+ * 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.protocol.amqp.sasl.scram;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.security.GeneralSecurityException;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.util.Iterator;
+import java.util.UUID;
+
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.LoginContext;
+
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.protocol.amqp.broker.AmqpInterceptor;
+import org.apache.activemq.artemis.protocol.amqp.broker.ProtonProtocolManager;
+import org.apache.activemq.artemis.protocol.amqp.sasl.SASLResult;
+import org.apache.activemq.artemis.protocol.amqp.sasl.ServerSASL;
+import org.apache.activemq.artemis.protocol.amqp.sasl.ServerSASLFactory;
+import org.apache.activemq.artemis.spi.core.protocol.ProtocolManager;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import org.apache.activemq.artemis.spi.core.remoting.Connection;
+import org.apache.activemq.artemis.spi.core.security.jaas.DigestCallback;
+import org.apache.activemq.artemis.spi.core.security.jaas.HmacCallback;
+import
org.apache.activemq.artemis.spi.core.security.jaas.SCRAMMechanismCallback;
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.activemq.artemis.spi.core.security.scram.UserData;
+import org.jboss.logging.Logger;
+
+/**
+ * abstract class that implements the SASL-SCRAM authentication scheme,
concrete implementations
+ * must supply the {@link SCRAM} type to use and be register via SPI
+ */
+public abstract class SCRAMServerSASLFactory implements ServerSASLFactory {
+
+ /**
+ *
+ */
Review comment:
Is this needed?
##########
File path:
artemis-protocols/artemis-amqp-protocol/src/main/resources/META-INF/services/org.apache.activemq.artemis.protocol.amqp.sasl.ServerSASLFactory
##########
@@ -1,4 +1,7 @@
org.apache.activemq.artemis.protocol.amqp.sasl.AnonymousServerSASLFactory
org.apache.activemq.artemis.protocol.amqp.sasl.PlainServerSASLFactory
org.apache.activemq.artemis.protocol.amqp.sasl.GSSAPIServerSASLFactory
-org.apache.activemq.artemis.protocol.amqp.sasl.ExternalServerSASLFactory
\ No newline at end of file
+org.apache.activemq.artemis.protocol.amqp.sasl.ExternalServerSASLFactory
+org.apache.activemq.artemis.protocol.amqp.sasl.scram.SHA1SCRAMServerSASLFactory
+org.apache.activemq.artemis.protocol.amqp.sasl.scram.SHA256SCRAMServerSASLFactory
+org.apache.activemq.artemis.protocol.amqp.sasl.scram.SHA512CRAMServerSASLFactory
Review comment:
Last class name is 'wrong', its missing the S in SCRAM:
_SHA512**S**CRAMServerSASLFactory_.
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PrincipalsCallback.java
##########
@@ -16,31 +16,30 @@
*/
package org.apache.activemq.artemis.spi.core.security.jaas;
-import javax.security.auth.callback.Callback;
import java.security.Principal;
+import javax.security.auth.callback.Callback;
+
/**
- * A Callback for kerberos peer principal.
+ * A Callback for getting the peer principal.
Review comment:
principal**s**
##########
File path: NOTICE
##########
@@ -3,3 +3,8 @@ Copyright [2014-2021] The Apache Software Foundation
This product includes software developed at
The Apache Software Foundation (http://www.apache.org/).
+
+The Initial Developer of the ScramServerFunctionality.java,
ScramServerFunctionalityImpl.java,
+ScramException.java, ScramUtils.java, UserData.java
+is "SCRAM SASL authentication for Java" (https://github.com/ogrebgr/scram-sasl)
+Copyright 2016 Ognyan Bankov, Licensed under the Apache License, Version 2.0
Review comment:
I dont believe this is necessary.
The NOTICE file is essentially for 'required legal notifications' that
aren't satisfied otherwise already. It is for example sometimes used to move
copyright notices for submissions *by the copyright owner/agent* out of the
related source files, since it is preferred they not have any. For entirely 3rd
party items though as in this case, the original header is to be left as is,
meaning this copyright notice is already conveyed by the files themselves,
which are still ALv2 and so needn't have that called out specifically (which
would also be in the LICENCE file if it were). So I think this isnt needed.
If we thought it were needed, I'd likely lift the qpid-broker-j code I
pointed to (for the tests which this code still lacks) instead.
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/JaasCallbackHandler.java
##########
@@ -67,18 +68,22 @@ public void handle(Callback[] callbacks) throws
IOException, UnsupportedCallback
CertificateCallback certCallback = (CertificateCallback) callback;
certCallback.setCertificates(getCertsFromConnection(remotingConnection));
- } else if (callback instanceof Krb5Callback) {
- Krb5Callback krb5Callback = (Krb5Callback) callback;
+ } else if (callback instanceof PrincipalsCallback) {
+ PrincipalsCallback krb5Callback = (PrincipalsCallback) callback;
Review comment:
Renaming the variable as well as changing the type would seem
appropriate, its now somewhat misleading.
##########
File path: examples/protocols/amqp/pom.xml
##########
@@ -51,6 +51,7 @@ under the License.
<module>proton-clustered-cpp</module>
<module>queue</module>
<module>proton-ruby</module>
+ <module>sasl-scram</module>
Review comment:
Has a tab instead of spaces.
##########
File path: examples/protocols/amqp/sasl-scram/sasl-client/readme.md
##########
@@ -0,0 +1,3 @@
+# Artemis SASL-SCRAM Server and Client Example
+
+demonstrate the usage of SASL-SCRAM authentication with Artemis-MQ
Review comment:
ActiveMQ Artemis or just Artemis, not Artemis-MQ.
##########
File path:
examples/protocols/amqp/sasl-scram/sasl-client/src/main/java/org/apache/activemq/artemis/jms/example/QPIDClient.java
##########
@@ -0,0 +1,53 @@
+/*
+ * <p>
+ * All rights reserved. Licensed 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.qpid.jms.JmsConnectionFactory;
+
+public class QPIDClient {
+ public static void main(String[] args) throws JMSException {
+ for (String method : new String[] { "SCRAM-SHA-1",
"SCRAM-SHA-256" }) {
+ ConnectionFactory connectionFactory = new
JmsConnectionFactory(
+
"amqp://localhost:5672?amqp.saslMechanisms=" + method);
+ Connection connection;
+ if ("SCRAM-SHA-256".equals(method)) {
+ connection =
connectionFactory.createConnection("test", "test");
Review comment:
Having a util method that is called twice passing the mech and user/pass
args would be nicer/clearer than the for+if.
##########
File path: examples/protocols/amqp/sasl-scram/pom.xml
##########
@@ -0,0 +1,33 @@
+<?xml version='1.0'?>
+<!-- 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. -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0";
Review comment:
File has tabs instead of spaces.
##########
File path: examples/protocols/amqp/sasl-scram/sasl-client/pom.xml
##########
@@ -0,0 +1,41 @@
+<?xml version='1.0'?>
+<!-- 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. -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0";
Review comment:
File has tabs instead of spaces.
##########
File path: tests/integration-tests/src/test/resources/broker-saslscram.xml
##########
@@ -0,0 +1,44 @@
+<!--
+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.
+-->
+<configuration
Review comment:
File uses tabs.
##########
File path: examples/protocols/amqp/sasl-scram/sasl-server/readme.md
##########
@@ -0,0 +1,3 @@
+# Artemis SASL-SCRAM Server and Client Example
+
+demonstrate the usage of SASL-SCRAM authentication with Artemis-MQ
Review comment:
ActiveMQ Artemis or just Artemis, not Artemis-MQ.
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/SCRAMPropertiesLoginModule.java
##########
@@ -0,0 +1,202 @@
+/*
+ * 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.spi.core.security.jaas;
+
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.security.MessageDigest;
+import java.security.Principal;
+import java.security.SecureRandom;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+
+import javax.crypto.Mac;
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.FailedLoginException;
+import javax.security.auth.login.LoginException;
+
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramUtils;
+import org.apache.activemq.artemis.spi.core.security.scram.UserData;
+import org.apache.activemq.artemis.utils.PasswordMaskingUtil;
+
+/**
+ * Login modules that uses properties files similar to the {@link
PropertiesLoginModule}. It can
+ * either store the username-password in plain text or in an encrypted/hashed
form. the
+ * {@link #main(String[])} method provides a way to prepare unencrypted data
to be encrypted/hashed.
+ */
+public class SCRAMPropertiesLoginModule extends PropertiesLoader implements
AuditLoginModule {
+
+ private static final String SEPARATOR = ":";
+ private static final int MIN_ITERATIONS = 4096;
+ private Subject subject;
+ private CallbackHandler callbackHandler;
+ private Properties users;
+ private Map<String, Set<String>> roles;
+ private UserData userData;
+ private String user;
+ private final Set<Principal> principals = new HashSet<>();
+
+ @Override
+ public void initialize(Subject subject, CallbackHandler callbackHandler,
Map<String, ?> sharedState,
+ Map<String, ?> options) {
+ this.subject = subject;
+ this.callbackHandler = callbackHandler;
+
+ init(options);
+ users = load(PropertiesLoginModule.USER_FILE_PROP_NAME, "user",
options).getProps();
+ roles = load(PropertiesLoginModule.ROLE_FILE_PROP_NAME, "role",
options).invertedPropertiesValuesMap();
+
+ }
+
+ @Override
+ public boolean login() throws LoginException {
+ NameCallback nameCallback = new NameCallback("Username: ");
+ executeCallbacks(new Callback[] {nameCallback});
+ user = nameCallback.getName();
+ if (user == null) {
+ throw new FailedLoginException("User is null");
+ }
+ String password = users.getProperty(user);
+ if (password == null) {
+ throw new FailedLoginException("User does not exist: " + user);
+ }
+ if (PasswordMaskingUtil.isEncMasked(password)) {
+ String[] unwrap =
PasswordMaskingUtil.unwrap(password).split(SEPARATOR);
+ userData = new UserData(unwrap[0], Integer.parseInt(unwrap[1]),
unwrap[2], unwrap[3]);
+ } else {
+ DigestCallback digestCallback = new DigestCallback();
+ HmacCallback hmacCallback = new HmacCallback();
+ executeCallbacks(new Callback[] {digestCallback, hmacCallback});
+ byte[] salt = generateSalt();
+ try {
+ ScramUtils.NewPasswordStringData data =
+
ScramUtils.byteArrayToStringData(ScramUtils.newPassword(password, salt, 4096,
+
digestCallback.getDigest(),
+
hmacCallback.getHmac()));
+ userData = new UserData(data.salt, data.iterations,
data.serverKey, data.storedKey);
+ } catch (ScramException e) {
+ throw new LoginException();
+ }
+ }
+ return true;
+ }
+
+ private static byte[] generateSalt() {
+ byte[] salt = new byte[24];
Review comment:
Any reason to use 24 specifically rather than say 32?
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/SCRAMPropertiesLoginModule.java
##########
@@ -0,0 +1,202 @@
+/*
+ * 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.spi.core.security.jaas;
+
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.security.MessageDigest;
+import java.security.Principal;
+import java.security.SecureRandom;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+
+import javax.crypto.Mac;
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.FailedLoginException;
+import javax.security.auth.login.LoginException;
+
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramUtils;
+import org.apache.activemq.artemis.spi.core.security.scram.UserData;
+import org.apache.activemq.artemis.utils.PasswordMaskingUtil;
+
+/**
+ * Login modules that uses properties files similar to the {@link
PropertiesLoginModule}. It can
+ * either store the username-password in plain text or in an encrypted/hashed
form. the
+ * {@link #main(String[])} method provides a way to prepare unencrypted data
to be encrypted/hashed.
+ */
+public class SCRAMPropertiesLoginModule extends PropertiesLoader implements
AuditLoginModule {
+
+ private static final String SEPARATOR = ":";
+ private static final int MIN_ITERATIONS = 4096;
+ private Subject subject;
+ private CallbackHandler callbackHandler;
+ private Properties users;
+ private Map<String, Set<String>> roles;
+ private UserData userData;
+ private String user;
+ private final Set<Principal> principals = new HashSet<>();
+
+ @Override
+ public void initialize(Subject subject, CallbackHandler callbackHandler,
Map<String, ?> sharedState,
+ Map<String, ?> options) {
+ this.subject = subject;
+ this.callbackHandler = callbackHandler;
+
+ init(options);
+ users = load(PropertiesLoginModule.USER_FILE_PROP_NAME, "user",
options).getProps();
+ roles = load(PropertiesLoginModule.ROLE_FILE_PROP_NAME, "role",
options).invertedPropertiesValuesMap();
+
+ }
+
+ @Override
+ public boolean login() throws LoginException {
+ NameCallback nameCallback = new NameCallback("Username: ");
+ executeCallbacks(new Callback[] {nameCallback});
+ user = nameCallback.getName();
+ if (user == null) {
+ throw new FailedLoginException("User is null");
+ }
+ String password = users.getProperty(user);
+ if (password == null) {
+ throw new FailedLoginException("User does not exist: " + user);
+ }
Review comment:
Its generally nice not to give away that specific users dont exist, as
that can be used to determine users that do. I've seen other impls essentially
continue in this case and generate a salt, so that the SCRAM process continues
and only fails later in the same way as an incorrect password being specified,
rather than clearly failing early because the user didn't exist.
##########
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SHA512CRAMServerSASLFactory.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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.protocol.amqp.sasl.scram;
+
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+
+/**
+ * provides SASL SCRAM-SHA512
+ */
+public class SHA512CRAMServerSASLFactory extends SCRAMServerSASLFactory {
Review comment:
Class name is missing the S in SCRAM as noted for the service loader
config.
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/SaslScramTest.java
##########
@@ -0,0 +1,87 @@
+/**
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.tests.integration.amqp.connect;
Review comment:
This is the wrong package for this test, the 'connect' package is for
testing of the brokers outbound connection abilities. The base amqp package or
a new scram specific one would seem best.
This raises another point though - the broker wont be able to connect to
another Artemis instance if it is configured to use this SCRAM-SHA support,
since it lacks the client side mechanism support.
##########
File path: examples/protocols/amqp/sasl-scram/sasl-server/pom.xml
##########
@@ -0,0 +1,46 @@
+<?xml version='1.0'?>
+<!-- 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. -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0";
Review comment:
File has tabs instead of spaces.
##########
File path:
examples/protocols/amqp/sasl-scram/sasl-server/src/main/resources/artemis-users.properties
##########
@@ -0,0 +1,18 @@
+## ---------------------------------------------------------------------------
+## 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.
+## ---------------------------------------------------------------------------
+test =
ENC(VeEFPtxYIGIS0VA7e+W5+LpdLmazpcLH:4096:rIPD9cVvOoVJqzD7u4/qRMW1xGwILRG90g2OtWmn8T0=:7k8fGWO1zWPuZF0pzDCYnDCtmaJxLptqHS26SNsbGHU=)
Review comment:
Might be good for the example to explain how this was created, here
and/or in the readme.
Separately, that points to the lack of new documentation around this, which
will need added.
Also, I wonder if the ENC(..) format used here likely to confuse people into
thinking it is masked in the same way other user config can be, which I believe
uses a similar ENC(..) format string, when it is obviously handled in a very
different way.
##########
File path:
examples/protocols/amqp/sasl-scram/sasl-client/src/main/java/org/apache/activemq/artemis/jms/example/QPIDClient.java
##########
@@ -0,0 +1,53 @@
+/*
+ * <p>
+ * All rights reserved. Licensed 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.qpid.jms.JmsConnectionFactory;
+
+public class QPIDClient {
+ public static void main(String[] args) throws JMSException {
+ for (String method : new String[] { "SCRAM-SHA-1",
"SCRAM-SHA-256" }) {
+ ConnectionFactory connectionFactory = new
JmsConnectionFactory(
+
"amqp://localhost:5672?amqp.saslMechanisms=" + method);
+ Connection connection;
+ if ("SCRAM-SHA-256".equals(method)) {
+ connection =
connectionFactory.createConnection("test", "test");
+ } else {
+ connection =
connectionFactory.createConnection("hello", "ogre1234");
+ }
+ try {
+ Session session =
connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue =
session.createQueue("exampleQueue");
+ MessageProducer sender =
session.createProducer(queue);
+ sender.send(session.createTextMessage("Hello "
+ method));
+ connection.start();
+ MessageConsumer consumer =
session.createConsumer(queue);
+ TextMessage m = (TextMessage)
consumer.receive(5000);
+ System.out.println("message = " + m.getText());
+ } finally {
+ connection.close();
+ }
Review comment:
Perhaps a try-with-resources instead?
##########
File path:
tests/integration-tests/src/test/resources/artemis-scram-users.properties
##########
@@ -0,0 +1,18 @@
+## ---------------------------------------------------------------------------
+## 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.
+## ---------------------------------------------------------------------------
+test =
ENC(VeEFPtxYIGIS0VA7e+W5+LpdLmazpcLH:4096:rIPD9cVvOoVJqzD7u4/qRMW1xGwILRG90g2OtWmn8T0=:7k8fGWO1zWPuZF0pzDCYnDCtmaJxLptqHS26SNsbGHU=)
Review comment:
Similar to the example, perhaps worth explaining how this was generated
/ what it represents.
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/SaslScramTest.java
##########
@@ -0,0 +1,87 @@
+/**
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.tests.integration.amqp.connect;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.core.server.embedded.EmbeddedActiveMQ;
+import
org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager;
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * This test SASL-SCRAM Support
+ */
+public class SaslScramTest {
+
+ private static EmbeddedActiveMQ BROKER;
+
+ @BeforeClass
+ public static void startBroker() throws Exception {
+ String loginConfPath = new
File(SaslScramTest.class.getResource("/login.config").toURI()).getAbsolutePath();
+ System.setProperty("java.security.auth.login.config", loginConfPath);
+ BROKER = new EmbeddedActiveMQ();
+
BROKER.setConfigResourcePath(SaslScramTest.class.getResource("/broker-saslscram.xml").toExternalForm());
+ BROKER.setSecurityManager(new
ActiveMQJAASSecurityManager("artemis-sasl-scram"));
+ BROKER.start();
+ }
+
+ @AfterClass
+ public static void shutdownBroker() throws Exception {
+ BROKER.stop();
+ }
+
+ @Test
+ public void testSendReceive() throws JMSException {
Review comment:
Same comments as the example since this is essentially the same code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 559950)
Time Spent: 4h (was: 3h 50m)
> Support for SASL-SCRAM
> ----------------------
>
> Key: ARTEMIS-3106
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3106
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Components: AMQP
> Reporter: Christoph Läubrich
> Priority: Major
> Time Spent: 4h
> Remaining Estimate: 0h
>
> With the enhancements in ARTEMIS-33 / [PR
> 3432|https://github.com/apache/activemq-artemis/pull/3432] it would be now
> possible to plug-in new SASL mechanism.
> One popular one is
> [SASL-SCRAM|https://en.wikipedia.org/wiki/Salted_Challenge_Response_Authentication_Mechanism]
> because it allows channelbinding together with secure storage of
> user-credential.
> I have created an [implementation of this for Artemis
> AMQP|https://github.com/laeubi/scram-sasl/tree/artemis/artemis] based on the
> [SCRAM SASL authentication for Java|https://github.com/ogrebgr/scram-sasl]
> code with some enhancements/cleanups to the original.
> As the source is already Apache licensed I'd like to propose to include this
> in the Artemis code-base. This would greatly enhance the interoperability
> with other implementations e.g. Apache QPID.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
