diff --git a/changelog/unreleased/ghsa-3fqm-frhg-7c85.toml b/changelog/unreleased/ghsa-3fqm-frhg-7c85.toml new file mode 100644 index 000000000000..d39da8b7ea59 --- /dev/null +++ b/changelog/unreleased/ghsa-3fqm-frhg-7c85.toml @@ -0,0 +1,2 @@ +type = "security" +message = "Fix stale session cache after logout. [GHSA-3fqm-frhg-7c85](https://github.com/Graylog2/graylog2-server/security/advisories/GHSA-3fqm-frhg-7c85)" diff --git a/graylog2-server/src/main/java/org/graylog2/rest/resources/users/UsersResource.java b/graylog2-server/src/main/java/org/graylog2/rest/resources/users/UsersResource.java index 9c9a3a3dfddf..56defe978e66 100644 --- a/graylog2-server/src/main/java/org/graylog2/rest/resources/users/UsersResource.java +++ b/graylog2-server/src/main/java/org/graylog2/rest/resources/users/UsersResource.java @@ -28,14 +28,12 @@ import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; import org.apache.commons.lang.StringUtils; -import org.apache.shiro.SecurityUtils; import org.apache.shiro.authz.annotation.RequiresAuthentication; import org.apache.shiro.authz.annotation.RequiresPermissions; import org.apache.shiro.authz.permission.WildcardPermission; import org.apache.shiro.mgt.DefaultSecurityManager; import org.apache.shiro.session.Session; -import org.apache.shiro.session.mgt.DefaultSessionManager; -import org.apache.shiro.session.mgt.eis.SessionDAO; +import org.apache.shiro.subject.Subject; import org.graylog.security.UserContext; import org.graylog.security.permissions.GRNPermission; import org.graylog2.audit.AuditEventTypes; @@ -61,6 +59,7 @@ import org.graylog2.security.AccessTokenService; import org.graylog2.security.MongoDBSessionService; import org.graylog2.security.MongoDbSession; +import org.graylog2.security.UserSessionTerminationService; import org.graylog2.shared.rest.resources.RestResource; import org.graylog2.shared.security.RestPermissions; import org.graylog2.shared.users.ChangeUserRequest; @@ -137,6 +136,8 @@ public class UsersResource extends RestResource { private final RoleService roleService; private final MongoDBSessionService sessionService; private final SearchQueryParser searchQueryParser; + private final UserSessionTerminationService sessionTerminationService; + private final DefaultSecurityManager securityManager; protected static final ImmutableMap SEARCH_FIELD_MAPPING = ImmutableMap.builder() .put(UserOverviewDTO.FIELD_ID, SearchQueryField.create("_id", SearchQueryField.Type.OBJECT_ID)) @@ -150,12 +151,15 @@ public UsersResource(UserManagementService userManagementService, PaginatedUserService paginatedUserService, AccessTokenService accessTokenService, RoleService roleService, - MongoDBSessionService sessionService) { + MongoDBSessionService sessionService, + UserSessionTerminationService sessionTerminationService, DefaultSecurityManager securityManager) { this.userManagementService = userManagementService; this.accessTokenService = accessTokenService; this.roleService = roleService; this.sessionService = sessionService; this.paginatedUserService = paginatedUserService; + this.sessionTerminationService = sessionTerminationService; + this.securityManager = securityManager; this.searchQueryParser = new SearchQueryParser(UserOverviewDTO.FIELD_FULL_NAME, SEARCH_FIELD_MAPPING); } @@ -439,8 +443,8 @@ public void changeUser(@ApiParam(name = "userId", value = "The ID of the user to if (isPermitted("*")) { final Long sessionTimeoutMs = cr.sessionTimeoutMs(); if (Objects.nonNull(sessionTimeoutMs) && sessionTimeoutMs != 0 && (user.getSessionTimeoutMs() != sessionTimeoutMs)) { - updateExistingSession(user, sessionTimeoutMs); - user.setSessionTimeoutMs(sessionTimeoutMs); + user.setSessionTimeoutMs(sessionTimeoutMs); + terminateSessions(user); } } @@ -451,17 +455,22 @@ public void changeUser(@ApiParam(name = "userId", value = "The ID of the user to userManagementService.update(user, cr); } - private void updateExistingSession(User user, long newSessionTimeOut) { - AllUserSessions allUserSessions = AllUserSessions.create(sessionService); - allUserSessions.forUser(user).ifPresent(userSession -> { - userSession.setTimeout(newSessionTimeOut); - Session session = sessionService.daoToSimpleSession(userSession); - - DefaultSecurityManager securityManager = (DefaultSecurityManager) SecurityUtils.getSecurityManager(); - DefaultSessionManager sessionManager = (DefaultSessionManager) securityManager.getSessionManager(); - SessionDAO sessionDAO = sessionManager.getSessionDAO(); - sessionDAO.update(session); - }); + private void terminateSessions(User user) { + final List allSessions = sessionTerminationService.getActiveSessionsForUser(user); + + final Subject subject = getSubject(); + final Session currentSession = subject.getSession(false); + final User currentUser = getCurrentUser(); + + if (currentSession != null && currentUser != null && user.getId().equals(currentUser.getId())) { + // Stop all sessions but handle the current session differently by issuing a proper logout + allSessions.stream() + .filter(session -> !session.getId().equals(currentSession.getId())) + .forEach(Session::stop); + securityManager.logout(subject); + } else { + allSessions.forEach(Session::stop); + } } private boolean rolesContainAdmin(List roles) { diff --git a/graylog2-server/src/main/java/org/graylog2/security/MongoDBSessionServiceImpl.java b/graylog2-server/src/main/java/org/graylog2/security/MongoDBSessionServiceImpl.java index 0023d8bca138..30359926dd10 100644 --- a/graylog2-server/src/main/java/org/graylog2/security/MongoDBSessionServiceImpl.java +++ b/graylog2-server/src/main/java/org/graylog2/security/MongoDBSessionServiceImpl.java @@ -27,6 +27,8 @@ import org.bson.types.ObjectId; import org.graylog2.database.MongoConnection; import org.graylog2.database.PersistedServiceImpl; +import org.graylog2.events.ClusterEventBus; +import org.graylog2.plugin.database.Persisted; import javax.annotation.Nullable; import javax.inject.Inject; @@ -36,9 +38,12 @@ @Singleton public class MongoDBSessionServiceImpl extends PersistedServiceImpl implements MongoDBSessionService { + private final ClusterEventBus eventBus; + @Inject - public MongoDBSessionServiceImpl(MongoConnection mongoConnection) { + public MongoDBSessionServiceImpl(MongoConnection mongoConnection, ClusterEventBus clusterEventBus) { super(mongoConnection); + this.eventBus = clusterEventBus; final MongoDatabase database = mongoConnection.getMongoDatabase(); final MongoCollection sessions = database.getCollection(MongoDbSession.COLLECTION_NAME); @@ -84,4 +89,12 @@ public SimpleSession daoToSimpleSession(MongoDbSession sessionDAO) { return session; } + @Override + public int destroy(T model) { + int affectedDocs = super.destroy(model); + if (affectedDocs != 0 && model instanceof MongoDbSession session) { + eventBus.post(new SessionDeletedEvent(session.getSessionId())); + } + return affectedDocs; + } } diff --git a/graylog2-server/src/main/java/org/graylog2/security/MongoDbSessionDAO.java b/graylog2-server/src/main/java/org/graylog2/security/MongoDbSessionDAO.java index 7b5a8b6fcf20..508940961eb0 100644 --- a/graylog2-server/src/main/java/org/graylog2/security/MongoDbSessionDAO.java +++ b/graylog2-server/src/main/java/org/graylog2/security/MongoDbSessionDAO.java @@ -23,6 +23,8 @@ import com.github.rholder.retry.WaitStrategies; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.eventbus.EventBus; +import com.google.common.eventbus.Subscribe; import com.mongodb.DuplicateKeyException; import org.apache.shiro.session.Session; import org.apache.shiro.session.mgt.SimpleSession; @@ -44,8 +46,19 @@ public class MongoDbSessionDAO extends CachingSessionDAO { private final MongoDBSessionService mongoDBSessionService; @Inject - public MongoDbSessionDAO(MongoDBSessionService mongoDBSessionService) { + public MongoDbSessionDAO(MongoDBSessionService mongoDBSessionService, EventBus eventBus) { this.mongoDBSessionService = mongoDBSessionService; + eventBus.register(this); + } + + @SuppressWarnings("unused") + @Subscribe + public void sessionDeleted(SessionDeletedEvent event) { + final Session cachedSession = getCachedSession(event.sessionId()); + if (cachedSession != null) { + LOG.debug("Removing deleted session from cache."); + uncache(cachedSession); + } } @Override diff --git a/graylog2-server/src/main/java/org/graylog2/security/SessionDeletedEvent.java b/graylog2-server/src/main/java/org/graylog2/security/SessionDeletedEvent.java new file mode 100644 index 000000000000..0b3b6f45be51 --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog2/security/SessionDeletedEvent.java @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog2.security; + +import com.fasterxml.jackson.annotation.JsonProperty; + +public record SessionDeletedEvent(@JsonProperty("session_id") String sessionId) { +} diff --git a/graylog2-server/src/main/java/org/graylog2/security/UserSessionTerminationService.java b/graylog2-server/src/main/java/org/graylog2/security/UserSessionTerminationService.java index 69efd09f6461..5473770bebeb 100644 --- a/graylog2-server/src/main/java/org/graylog2/security/UserSessionTerminationService.java +++ b/graylog2-server/src/main/java/org/graylog2/security/UserSessionTerminationService.java @@ -39,6 +39,7 @@ import javax.inject.Singleton; import java.io.Serializable; import java.util.EnumSet; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -140,15 +141,15 @@ public void runGlobalSessionTermination() { clusterConfigService.write(GlobalTerminationRevisionConfig.withCurrentRevision()); } + public List getActiveSessionsForUser(User user) { + return getSessionIDsForUser(user).stream().map(this::getActiveSessionForID).flatMap(Optional::stream).toList(); + } + private void terminateSessionsForUser(User user) { try { - final Set sessionIds = getSessionIDsForUser(user); - - for (final String sessionId : sessionIds) { - getActiveSessionForID(sessionId).ifPresent(session -> { - LOG.info("Terminating session for user <{}/{}>", user.getName(), user.getId()); - session.stop(); - }); + for (final Session session : getActiveSessionsForUser(user)) { + LOG.info("Terminating session for user <{}/{}>", user.getName(), user.getId()); + session.stop(); } } catch (Exception e) { LOG.error("Couldn't terminate session for user <{}/{}>", user.getName(), user.getId(), e); diff --git a/graylog2-server/src/test/java/org/graylog2/rest/resources/users/UsersResourceTest.java b/graylog2-server/src/test/java/org/graylog2/rest/resources/users/UsersResourceTest.java index 4da05ff0e76b..4cc24cbb6a1a 100644 --- a/graylog2-server/src/test/java/org/graylog2/rest/resources/users/UsersResourceTest.java +++ b/graylog2-server/src/test/java/org/graylog2/rest/resources/users/UsersResourceTest.java @@ -17,6 +17,7 @@ package org.graylog2.rest.resources.users; import com.google.common.collect.ImmutableSet; +import org.apache.shiro.mgt.DefaultSecurityManager; import org.apache.shiro.subject.Subject; import org.bson.types.ObjectId; import org.graylog.testing.mongodb.MongoDBInstance; @@ -30,6 +31,7 @@ import org.graylog2.security.AccessTokenService; import org.graylog2.security.MongoDBSessionService; import org.graylog2.security.PasswordAlgorithmFactory; +import org.graylog2.security.UserSessionTerminationService; import org.graylog2.security.hashing.SHA1HashPasswordAlgorithm; import org.graylog2.shared.security.Permissions; import org.graylog2.shared.security.RestPermissions; @@ -90,15 +92,20 @@ public class UsersResourceTest { private Subject subject; @Mock private UserManagementService userManagementService; + @Mock + private UserSessionTerminationService sessionTerminationService; + @Mock + private DefaultSecurityManager securityManager; UserImplFactory userImplFactory; @Before public void setUp() throws Exception { userImplFactory = new UserImplFactory(new Configuration(), - new Permissions(ImmutableSet.of(new RestPermissions()))); + new Permissions(ImmutableSet.of(new RestPermissions()))); usersResource = new TestUsersResource(userManagementService, paginatedUserService, accessTokenService, - roleService, sessionService, new HttpConfiguration(), subject); + roleService, sessionService, new HttpConfiguration(), subject, + sessionTerminationService, securityManager); } /** @@ -139,8 +146,10 @@ public static class TestUsersResource extends UsersResource { public TestUsersResource(UserManagementService userManagementService, PaginatedUserService paginatedUserService, AccessTokenService accessTokenService, RoleService roleService, MongoDBSessionService sessionService, HttpConfiguration configuration, - Subject subject) { - super(userManagementService, paginatedUserService, accessTokenService, roleService, sessionService); + Subject subject, UserSessionTerminationService sessionTerminationService, + DefaultSecurityManager securityManager) { + super(userManagementService, paginatedUserService, accessTokenService, roleService, sessionService, + sessionTerminationService, securityManager); this.subject = subject; super.configuration = configuration; } diff --git a/graylog2-web-interface/src/components/users/TimeoutInput.tsx b/graylog2-web-interface/src/components/users/TimeoutInput.tsx index f19aa66925d4..ab1a959bf23b 100644 --- a/graylog2-web-interface/src/components/users/TimeoutInput.tsx +++ b/graylog2-web-interface/src/components/users/TimeoutInput.tsx @@ -18,8 +18,9 @@ import * as React from 'react'; import { useState } from 'react'; import PropTypes from 'prop-types'; -import { Row, Col, HelpBlock, Input } from 'components/bootstrap'; +import { Row, Col, HelpBlock, Input, Alert } from 'components/bootstrap'; import TimeoutUnitSelect from 'components/users/TimeoutUnitSelect'; +import { Icon } from 'components/common'; import { MS_DAY, MS_HOUR, MS_MINUTE, MS_SECOND } from './timeoutConstants'; @@ -82,6 +83,16 @@ const TimeoutInput = ({ value: propsValue, onChange }: Props) => { labelClassName="col-sm-3" wrapperClassName="col-sm-9" label="Sessions Timeout"> + + + + {' '}Changing the session timeout
+ Changing the timeout setting for sessions will log the user out of Graylog and will invalidate all their + current sessions. If you are changing the setting for your own user, you will be logged out at the moment + of saving the setting. In that case, make sure to save any pending changes before changing the timeout. + + + <>