From bac579ac253ef0566adb2316264c8abab0b82272 Mon Sep 17 00:00:00 2001 From: Domenico Francesco Bruscino Date: Mon, 7 Feb 2022 21:46:10 +0100 Subject: [PATCH] ARTEMIS-3573 Support PropertiesLoginModule custom password codecs --- .../utils/DefaultSensitiveStringCodec.java | 12 ++++ .../artemis/utils/LazyHashProcessor.java | 45 ++++++++++++++ .../artemis/utils/PasswordMaskingUtil.java | 7 +++ .../artemis/utils/SecureHashProcessor.java | 4 +- .../artemis/utils/SensitiveDataCodec.java | 8 ++- .../DefaultSensitiveStringCodecTest.java | 62 ++++++++++++++----- .../security/jaas/PropertiesLoginModule.java | 18 +++++- docs/user-manual/en/masking-passwords.md | 45 +++++++++----- docs/user-manual/en/security.md | 4 ++ .../integration/security/SecurityTest.java | 34 ++++++++++ .../src/test/resources/login.config | 7 +++ .../src/test/resources/roles.properties | 1 + .../src/test/resources/users.properties | 1 + 13 files changed, 208 insertions(+), 40 deletions(-) create mode 100644 artemis-commons/src/main/java/org/apache/activemq/artemis/utils/LazyHashProcessor.java diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodec.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodec.java index 83c2ba7b25..6f07a7b80b 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodec.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodec.java @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Properties; import org.jboss.logging.Logger; @@ -105,6 +106,7 @@ public class DefaultSensitiveStringCodec implements SensitiveDataCodec { System.out.println("Encoded password (without quotes): \"" + encode + "\""); } + @Override public boolean verify(char[] inputValue, String storedValue) { return algorithm.verify(inputValue, storedValue); } @@ -188,6 +190,16 @@ public class DefaultSensitiveStringCodec implements SensitiveDataCodec { BigInteger n = new BigInteger(encoding); return n.toString(16); } + + @Override + public boolean verify(char[] inputValue, String storedValue) { + try { + return Objects.equals(storedValue, encode(String.valueOf(inputValue))); + } catch (Exception e) { + logger.debug("Exception on verifying: " + e); + return false; + } + } } private class PBKDF2Algorithm extends CodecAlgorithm { diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/LazyHashProcessor.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/LazyHashProcessor.java new file mode 100644 index 0000000000..cfde12d853 --- /dev/null +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/LazyHashProcessor.java @@ -0,0 +1,45 @@ +/** + * 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.utils; + +import java.util.function.Supplier; + +public abstract class LazyHashProcessor implements HashProcessor { + + private Supplier hashProcessorSupplier = Suppliers.memoize(() -> { + try { + return createHashProcessor(); + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + @Override + public String hash(String plainText) throws Exception { + return hashProcessorSupplier.get().hash(plainText); + } + + @Override + public boolean compare(char[] inputValue, String storedHash) { + return hashProcessorSupplier.get().compare(inputValue, storedHash); + } + + protected abstract HashProcessor createHashProcessor() throws Exception; +} diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/PasswordMaskingUtil.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/PasswordMaskingUtil.java index 4c64a450f5..2287e5303d 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/PasswordMaskingUtil.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/PasswordMaskingUtil.java @@ -122,10 +122,17 @@ public final class PasswordMaskingUtil { //stored password takes 2 forms, ENC() or plain text public static HashProcessor getHashProcessor(String storedPassword) { + return getHashProcessor(storedPassword, null); + } + + public static HashProcessor getHashProcessor(String storedPassword, HashProcessor secureHashProcessor) { if (!isEncoded(storedPassword)) { return LazyPlainTextProcessorHolder.INSTANCE; } + if (secureHashProcessor != null) { + return secureHashProcessor; + } final Exception secureProcessorException = LazySecureProcessorHolder.EXCEPTION; if (secureProcessorException != null) { //reuse old descriptions/messages of the exception but refill the stack trace diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SecureHashProcessor.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SecureHashProcessor.java index a17cfd4a2e..544b0b0645 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SecureHashProcessor.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SecureHashProcessor.java @@ -18,9 +18,9 @@ package org.apache.activemq.artemis.utils; public class SecureHashProcessor implements HashProcessor { - private DefaultSensitiveStringCodec codec; + private SensitiveDataCodec codec; - public SecureHashProcessor(DefaultSensitiveStringCodec codec) { + public SecureHashProcessor(SensitiveDataCodec codec) { this.codec = codec; } diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SensitiveDataCodec.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SensitiveDataCodec.java index 5f3c08a861..015af65c0f 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SensitiveDataCodec.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SensitiveDataCodec.java @@ -27,10 +27,14 @@ import java.util.Map; */ public interface SensitiveDataCodec { - T decode(Object mask) throws Exception; + T decode(Object encodedValue) throws Exception; - T encode(Object secret) throws Exception; + T encode(Object value) throws Exception; default void init(Map params) throws Exception { } + + default boolean verify(char[] value, T encodedValue) { + return false; + } } diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodecTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodecTest.java index 9a97454b04..c670345fa0 100644 --- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodecTest.java +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodecTest.java @@ -39,20 +39,33 @@ public class DefaultSensitiveStringCodecTest { @Test public void testOnewayAlgorithm() throws Exception { - DefaultSensitiveStringCodec codec = new DefaultSensitiveStringCodec(); - Map params = new HashMap<>(); - params.put(DefaultSensitiveStringCodec.ALGORITHM, DefaultSensitiveStringCodec.ONE_WAY); - codec.init(params); + testAlgorithm(DefaultSensitiveStringCodec.ONE_WAY); + } + + @Test + public void testTwowayAlgorithm() throws Exception { + testAlgorithm(DefaultSensitiveStringCodec.TWO_WAY); + } + + private void testAlgorithm(String algorithm) throws Exception { + DefaultSensitiveStringCodec codec = getDefaultSensitiveStringCodec(algorithm); String plainText = "some_password"; String maskedText = codec.encode(plainText); log.debug("encoded value: " + maskedText); - //one way can't decode - try { - codec.decode(maskedText); - fail("one way algorithm can't decode"); - } catch (IllegalArgumentException expected) { + if (algorithm.equals(DefaultSensitiveStringCodec.ONE_WAY)) { + //one way can't decode + try { + codec.decode(maskedText); + fail("one way algorithm can't decode"); + } catch (IllegalArgumentException expected) { + } + } else { + String decoded = codec.decode(maskedText); + log.debug("encoded value: " + maskedText); + + assertEquals("decoded result not match: " + decoded, decoded, plainText); } assertTrue(codec.verify(plainText.toCharArray(), maskedText)); @@ -62,19 +75,34 @@ public class DefaultSensitiveStringCodecTest { } @Test - public void testTwowayAlgorithm() throws Exception { - DefaultSensitiveStringCodec codec = new DefaultSensitiveStringCodec(); - Map params = new HashMap<>(); - params.put(DefaultSensitiveStringCodec.ALGORITHM, DefaultSensitiveStringCodec.TWO_WAY); - codec.init(params); + public void testCompareWithOnewayAlgorithm() throws Exception { + testCompareWithAlgorithm(DefaultSensitiveStringCodec.ONE_WAY); + } + + @Test + public void testCompareWithTwowayAlgorithm() throws Exception { + testCompareWithAlgorithm(DefaultSensitiveStringCodec.TWO_WAY); + } + + private void testCompareWithAlgorithm(String algorithm) throws Exception { + DefaultSensitiveStringCodec codec = getDefaultSensitiveStringCodec(algorithm); String plainText = "some_password"; String maskedText = codec.encode(plainText); log.debug("encoded value: " + maskedText); - String decoded = codec.decode(maskedText); - log.debug("encoded value: " + maskedText); + assertTrue(codec.verify(plainText.toCharArray(), maskedText)); - assertEquals("decoded result not match: " + decoded, decoded, plainText); + String otherPassword = "some_other_password"; + assertFalse(codec.verify(otherPassword.toCharArray(), maskedText)); + } + + private DefaultSensitiveStringCodec getDefaultSensitiveStringCodec(String algorithm) throws Exception { + DefaultSensitiveStringCodec codec = new DefaultSensitiveStringCodec(); + Map params = new HashMap<>(); + params.put(DefaultSensitiveStringCodec.ALGORITHM, algorithm); + codec.init(params); + + return codec; } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java index e021b5bc35..79af7f34b4 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java @@ -32,7 +32,9 @@ import java.util.Properties; import java.util.Set; import org.apache.activemq.artemis.utils.HashProcessor; +import org.apache.activemq.artemis.utils.LazyHashProcessor; import org.apache.activemq.artemis.utils.PasswordMaskingUtil; +import org.apache.activemq.artemis.utils.SecureHashProcessor; import org.jboss.logging.Logger; public class PropertiesLoginModule extends PropertiesLoader implements AuditLoginModule { @@ -41,6 +43,7 @@ public class PropertiesLoginModule extends PropertiesLoader implements AuditLogi public static final String USER_FILE_PROP_NAME = "org.apache.activemq.jaas.properties.user"; public static final String ROLE_FILE_PROP_NAME = "org.apache.activemq.jaas.properties.role"; + public static final String PASSWORD_CODEC_PROP_NAME = "org.apache.activemq.jaas.properties.password.codec"; private Subject subject; private CallbackHandler callbackHandler; @@ -64,10 +67,21 @@ public class PropertiesLoginModule extends PropertiesLoader implements AuditLogi init(options); users = load(USER_FILE_PROP_NAME, "user", options).getProps(); roles = load(ROLE_FILE_PROP_NAME, "role", options).invertedPropertiesValuesMap(); + + String passwordCodec = (String)options.get(PASSWORD_CODEC_PROP_NAME); + if (passwordCodec != null) { + hashProcessor = new LazyHashProcessor() { + @Override + protected HashProcessor createHashProcessor() throws Exception { + return new SecureHashProcessor(PasswordMaskingUtil.getCodec(passwordCodec)); + } + }; + } } @Override public boolean login() throws LoginException { + HashProcessor userHashProcessor; Callback[] callbacks = new Callback[2]; callbacks[0] = new NameCallback("Username: "); @@ -94,12 +108,12 @@ public class PropertiesLoginModule extends PropertiesLoader implements AuditLogi } try { - hashProcessor = PasswordMaskingUtil.getHashProcessor(password); + userHashProcessor = PasswordMaskingUtil.getHashProcessor(password, hashProcessor); } catch (Exception e) { throw new FailedLoginException("Failed to get hash processor"); } - if (!hashProcessor.compare(tmpPassword, password)) { + if (!userHashProcessor.compare(tmpPassword, password)) { throw new FailedLoginException("Password does not match for user: " + user); } loginSucceeded = true; diff --git a/docs/user-manual/en/masking-passwords.md b/docs/user-manual/en/masking-passwords.md index d104823343..24400caa87 100644 --- a/docs/user-manual/en/masking-passwords.md +++ b/docs/user-manual/en/masking-passwords.md @@ -259,7 +259,26 @@ codec other than the default one. For example With this configuration, both passwords in ra.xml and all of its MDBs will have to be in masked form. -### login.config +### PropertiesLoginModule +Artemis supports Properties login module to be configured in JAAS configuration file +(default name is `login.config`). By default, the passwords of the users are in plain text +or masked with the [the default codec](#the-default-codec). + +To use a custom codec class, set the `org.apache.activemq.jaas.properties.password.codec` property to the class name +e.g. to use the `com.example.MySensitiveDataCodecImpl` codec class: + +``` +PropertiesLoginWithPasswordCodec { + org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModule required + debug=true + org.apache.activemq.jaas.properties.user="users.properties" + org.apache.activemq.jaas.properties.role="roles.properties" + org.apache.activemq.jaas.properties.password.codec="com.example.MySensitiveDataCodecImpl"; +}; +``` + + +### LDAPLoginModule Artemis supports LDAP login modules to be configured in JAAS configuration file (default name is `login.config`). When connecting to an LDAP server usually you @@ -429,22 +448,8 @@ using the new defined codec. To use a different codec than the built-in one, you either pick one from existing libraries or you implement it yourself. All codecs must implement -the `org.apache.activemq.artemis.utils.SensitiveDataCodec` interface: - -```java -public interface SensitiveDataCodec { - - T decode(Object mask) throws Exception; - - T encode(Object secret) throws Exception; - - default void init(Map params) throws Exception { - }; -} -``` - -This is a generic type interface but normally for a password you just need -String type. So a new codec would be defined like +the `org.apache.activemq.artemis.utils.SensitiveDataCodec` interface. +So a new codec would be defined like ```java public class MyCodec implements SensitiveDataCodec { @@ -464,6 +469,12 @@ public class MyCodec implements SensitiveDataCodec { public void init(Map params) { // Initialization done here. It is called right after the codec has been created. } + + @Override + public boolean verify(char[] value, String encodedValue) { + // Return true if the value matches the encodedValue. + return checkValueMatchesEncoding(value, encodedValue); + } } ``` diff --git a/docs/user-manual/en/security.md b/docs/user-manual/en/security.md index 8c8e881f75..f5a3ea5fef 100644 --- a/docs/user-manual/en/security.md +++ b/docs/user-manual/en/security.md @@ -567,6 +567,10 @@ integration with LDAP is preferable. It is implemented by - `org.apache.activemq.jaas.properties.role` - the path to the file which contains user and role properties +- `org.apache.activemq.jaas.properties.password.codec` - the fully qualified + class name of the password codec to use. See the [password masking](masking-passwords.md) + documentation for more details on how this works. + - `reload` - boolean flag; whether or not to reload the properties files when a modification occurs; default is `false` diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java index 45b1082ac3..ba33949184 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java @@ -71,6 +71,7 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager4; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.apache.activemq.artemis.tests.util.CreateMessage; import org.apache.activemq.artemis.utils.CompositeAddress; +import org.apache.activemq.artemis.utils.SensitiveDataCodec; import org.apache.activemq.artemis.utils.Wait; import org.apache.activemq.command.ActiveMQQueue; import org.junit.Assert; @@ -129,6 +130,22 @@ public class SecurityTest extends ActiveMQTestBase { } } + @Test + public void testJAASSecurityManagerAuthenticationWithPasswordCodec() throws Exception { + ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLoginWithPasswordCodec"); + ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(createDefaultInVMConfig().setSecurityEnabled(true), ManagementFactory.getPlatformMBeanServer(), securityManager, false)); + server.start(); + ClientSessionFactory cf = createSessionFactory(locator); + + try { + ClientSession session = cf.createSession("test","password", false, true, true, false, 0); + session.close(); + } catch (ActiveMQException e) { + e.printStackTrace(); + Assert.fail("should not throw exception"); + } + } + @Test public void testJAASSecurityManagerAuthenticationWithValidateUser() throws Exception { ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLogin"); @@ -2564,4 +2581,21 @@ public class SecurityTest extends ActiveMQTestBase { fail("Invalid Exception type:" + e.getType()); } } + + public static class DummySensitiveDataCodec implements SensitiveDataCodec { + @Override + public String decode(Object encodedValue) throws Exception { + throw new IllegalStateException("Decoding not supported"); + } + + @Override + public String encode(Object value) throws Exception { + return new StringBuffer((String)value).reverse().toString(); + } + + @Override + public boolean verify(char[] value, String encodedValue) { + return encodedValue.equals(new StringBuffer(String.valueOf(value)).reverse().toString()); + } + } } diff --git a/tests/integration-tests/src/test/resources/login.config b/tests/integration-tests/src/test/resources/login.config index b1af8253bf..f1fe8dfaba 100644 --- a/tests/integration-tests/src/test/resources/login.config +++ b/tests/integration-tests/src/test/resources/login.config @@ -21,6 +21,13 @@ PropertiesLogin { org.apache.activemq.jaas.properties.role="roles.properties"; }; +PropertiesLoginWithPasswordCodec { + org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModule required + debug=true + org.apache.activemq.jaas.properties.user="users.properties" + org.apache.activemq.jaas.properties.role="roles.properties" + org.apache.activemq.jaas.properties.password.codec="org.apache.activemq.artemis.tests.integration.security.SecurityTest$DummySensitiveDataCodec"; +}; LDAPLogin { org.apache.activemq.artemis.spi.core.security.jaas.LDAPLoginModule required diff --git a/tests/integration-tests/src/test/resources/roles.properties b/tests/integration-tests/src/test/resources/roles.properties index 31b0bd2168..41ca59d208 100644 --- a/tests/integration-tests/src/test/resources/roles.properties +++ b/tests/integration-tests/src/test/resources/roles.properties @@ -17,6 +17,7 @@ programmers=first accounting=second +test=test employees=first,second a=a b=b diff --git a/tests/integration-tests/src/test/resources/users.properties b/tests/integration-tests/src/test/resources/users.properties index d4c65e97b2..a275fad6bd 100644 --- a/tests/integration-tests/src/test/resources/users.properties +++ b/tests/integration-tests/src/test/resources/users.properties @@ -17,6 +17,7 @@ first=secret second=password +test=ENC(drowssap) a=a b=b x=x