This is an automated email from the ASF dual-hosted git repository.
robbie pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 29fafb5fed ARTEMIS-4399 fix disabled authn/z cache
29fafb5fed is described below
commit 29fafb5fed78788851273491f6f49638cbff828b
Author: Justin Bertram <[email protected]>
AuthorDate: Wed Aug 23 14:43:14 2023 -0500
ARTEMIS-4399 fix disabled authn/z cache
---
.../core/security/impl/SecurityStoreImpl.java | 101 +++++++++++++++++----
.../core/security/impl/SecurityStoreImplTest.java | 94 +++++++++++++++++++
2 files changed, 176 insertions(+), 19 deletions(-)
diff --git
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
index 14c393d032..35c4c87417 100644
---
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
+++
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
@@ -98,14 +98,22 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
this.managementClusterUser = managementClusterUser;
this.managementClusterPassword = managementClusterPassword;
this.notificationService = notificationService;
- authenticationCache = CacheBuilder.newBuilder()
- .maximumSize(authenticationCacheSize)
-
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
- .build();
- authorizationCache = CacheBuilder.newBuilder()
- .maximumSize(authorizationCacheSize)
- .expireAfterWrite(invalidationInterval,
TimeUnit.MILLISECONDS)
- .build();
+ if (authenticationCacheSize == 0) {
+ authenticationCache = null;
+ } else {
+ authenticationCache = CacheBuilder.newBuilder()
+
.maximumSize(authenticationCacheSize)
+
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
+ .build();
+ }
+ if (authorizationCacheSize == 0) {
+ authorizationCache = null;
+ } else {
+ authorizationCache = CacheBuilder.newBuilder()
+ .maximumSize(authorizationCacheSize)
+
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
+ .build();
+ }
this.securityRepository.registerListener(this);
}
@@ -159,7 +167,7 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
boolean check = true;
Subject subject = null;
- Pair<Boolean, Subject> cacheEntry =
authenticationCache.getIfPresent(createAuthenticationCacheKey(user, password,
connection));
+ Pair<Boolean, Subject> cacheEntry = getAuthenticationCacheEntry(user,
password, connection);
if (cacheEntry != null) {
if (!cacheEntry.getA()) {
// cached authentication failed previously so don't check again
@@ -186,7 +194,7 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
if (securityManager instanceof ActiveMQSecurityManager5) {
try {
subject = ((ActiveMQSecurityManager5)
securityManager).authenticate(user, password, connection, securityDomain);
- authenticationCache.put(createAuthenticationCacheKey(user,
password, connection), new Pair<>(subject != null, subject));
+ putAuthenticationCacheEntry(user, password, connection,
subject);
validatedUser = getUserFromSubject(subject);
} catch (NoCacheLoginException e) {
handleNoCacheLoginException(e);
@@ -313,12 +321,12 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
// if we get here we're granted, add to the cache
ConcurrentHashSet<SimpleString> set;
String key = createAuthorizationCacheKey(user, checkType);
- ConcurrentHashSet<SimpleString> act =
authorizationCache.getIfPresent(key);
+ ConcurrentHashSet<SimpleString> act = getAuthorizationCacheEntry(key);
if (act != null) {
set = act;
} else {
set = new ConcurrentHashSet<>();
- authorizationCache.put(key, set);
+ putAuthorizationCacheEntry(set, key);
}
set.add(fqqn != null ? fqqn : bareAddress);
}
@@ -394,7 +402,7 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
* @return the authenticated Subject with all associated role principals
*/
private Subject getSubjectForAuthorization(SecurityAuth auth,
ActiveMQSecurityManager5 securityManager) {
- Pair<Boolean, Subject> cached =
authenticationCache.getIfPresent(createAuthenticationCacheKey(auth.getUsername(),
auth.getPassword(), auth.getRemotingConnection()));
+ Pair<Boolean, Subject> cached =
getAuthenticationCacheEntry(auth.getUsername(), auth.getPassword(),
auth.getRemotingConnection());
if (cached == null && auth.getUsername() == null && auth.getPassword()
== null && auth.getRemotingConnection() instanceof
ManagementRemotingConnection) {
AccessControlContext accessControlContext =
AccessController.getContext();
@@ -410,7 +418,7 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
if (cached == null) {
try {
Subject subject = securityManager.authenticate(auth.getUsername(),
auth.getPassword(), auth.getRemotingConnection(), auth.getSecurityDomain());
-
authenticationCache.put(createAuthenticationCacheKey(auth.getUsername(),
auth.getPassword(), auth.getRemotingConnection()), new Pair<>(subject != null,
subject));
+ putAuthenticationCacheEntry(auth.getUsername(),
auth.getPassword(), auth.getRemotingConnection(), subject);
return subject;
} catch (NoCacheLoginException e) {
handleNoCacheLoginException(e);
@@ -424,26 +432,71 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
logger.debug("Skipping authentication cache due to exception: {}",
e.getMessage());
}
+ private void putAuthenticationCacheEntry(String user,
+ String password,
+ RemotingConnection connection,
+ Subject subject) {
+ if (authenticationCache != null) {
+ authenticationCache.put(createAuthenticationCacheKey(user, password,
connection), new Pair<>(subject != null, subject));
+ }
+ }
+
+ private Pair<Boolean, Subject> getAuthenticationCacheEntry(String user,
+ String password,
+
RemotingConnection connection) {
+ if (authenticationCache == null) {
+ return null;
+ } else {
+ return
authenticationCache.getIfPresent(createAuthenticationCacheKey(user, password,
connection));
+ }
+ }
+
+ private void putAuthorizationCacheEntry(ConcurrentHashSet<SimpleString>
set, String key) {
+ if (authorizationCache != null) {
+ authorizationCache.put(key, set);
+ }
+ }
+
+ private ConcurrentHashSet<SimpleString> getAuthorizationCacheEntry(String
key) {
+ if (authorizationCache == null) {
+ return null;
+ } else {
+ return authorizationCache.getIfPresent(key);
+ }
+ }
+
public void invalidateAuthorizationCache() {
- authorizationCache.invalidateAll();
+ if (authorizationCache != null) {
+ authorizationCache.invalidateAll();
+ }
}
public void invalidateAuthenticationCache() {
- authenticationCache.invalidateAll();
+ if (authenticationCache != null) {
+ authenticationCache.invalidateAll();
+ }
}
public long getAuthenticationCacheSize() {
- return authenticationCache.size();
+ if (authenticationCache == null) {
+ return 0;
+ } else {
+ return authenticationCache.size();
+ }
}
public long getAuthorizationCacheSize() {
- return authorizationCache.size();
+ if (authorizationCache == null) {
+ return 0;
+ } else {
+ return authorizationCache.size();
+ }
}
private boolean checkAuthorizationCache(final SimpleString dest, final
String user, final CheckType checkType) {
boolean granted = false;
- ConcurrentHashSet<SimpleString> act =
authorizationCache.getIfPresent(createAuthorizationCacheKey(user, checkType));
+ ConcurrentHashSet<SimpleString> act =
getAuthorizationCacheEntry(createAuthorizationCacheKey(user, checkType));
if (act != null) {
granted = act.contains(dest);
}
@@ -458,4 +511,14 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
private String createAuthorizationCacheKey(String user, CheckType
checkType) {
return user + "." + checkType.name();
}
+
+ // for testing
+ protected Cache<String, Pair<Boolean, Subject>> getAuthenticationCache() {
+ return authenticationCache;
+ }
+
+ // for testing
+ protected Cache<String, ConcurrentHashSet<SimpleString>>
getAuthorizationCache() {
+ return authorizationCache;
+ }
}
diff --git
a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
new file mode 100644
index 0000000000..ae22d1b812
--- /dev/null
+++
b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
@@ -0,0 +1,94 @@
+/*
+ * 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.core.security.impl;
+
+import javax.security.auth.Subject;
+import java.util.Set;
+
+import org.apache.activemq.artemis.core.security.CheckType;
+import org.apache.activemq.artemis.core.security.Role;
+import org.apache.activemq.artemis.core.security.SecurityAuth;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager5;
+import
org.apache.activemq.artemis.spi.core.security.jaas.NoCacheLoginException;
+import org.apache.activemq.artemis.utils.RandomUtil;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class SecurityStoreImplTest {
+
+ @Test
+ public void zeroCacheSizeTest() throws Exception {
+ ActiveMQSecurityManager5 securityManager = new
ActiveMQSecurityManager5() {
+ @Override
+ public Subject authenticate(String user,
+ String password,
+ RemotingConnection remotingConnection,
+ String securityDomain) throws
NoCacheLoginException {
+ return new Subject();
+ }
+
+ @Override
+ public boolean authorize(Subject subject, Set<Role> roles, CheckType
checkType, String address) {
+ return true;
+ }
+
+ @Override
+ public boolean validateUser(String user, String password) {
+ return false;
+ }
+
+ @Override
+ public boolean validateUserAndRole(String user, String password,
Set<Role> roles, CheckType checkType) {
+ return false;
+ }
+ };
+ SecurityStoreImpl securityStore = new SecurityStoreImpl(new
HierarchicalObjectRepository<>(), securityManager, 999, true, "", null, null,
0, 0);
+ assertNull(securityStore.getAuthenticationCache());
+ securityStore.authenticate(RandomUtil.randomString(),
RandomUtil.randomString(), null);
+ assertEquals(0, securityStore.getAuthenticationCacheSize());
+ securityStore.invalidateAuthenticationCache(); // ensure this doesn't
throw an NPE
+
+ assertNull(securityStore.getAuthorizationCache());
+ securityStore.check(RandomUtil.randomSimpleString(), CheckType.SEND, new
SecurityAuth() {
+ @Override
+ public String getUsername() {
+ return RandomUtil.randomString();
+ }
+
+ @Override
+ public String getPassword() {
+ return RandomUtil.randomString();
+ }
+
+ @Override
+ public RemotingConnection getRemotingConnection() {
+ return null;
+ }
+
+ @Override
+ public String getSecurityDomain() {
+ return null;
+ }
+ });
+ assertEquals(0, securityStore.getAuthorizationCacheSize());
+ securityStore.invalidateAuthorizationCache(); // ensure this doesn't
throw an NPE
+ }
+}
\ No newline at end of file
