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