[ 
https://issues.apache.org/jira/browse/ARTEMIS-3106?focusedWorklogId=571339&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-571339
 ]

ASF GitHub Bot logged work on ARTEMIS-3106:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 24/Mar/21 17:28
            Start Date: 24/Mar/21 17:28
    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_r600584160



##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SCRAMClientSASL.java
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.nio.charset.StandardCharsets;
+import java.util.Objects;
+import java.util.UUID;
+
+import org.apache.activemq.artemis.protocol.amqp.sasl.ClientSASL;
+import 
org.apache.activemq.artemis.protocol.amqp.sasl.scram.ScramClientFunctionality.State;
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.qpid.proton.codec.DecodeException;

Review comment:
       Seems unusual to be throwing Proton's decode exception here.

##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SCRAMClientSASL.java
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.nio.charset.StandardCharsets;
+import java.util.Objects;
+import java.util.UUID;
+
+import org.apache.activemq.artemis.protocol.amqp.sasl.ClientSASL;
+import 
org.apache.activemq.artemis.protocol.amqp.sasl.scram.ScramClientFunctionality.State;
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.qpid.proton.codec.DecodeException;
+
+/**
+ * implements the client part of SASL-SCRAM for broker interconnect
+ */
+public class SCRAMClientSASL implements ClientSASL {
+   private final SCRAM scramType;
+   private final ScramClientFunctionalityImpl client;
+   private final String username;
+   private final String password;
+
+   /**
+    * @param scram the SCRAM mechanism to use
+    * @param username the username for authentication
+    * @param password the password for authentication
+    */
+   public SCRAMClientSASL(SCRAM scram, String username, String password) {
+      Objects.requireNonNull(scram);
+      Objects.requireNonNull(username);
+      Objects.requireNonNull(password);
+      this.username = username;
+      this.password = password;
+      this.scramType = scram;
+      client = new ScramClientFunctionalityImpl(scram.getDigest(), 
scram.getHmac(), UUID.randomUUID().toString());
+   }
+
+   @Override
+   public String getName() {
+      return scramType.getName();
+   }
+
+   @Override
+   public byte[] getInitialResponse() {
+      try {
+         String firstMessage = client.prepareFirstMessage(username);
+         System.out.println("SCRAMClientSASL.getInitialResponse() >> " + 
firstMessage);
+         return firstMessage.getBytes(StandardCharsets.US_ASCII);
+      } catch (ScramException e) {
+         throw new DecodeException("prepareFirstMessage failed", e);
+      }
+   }
+
+   @Override
+   public byte[] getResponse(byte[] challenge) {
+      String msg = new String(challenge, StandardCharsets.US_ASCII);
+      System.out.println("SCRAMClientSASL.getResponse() << " + msg);

Review comment:
       Leftover System.out.println

##########
File path: 
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/SCRAMPropertiesLoginModule.java
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.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;
+import org.jgroups.util.UUID;
+
+/**
+ * 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_MECHANISM = "|";
+   private static final String SEPARATOR_PARAMETER = ":";
+   private static final int MIN_ITERATIONS = 4096;
+   private static final SecureRandom RANDOM_GENERATOR = new SecureRandom();
+   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(nameCallback);
+      user = nameCallback.getName();
+      if (user == null) {
+         user = UUID.randomUUID().toString();

Review comment:
       Do we need to generate a fake username, cant it just be left null and 
the rest handle that?

##########
File path: 
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/SCRAMPropertiesLoginModule.java
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.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;
+import org.jgroups.util.UUID;
+
+/**
+ * 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_MECHANISM = "|";
+   private static final String SEPARATOR_PARAMETER = ":";
+   private static final int MIN_ITERATIONS = 4096;
+   private static final SecureRandom RANDOM_GENERATOR = new SecureRandom();
+   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(nameCallback);
+      user = nameCallback.getName();
+      if (user == null) {
+         user = UUID.randomUUID().toString();
+      }
+      SCRAMMechanismCallback mechanismCallback = new SCRAMMechanismCallback();
+      executeCallbacks(mechanismCallback);
+      SCRAM scram = getTypeByString(mechanismCallback.getMechanism());
+      String password = users.getProperty(user, users.getProperty(user + 
SEPARATOR_MECHANISM + scram.name()));
+      if (PasswordMaskingUtil.isEncMasked(password)) {
+         String[] unwrap = 
PasswordMaskingUtil.unwrap(password).split(SEPARATOR_PARAMETER);
+         userData = new UserData(unwrap[0], Integer.parseInt(unwrap[1]), 
unwrap[2], unwrap[3]);
+      } else {
+         userData = generateUserData(password);
+      }
+      return true;
+   }
+
+   private UserData generateUserData(String plainTextPassword) throws 
LoginException {
+      if (plainTextPassword == null) {
+         // if the user is not available (or the password) generate a random 
password here so an
+         // attacker can't
+         // distinguish between a missing username and a wrong password
+         byte[] randomPassword = new byte[256];
+         RANDOM_GENERATOR.nextBytes(randomPassword);
+         plainTextPassword = new String(randomPassword);
+      }
+      DigestCallback digestCallback = new DigestCallback();
+      HmacCallback hmacCallback = new HmacCallback();
+      executeCallbacks(digestCallback, hmacCallback);
+      byte[] salt = generateSalt();
+      try {
+         ScramUtils.NewPasswordStringData data =
+                  
ScramUtils.byteArrayToStringData(ScramUtils.newPassword(plainTextPassword, 
salt, 4096,
+                                                                          
digestCallback.getDigest(),
+                                                                          
hmacCallback.getHmac()));
+         return new UserData(data.salt, data.iterations, data.serverKey, 
data.storedKey);
+      } catch (ScramException e) {
+         throw new LoginException();
+      }
+   }
+
+   private static byte[] generateSalt() {
+      byte[] salt = new byte[32];
+      RANDOM_GENERATOR.nextBytes(salt);
+      return salt;
+   }
+
+   private void executeCallbacks(Callback... callbacks) throws LoginException {
+      try {
+         callbackHandler.handle(callbacks);
+      } catch (UnsupportedCallbackException | IOException e) {
+         throw new LoginException();
+      }
+   }
+
+   @Override
+   public boolean commit() throws LoginException {
+      if (userData == null) {
+         throw new LoginException();
+      }
+      subject.getPublicCredentials().add(userData);
+      Set<UserPrincipal> authenticatedUsers = 
subject.getPrincipals(UserPrincipal.class);
+      UserPrincipal principal = new UserPrincipal(user);
+      principals.add(principal);
+      authenticatedUsers.add(principal);
+      for (UserPrincipal userPrincipal : authenticatedUsers) {
+         Set<String> matchedRoles = roles.get(userPrincipal.getName());
+         if (matchedRoles != null) {
+            for (String entry : matchedRoles) {
+               principals.add(new RolePrincipal(entry));
+            }
+         }
+      }
+      subject.getPrincipals().addAll(principals);
+      return true;
+   }
+
+   @Override
+   public boolean abort() throws LoginException {
+      return true;
+   }
+
+   @Override
+   public boolean logout() throws LoginException {
+      subject.getPrincipals().removeAll(principals);
+      principals.clear();
+      subject.getPublicCredentials().remove(userData);
+      userData = null;
+      return true;
+   }
+
+   /**
+    * Main method that could be used to encrypt given credentials for use in 
properties files
+    * @param args username password type [iterations]
+    * @throws GeneralSecurityException if any security mechanism is not 
available on this JVM
+    * @throws ScramException if invalid data is supplied
+    */
+   public static void main(String[] args) throws GeneralSecurityException, 
ScramException {
+      if (args.length < 2) {
+         System.out.println("Usage: " + 
SCRAMPropertiesLoginModule.class.getSimpleName() +
+                  " <username> <password> [<iterations>]");
+         System.out.println("\ttype: " + getSupportedTypes());
+         System.out.println("\titerations desired number of iteration (min 
value: " + MIN_ITERATIONS + ")");
+         return;
+      }
+      String username = args[0];
+      String password = args[1];
+      for (SCRAM scram : SCRAM.values()) {
+         MessageDigest digest = MessageDigest.getInstance(scram.getDigest());
+         Mac hmac = Mac.getInstance(scram.getHmac());
+         byte[] salt = generateSalt();
+         int iterations;
+         if (args.length > 2) {
+            iterations = Integer.parseInt(args[2]);
+            if (iterations < MIN_ITERATIONS) {
+               throw new IllegalArgumentException("minimum of " + 
MIN_ITERATIONS + " required!");
+            }
+         } else {
+            iterations = MIN_ITERATIONS;
+         }
+         ScramUtils.NewPasswordStringData data =
+                  
ScramUtils.byteArrayToStringData(ScramUtils.newPassword(password, salt, 
iterations, digest, hmac));
+         System.out.println(username + SEPARATOR_MECHANISM + scram.name() + " 
= " +

Review comment:
       This is printing the given username as-is, while the client bits added 
for the outgoing broker-connection stuff does all the string prep 
reworking...it seems the broker just does a direct lookup of whats received, 
wouldn't this output or the lookup need to have similar hoop jumping so they 
will align and the lookup works?
   
   (Elsewhere we just avoided this complexity and required use of ASCII 
usernames, reducing things to simple substitutions of a couple characters)

##########
File path: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AMQPConnectSaslTest.java
##########
@@ -296,4 +342,92 @@ public String getChosenMech() {
       }
    }
 
+   private static final class SCRAMTestAuthenticator implements 
ProtonSaslAuthenticator {
+
+      private final SCRAM mech;
+      private Sasl sasl;
+      private TestSCRAMServerSASL serverSASL;
+
+      SCRAMTestAuthenticator(SCRAM mech) {
+         this.mech = mech;
+      }
+
+      @Override
+      public void init(NetSocket socket, ProtonConnection protonConnection, 
Transport transport) {
+         this.sasl = transport.sasl();
+         sasl.server();
+         sasl.allowSkip(false);
+         sasl.setMechanisms(mech.getName());
+         try {
+            serverSASL = new TestSCRAMServerSASL(mech, 
Logger.getLogger(getClass()));
+         } catch (NoSuchAlgorithmException e) {
+            throw new AssertionError(e);
+         }
+
+      }
+
+      @Override
+      public void process(Handler<Boolean> completionHandler) {
+         String[] remoteMechanisms = sasl.getRemoteMechanisms();
+         int pending = sasl.pending();
+         if (remoteMechanisms.length == 0 || pending == 0) {
+            completionHandler.handle(false);
+            return;
+         }
+         byte[] msg = new byte[pending];
+         sasl.recv(msg, 0, msg.length);
+         byte[] result = serverSASL.processSASL(msg);
+         if (result != null) {
+            sasl.send(result, 0, result.length);
+         }
+         boolean ended = serverSASL.isEnded();
+         System.out.println("end=" + ended);
+         if (ended) {
+            if (succeeded()) {
+               sasl.done(SaslOutcome.PN_SASL_OK);
+            } else {
+               sasl.done(SaslOutcome.PN_SASL_AUTH);
+            }
+            completionHandler.handle(true);
+         } else {
+            completionHandler.handle(false);
+         }
+      }
+
+      @Override
+      public boolean succeeded() {
+         SASLResult result = serverSASL.result();
+         return result != null && result.isSuccess();
+      }
+
+   }
+
+   private static final class TestSCRAMServerSASL extends SCRAMServerSASL {
+
+      TestSCRAMServerSASL(SCRAM mechanism, Logger logger) throws 
NoSuchAlgorithmException {
+         super(mechanism, logger);
+      }
+
+      @Override
+      public void done() {
+         // nothing to do
+      }
+
+      @Override
+      protected UserData aquireUserData(String userName) throws LoginException 
{

Review comment:
       Should verify the username was as expected also.

##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SCRAMClientSASL.java
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.nio.charset.StandardCharsets;
+import java.util.Objects;
+import java.util.UUID;
+
+import org.apache.activemq.artemis.protocol.amqp.sasl.ClientSASL;
+import 
org.apache.activemq.artemis.protocol.amqp.sasl.scram.ScramClientFunctionality.State;
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.qpid.proton.codec.DecodeException;
+
+/**
+ * implements the client part of SASL-SCRAM for broker interconnect
+ */
+public class SCRAMClientSASL implements ClientSASL {
+   private final SCRAM scramType;
+   private final ScramClientFunctionalityImpl client;
+   private final String username;
+   private final String password;
+
+   /**
+    * @param scram the SCRAM mechanism to use
+    * @param username the username for authentication
+    * @param password the password for authentication
+    */
+   public SCRAMClientSASL(SCRAM scram, String username, String password) {
+      Objects.requireNonNull(scram);
+      Objects.requireNonNull(username);
+      Objects.requireNonNull(password);
+      this.username = username;
+      this.password = password;
+      this.scramType = scram;
+      client = new ScramClientFunctionalityImpl(scram.getDigest(), 
scram.getHmac(), UUID.randomUUID().toString());
+   }
+
+   @Override
+   public String getName() {
+      return scramType.getName();
+   }
+
+   @Override
+   public byte[] getInitialResponse() {
+      try {
+         String firstMessage = client.prepareFirstMessage(username);
+         System.out.println("SCRAMClientSASL.getInitialResponse() >> " + 
firstMessage);

Review comment:
       Leftover System.out.println

##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/sasl/scram/SCRAMClientSASL.java
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.nio.charset.StandardCharsets;
+import java.util.Objects;
+import java.util.UUID;
+
+import org.apache.activemq.artemis.protocol.amqp.sasl.ClientSASL;
+import 
org.apache.activemq.artemis.protocol.amqp.sasl.scram.ScramClientFunctionality.State;
+import org.apache.activemq.artemis.spi.core.security.scram.SCRAM;
+import org.apache.activemq.artemis.spi.core.security.scram.ScramException;
+import org.apache.qpid.proton.codec.DecodeException;
+
+/**
+ * implements the client part of SASL-SCRAM for broker interconnect
+ */
+public class SCRAMClientSASL implements ClientSASL {
+   private final SCRAM scramType;
+   private final ScramClientFunctionalityImpl client;
+   private final String username;
+   private final String password;
+
+   /**
+    * @param scram the SCRAM mechanism to use
+    * @param username the username for authentication
+    * @param password the password for authentication
+    */
+   public SCRAMClientSASL(SCRAM scram, String username, String password) {
+      Objects.requireNonNull(scram);
+      Objects.requireNonNull(username);
+      Objects.requireNonNull(password);
+      this.username = username;
+      this.password = password;
+      this.scramType = scram;
+      client = new ScramClientFunctionalityImpl(scram.getDigest(), 
scram.getHmac(), UUID.randomUUID().toString());
+   }
+
+   @Override
+   public String getName() {
+      return scramType.getName();
+   }
+
+   @Override
+   public byte[] getInitialResponse() {
+      try {
+         String firstMessage = client.prepareFirstMessage(username);
+         System.out.println("SCRAMClientSASL.getInitialResponse() >> " + 
firstMessage);
+         return firstMessage.getBytes(StandardCharsets.US_ASCII);
+      } catch (ScramException e) {
+         throw new DecodeException("prepareFirstMessage failed", e);
+      }
+   }
+
+   @Override
+   public byte[] getResponse(byte[] challenge) {
+      String msg = new String(challenge, StandardCharsets.US_ASCII);
+      System.out.println("SCRAMClientSASL.getResponse() << " + msg);
+      if (client.getState() == State.FIRST_PREPARED) {
+         try {
+            String finalMessage = client.prepareFinalMessage(password, msg);
+            System.out.println("SCRAMClientSASL.getResponse() >> " + 
finalMessage);

Review comment:
       Leftover System.out.println

##########
File path: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AMQPConnectSaslTest.java
##########
@@ -151,6 +164,34 @@ public void testConnectsWithPlain() throws Exception {
       assertArrayEquals(expectedPlainInitialResponse(USER, PASSWD), 
authenticator.getInitialResponse());
    }
 
+   @Test(timeout = 200000)
+   public void testConnectsWithSCRAM() throws Exception {
+      CountDownLatch serverConnectionOpen = new CountDownLatch(1);
+      SCRAMTestAuthenticator authenticator = new 
SCRAMTestAuthenticator(SCRAM.SHA512);

Review comment:
       A better test would be to also pass which mechanisms to offer and offer 
some others (e.g 512, 256, and PLAIN), then you have also verified it prefers 
selecting 512 when offered.

##########
File path: 
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/SCRAMPropertiesLoginModule.java
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.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;
+import org.jgroups.util.UUID;
+
+/**
+ * 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_MECHANISM = "|";
+   private static final String SEPARATOR_PARAMETER = ":";
+   private static final int MIN_ITERATIONS = 4096;
+   private static final SecureRandom RANDOM_GENERATOR = new SecureRandom();
+   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(nameCallback);
+      user = nameCallback.getName();
+      if (user == null) {
+         user = UUID.randomUUID().toString();
+      }
+      SCRAMMechanismCallback mechanismCallback = new SCRAMMechanismCallback();
+      executeCallbacks(mechanismCallback);
+      SCRAM scram = getTypeByString(mechanismCallback.getMechanism());
+      String password = users.getProperty(user, users.getProperty(user + 
SEPARATOR_MECHANISM + scram.name()));

Review comment:
       Seems odd to go with the less specific one if both exist (though they of 
course shouldn't really)...especially so as it will already have had to look up 
the more-specific value first in order to pass it in as the default. Perhaps 
flip and only do the less specific lookup if 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.

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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 571339)
    Time Spent: 13.5h  (was: 13h 20m)

> 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: 13.5h
>  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)

Reply via email to