Skip to content

Commit

Permalink
Merge pull request from GHSA-3fqm-frhg-7c85
Browse files Browse the repository at this point in the history 
- Clear session from cache on all nodes after deletion
- Add changelog

Co-authored-by: Othello Maurer <othello@graylog.com>
  • Loading branch information
@bernd @thll
bernd and thll committed Jul 5, 2023
1 parent 466af81 commit ff90f3e
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 31 deletions.
2 changes: 2 additions & 0 deletions changelog/unreleased/ghsa-3fqm-frhg-7c85.toml
View file
@@ -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)"
43 changes: 26 additions & 17 deletions graylog2-server/src/main/java/org/graylog2/rest/resources/users/UsersResource.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, SearchQueryField> SEARCH_FIELD_MAPPING = ImmutableMap.<String, SearchQueryField>builder()
.put(UserOverviewDTO.FIELD_ID, SearchQueryField.create("_id", SearchQueryField.Type.OBJECT_ID))
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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<Session> 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<String> roles) {
Expand Down
15 changes: 14 additions & 1 deletion graylog2-server/src/main/java/org/graylog2/security/MongoDBSessionServiceImpl.java
View file
Expand Up @@ -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;
Expand All @@ -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<Document> sessions = database.getCollection(MongoDbSession.COLLECTION_NAME);
Expand Down Expand Up @@ -84,4 +89,12 @@ public SimpleSession daoToSimpleSession(MongoDbSession sessionDAO) {
return session;
}

@Override
public <T extends Persisted> int destroy(T model) {
int affectedDocs = super.destroy(model);
if (affectedDocs != 0 && model instanceof MongoDbSession session) {
eventBus.post(new SessionDeletedEvent(session.getSessionId()));
}
return affectedDocs;
}
}
15 changes: 14 additions & 1 deletion graylog2-server/src/main/java/org/graylog2/security/MongoDbSessionDAO.java
View file
Expand Up @@ -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;
Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/security/SessionDeletedEvent.java
View file
@@ -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
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
package org.graylog2.security;

import com.fasterxml.jackson.annotation.JsonProperty;

public record SessionDeletedEvent(@JsonProperty("session_id") String sessionId) {
}
15 changes: 8 additions & 7 deletions graylog2-server/src/main/java/org/graylog2/security/UserSessionTerminationService.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -140,15 +141,15 @@ public void runGlobalSessionTermination() {
clusterConfigService.write(GlobalTerminationRevisionConfig.withCurrentRevision());
}

public List<Session> getActiveSessionsForUser(User user) {
return getSessionIDsForUser(user).stream().map(this::getActiveSessionForID).flatMap(Optional::stream).toList();
}

private void terminateSessionsForUser(User user) {
try {
final Set<String> 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);
Expand Down
17 changes: 13 additions & 4 deletions graylog2-server/src/test/java/org/graylog2/rest/resources/users/UsersResourceTest.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 12 additions & 1 deletion graylog2-web-interface/src/components/users/TimeoutInput.tsx
View file
Expand Up @@ -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';

Expand Down Expand Up @@ -82,6 +83,16 @@ const TimeoutInput = ({ value: propsValue, onChange }: Props) => {
labelClassName="col-sm-3"
wrapperClassName="col-sm-9"
label="Sessions Timeout">
<Row className="no-bm">
<Col xs={12}>
<Alert bsStyle="info">
<Icon name="info-circle" />{' '}<b>Changing the session timeout</b><br />
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.
</Alert>
</Col>
</Row>
<>
<Input type="checkbox"
id="session-timeout-never"
Expand Down

0 comments on commit ff90f3e

Please sign in to comment.