Skip to content

Commit

Permalink
Restrict classes allowed for cluster config and event types (#18165)
Browse files Browse the repository at this point in the history
Add a new safe_classes configuration option to restrict the classes allowed to be used
as cluster config and event types.
The configuration option allows to specify a comma-separated set of prefixes matched
against the fully qualified class name.

For now, the default value for the configuration is org.graylog.,org.graylog2., which will
allow all classes that Graylog maintains.

This should work out of the box for almost all setups. Changing the default value might
only be necessary if external plugins require cluster config or event types outside the
"org.graylog." or "org.graylog2." namespaces. If that is the case, the configuration setting
can be adjusted to cover this use case, e.b. by setting it to

    safe_classes = org.graylog.,org.graylog2.,custom.plugin.namespace.

if said classes are located within the custom.plugin.namespace package.

Refs: GHSA-p6gg-5hf4-4rgj
  • Loading branch information
thll authored Feb 6, 2024
1 parent f273373 commit 8132032
Show file tree
Hide file tree
Showing 16 changed files with 426 additions and 29 deletions.
4 changes: 4 additions & 0 deletions changelog/unreleased/pr-18165.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type = "security"
message = "Restrict classes allowed for cluster config and event types. [GHSA-p6gg-5hf4-4rgj](https://github.com/Graylog2/graylog2-server/security/advisories/GHSA-p6gg-5hf4-4rgj)"

pulls = ["18165"]
11 changes: 10 additions & 1 deletion data-node/src/main/java/org/graylog/datanode/Configuration.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.github.joschi.jadconfig.ValidatorMethod;
import com.github.joschi.jadconfig.converters.IntegerConverter;
import com.github.joschi.jadconfig.converters.StringListConverter;
import com.github.joschi.jadconfig.converters.StringSetConverter;
import com.github.joschi.jadconfig.util.Duration;
import com.github.joschi.jadconfig.validators.PositiveIntegerValidator;
import com.github.joschi.jadconfig.validators.StringNotBlankValidator;
Expand All @@ -31,6 +32,7 @@
import com.google.common.net.InetAddresses;
import org.graylog.datanode.configuration.BaseConfiguration;
import org.graylog.datanode.configuration.DatanodeDirectories;
import org.graylog2.Configuration.SafeClassesValidator;
import org.graylog2.plugin.Tools;
import org.graylog2.shared.SuppressForbidden;
import org.joda.time.DateTimeZone;
Expand All @@ -50,6 +52,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;

/**
* Helper class to hold configuration of Graylog
Expand Down Expand Up @@ -213,9 +216,15 @@ public class Configuration extends BaseConfiguration {
@Parameter(value = "http_allow_embedding")
private boolean httpAllowEmbedding = false;

/**
* Classes considered safe to load by name. A set of prefixes matched against the fully qualified class name.
*/
@Parameter(value = org.graylog2.Configuration.SAFE_CLASSES, converter = StringSetConverter.class, validators = SafeClassesValidator.class)
private Set<String> safeClasses = Set.of("org.graylog.", "org.graylog2.");

@Parameter(value = "metrics_timestamp")
private String metricsTimestamp = "timestamp";

@Parameter(value = "metrics_stream")
private String metricsStream = "gl-datanode-metrics";

Expand Down
27 changes: 27 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/Configuration.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.github.joschi.jadconfig.validators.PositiveLongValidator;
import com.github.joschi.jadconfig.validators.StringNotBlankValidator;
import com.google.common.collect.Sets;
import org.apache.commons.lang3.StringUtils;
import org.graylog.plugins.views.search.engine.suggestions.FieldValueSuggestionMode;
import org.graylog.plugins.views.search.engine.suggestions.FieldValueSuggestionModeConverter;
import org.graylog.security.certutil.CaConfiguration;
Expand All @@ -50,11 +51,14 @@
import java.util.Collections;
import java.util.Set;

import static org.graylog2.shared.utilities.StringUtils.f;

/**
* Helper class to hold configuration of Graylog
*/
@SuppressWarnings("FieldMayBeFinal")
public class Configuration extends CaConfiguration {
public static final String SAFE_CLASSES = "safe_classes";

/**
* Deprecated! Use isLeader() instead.
Expand Down Expand Up @@ -231,6 +235,12 @@ public class Configuration extends CaConfiguration {
@Parameter(value = "minimum_auto_refresh_interval", required = true)
private Period minimumAutoRefreshInterval = Period.seconds(1);

/**
* Classes considered safe to load by name. A set of prefixes matched against the fully qualified class name.
*/
@Parameter(value = SAFE_CLASSES, converter = StringSetConverter.class, validators = SafeClassesValidator.class)
private Set<String> safeClasses = Set.of("org.graylog.", "org.graylog2.");

@Parameter(value = "field_value_suggestion_mode", required = true, converter = FieldValueSuggestionModeConverter.class)
private FieldValueSuggestionMode fieldValueSuggestionMode = FieldValueSuggestionMode.ON;

Expand Down Expand Up @@ -457,6 +467,10 @@ public Period getMinimumAutoRefreshInterval() {
return minimumAutoRefreshInterval;
}

public Set<String> getSafeClasses() {
return safeClasses;
}

public FieldValueSuggestionMode getFieldValueSuggestionMode() {
return fieldValueSuggestionMode;
}
Expand Down Expand Up @@ -573,6 +587,19 @@ public void validate(String name, String path) throws ValidationException {
}
}

public static class SafeClassesValidator implements Validator<Set<String>> {
@Override
public void validate(String name, Set<String> set) throws ValidationException {
if (set.isEmpty()) {
throw new ValidationException(f("\"%s\" must not be empty. Please specify a comma-separated list of " +
"fully-qualified class name prefixes.", name));
}
if (set.stream().anyMatch(StringUtils::isBlank)) {
throw new ValidationException(f("\"%s\" must only contain non-empty class name prefixes.", name));
}
}
}

/**
* Calculate the default number of output buffer processors as a linear function of available CPU cores.
* The function is designed to yield predetermined values for the following select numbers of CPU cores that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
import org.graylog2.events.ClusterEventBus;
import org.graylog2.plugin.cluster.ClusterConfigService;
import org.graylog2.plugin.system.NodeId;
import org.graylog2.security.RestrictedChainingClassLoader;
import org.graylog2.security.SafeClasses;
import org.graylog2.security.UnsafeClassLoadingAttemptException;
import org.graylog2.shared.plugins.ChainingClassLoader;
import org.graylog2.shared.utilities.AutoValueUtils;
import org.joda.time.DateTime;
Expand All @@ -53,23 +56,33 @@ public class ClusterConfigServiceImpl implements ClusterConfigService {
private final JacksonDBCollection<ClusterConfig, String> dbCollection;
private final NodeId nodeId;
private final ObjectMapper objectMapper;
private final ChainingClassLoader chainingClassLoader;
private final RestrictedChainingClassLoader chainingClassLoader;
private final EventBus clusterEventBus;

@Inject
public ClusterConfigServiceImpl(final MongoJackObjectMapperProvider mapperProvider,
final MongoConnection mongoConnection,
final NodeId nodeId,
final ChainingClassLoader chainingClassLoader,
final RestrictedChainingClassLoader chainingClassLoader,
final ClusterEventBus clusterEventBus) {
this(JacksonDBCollection.wrap(prepareCollection(mongoConnection), ClusterConfig.class, String.class, mapperProvider.get()),
nodeId, mapperProvider.get(), chainingClassLoader, clusterEventBus);
}

@Deprecated
public ClusterConfigServiceImpl(final MongoJackObjectMapperProvider mapperProvider,
final MongoConnection mongoConnection,
final NodeId nodeId,
final ChainingClassLoader chainingClassLoader,
final ClusterEventBus clusterEventBus) {
this(JacksonDBCollection.wrap(prepareCollection(mongoConnection), ClusterConfig.class, String.class, mapperProvider.get()),
nodeId, mapperProvider.get(), new RestrictedChainingClassLoader(chainingClassLoader, SafeClasses.allGraylogInternal()), clusterEventBus);
}

private ClusterConfigServiceImpl(final JacksonDBCollection<ClusterConfig, String> dbCollection,
final NodeId nodeId,
final ObjectMapper objectMapper,
final ChainingClassLoader chainingClassLoader,
final RestrictedChainingClassLoader chainingClassLoader,
final EventBus clusterEventBus) {
this.nodeId = checkNotNull(nodeId);
this.dbCollection = checkNotNull(dbCollection);
Expand Down Expand Up @@ -175,10 +188,12 @@ public Set<Class<?>> list() {
for (ClusterConfig clusterConfig : clusterConfigs) {
final String type = clusterConfig.type();
try {
final Class<?> cls = chainingClassLoader.loadClass(type);
final Class<?> cls = chainingClassLoader.loadClassSafely(type);
classes.add(cls);
} catch (ClassNotFoundException e) {
LOG.debug("Couldn't find configuration class \"{}\"", type, e);
} catch (UnsafeClassLoadingAttemptException e) {
LOG.warn("Couldn't load class <{}>.", type, e);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@
import com.mongodb.DBObject;
import com.mongodb.MongoException;
import com.mongodb.WriteConcern;
import jakarta.inject.Inject;
import org.graylog2.bindings.providers.MongoJackObjectMapperProvider;
import org.graylog2.database.MongoConnection;
import org.graylog2.plugin.periodical.Periodical;
import org.graylog2.plugin.system.NodeId;
import org.graylog2.security.RestrictedChainingClassLoader;
import org.graylog2.security.SafeClasses;
import org.graylog2.security.UnsafeClassLoadingAttemptException;
import org.graylog2.shared.plugins.ChainingClassLoader;
import org.graylog2.shared.utilities.AutoValueUtils;
import org.mongojack.DBCursor;
Expand All @@ -41,8 +45,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import jakarta.inject.Inject;

import java.util.Collections;

import static com.google.common.base.Preconditions.checkNotNull;
Expand All @@ -57,23 +59,36 @@ public class ClusterEventPeriodical extends Periodical {
private final NodeId nodeId;
private final ObjectMapper objectMapper;
private final EventBus serverEventBus;
private final ChainingClassLoader chainingClassLoader;
private final RestrictedChainingClassLoader chainingClassLoader;

@Inject
public ClusterEventPeriodical(final MongoJackObjectMapperProvider mapperProvider,
final MongoConnection mongoConnection,
final NodeId nodeId,
final ChainingClassLoader chainingClassLoader,
final RestrictedChainingClassLoader chainingClassLoader,
final EventBus serverEventBus,
final ClusterEventBus clusterEventBus) {
this(JacksonDBCollection.wrap(prepareCollection(mongoConnection), ClusterEvent.class, String.class, mapperProvider.get()),
nodeId, mapperProvider.get(), chainingClassLoader, serverEventBus, clusterEventBus);
}

@Deprecated
public ClusterEventPeriodical(final MongoJackObjectMapperProvider mapperProvider,
final MongoConnection mongoConnection,
final NodeId nodeId,
final ChainingClassLoader chainingClassLoader,
final EventBus serverEventBus,
final ClusterEventBus clusterEventBus) {
this(JacksonDBCollection.wrap(prepareCollection(mongoConnection), ClusterEvent.class, String.class,
mapperProvider.get()), nodeId, mapperProvider.get(),
new RestrictedChainingClassLoader(chainingClassLoader, SafeClasses.allGraylogInternal()),
serverEventBus, clusterEventBus);
}

private ClusterEventPeriodical(final JacksonDBCollection<ClusterEvent, String> dbCollection,
final NodeId nodeId,
final ObjectMapper objectMapper,
final ChainingClassLoader chainingClassLoader,
final RestrictedChainingClassLoader chainingClassLoader,
final EventBus serverEventBus,
final ClusterEventBus clusterEventBus) {
this.nodeId = checkNotNull(nodeId);
Expand Down Expand Up @@ -205,15 +220,15 @@ private void updateConsumers(final String eventId, final NodeId nodeId) {

private Object extractPayload(Object payload, String eventClass) {
try {
final Class<?> clazz = chainingClassLoader.loadClass(eventClass);
final Class<?> clazz = chainingClassLoader.loadClassSafely(eventClass);
return objectMapper.convertValue(payload, clazz);
} catch (ClassNotFoundException e) {
LOG.debug("Couldn't load class <" + eventClass + "> for event", e);
return null;
} catch (IllegalArgumentException e) {
LOG.debug("Error while deserializing payload", e);
return null;

} catch (UnsafeClassLoadingAttemptException e) {
LOG.warn("Couldn't load class <{}>.", eventClass, e);
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
import org.graylog2.plugin.validate.ConfigValidationException;
import org.graylog2.rest.MoreMediaTypes;
import org.graylog2.rest.models.system.config.ClusterConfigList;
import org.graylog2.shared.plugins.ChainingClassLoader;
import org.graylog2.security.RestrictedChainingClassLoader;
import org.graylog2.security.UnsafeClassLoadingAttemptException;
import org.graylog2.shared.rest.resources.RestResource;
import org.graylog2.shared.security.RestPermissions;
import org.slf4j.Logger;
Expand All @@ -73,13 +74,13 @@ public class ClusterConfigResource extends RestResource {
public static final String NO_CLASS_MSG = "Couldn't find configuration class '%s'";

private final ClusterConfigService clusterConfigService;
private final ChainingClassLoader chainingClassLoader;
private final RestrictedChainingClassLoader chainingClassLoader;
private final ObjectMapper objectMapper;
private final ClusterConfigValidatorService clusterConfigValidatorService;

@Inject
public ClusterConfigResource(ClusterConfigService clusterConfigService,
ChainingClassLoader chainingClassLoader,
RestrictedChainingClassLoader chainingClassLoader,
ObjectMapper objectMapper,
ClusterConfigValidatorService clusterConfigValidatorService) {
this.clusterConfigService = requireNonNull(clusterConfigService);
Expand Down Expand Up @@ -209,9 +210,11 @@ public JsonSchema schema(@ApiParam(name = "configClass", value = "The name of th
@Nullable
private Class<?> classFromName(String className) {
try {
return chainingClassLoader.loadClass(className);
return chainingClassLoader.loadClassSafely(className);
} catch (ClassNotFoundException e) {
return null;
} catch (UnsafeClassLoadingAttemptException e) {
throw new BadRequestException(e.getLocalizedMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* 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 jakarta.inject.Inject;
import org.graylog2.Configuration;
import org.graylog2.shared.plugins.ChainingClassLoader;

import static org.graylog2.shared.utilities.StringUtils.f;

/**
* A wrapper around the chaining class loader intended only for loading classes safely by considering an allow-list of
* class name prefixes.
*/
public class RestrictedChainingClassLoader {
private final ChainingClassLoader delegate;
private final SafeClasses safeClasses;

@Inject
public RestrictedChainingClassLoader(ChainingClassLoader delegate, SafeClasses safeClasses) {
this.delegate = delegate;
this.safeClasses = safeClasses;
}

/**
* Load the class only if the name passes the check of {@link SafeClasses#isSafeToLoad(String)}. If the class name
* passes the check, the call is delegated to {@link ChainingClassLoader#loadClass(String)}. If it doesn't pass the
* check, an {@link UnsafeClassLoadingAttemptException} is thrown.
*
* @return class as returned by the delegated call to {@link ChainingClassLoader#loadClass(String)}
* @throws ClassNotFoundException if the class was not found
* @throws UnsafeClassLoadingAttemptException if the class name didn't pass the safety check of
* {@link SafeClasses#isSafeToLoad(String)}
*/
public Class<?> loadClassSafely(String name) throws ClassNotFoundException, UnsafeClassLoadingAttemptException {
if (safeClasses.isSafeToLoad(name)) {
return delegate.loadClass(name);
} else {
throw new UnsafeClassLoadingAttemptException(
f("Prevented loading of unsafe class \"%s\". Consider adjusting the configuration setting " +
"\"%s\", if you think that this is a mistake.", name, Configuration.SAFE_CLASSES)
);
}
}

}
Loading

0 comments on commit 8132032

Please sign in to comment.