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
…18180)

* Restrict classes allowed for cluster config and event types (#18165)

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

(cherry picked from commit 8132032)

* Use javax.inject.Inject instead of jakarta.inject.Inject

* Add "jakarta.inject.**" to forbidden APIs

This will help us with issue for backported code that's already using
jakarta.inject.

* Use javax.ws.rs instead of jakarta.ws.rs

---------

Co-authored-by: Othello Maurer <othello@graylog.com>
  • Loading branch information
bernd and thll committed Feb 6, 2024
1 parent fa99567 commit 7f8ef7f
Show file tree
Hide file tree
Showing 17 changed files with 430 additions and 30 deletions.
4 changes: 4 additions & 0 deletions changelog/unreleased/pr-18165.toml
@@ -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"]
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.DirectoryPathReadableValidator;
import com.github.joschi.jadconfig.validators.DirectoryPathWritableValidator;
Expand All @@ -33,6 +34,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 @@ -52,6 +54,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 @@ -212,6 +215,12 @@ 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.");

public boolean isInsecureStartup() {
return insecureStartup;
}
Expand Down
27 changes: 27 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/Configuration.java
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 @@ -49,11 +50,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 @@ -224,6 +228,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 @@ -450,6 +460,10 @@ public Period getMinimumAutoRefreshInterval() {
return minimumAutoRefreshInterval;
}

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

public FieldValueSuggestionMode getFieldValueSuggestionMode() {
return fieldValueSuggestionMode;
}
Expand Down Expand Up @@ -557,4 +571,17 @@ public void validate(String name, String path) throws ValidationException {
throw new ValidationException("Node ID file at path " + path + " isn't " + b + ". Please specify the correct path or change the permissions");
}
}

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));
}
}
}
}
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 @@ -52,23 +55,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 @@ -174,10 +187,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
Expand Up @@ -32,6 +32,9 @@
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 @@ -56,25 +59,38 @@ 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 EventBus serverEventBus,
final ClusterEventBus clusterEventBus) {
final NodeId nodeId,
final ObjectMapper objectMapper,
final RestrictedChainingClassLoader chainingClassLoader,
final EventBus serverEventBus,
final ClusterEventBus clusterEventBus) {
this.nodeId = checkNotNull(nodeId);
this.dbCollection = checkNotNull(dbCollection);
this.objectMapper = checkNotNull(objectMapper);
Expand Down Expand Up @@ -204,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;
}
}
Expand Up @@ -33,7 +33,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 Down Expand Up @@ -71,13 +72,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 @@ -207,9 +208,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
@@ -0,0 +1,61 @@
/*
* 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 org.graylog2.Configuration;
import org.graylog2.shared.plugins.ChainingClassLoader;

import javax.inject.Inject;

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)
);
}
}

}
@@ -0,0 +1,52 @@
/*
* 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 org.graylog2.Configuration;

import javax.annotation.Nonnull;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import java.util.Objects;
import java.util.Set;

/**
* Adds a safety net for class loading.
*/
@Singleton
public class SafeClasses {
private final Set<String> prefixes;

public static SafeClasses allGraylogInternal() {
return new SafeClasses(Set.of("org.graylog.", "org.graylog2."));
}

@Inject
public SafeClasses(@Named(Configuration.SAFE_CLASSES) @Nonnull Set<String> prefixes) {
this.prefixes = Objects.requireNonNull(prefixes);
}

/**
* Check if the class name is considered safe for loading by names from a potentially user-provided input.
* Classes are considered safe if their fully qualified class name starts with any of the prefixes configured in
* {@link Configuration#getSafeClasses()}.
*/
public boolean isSafeToLoad(String className) {
return prefixes.stream().anyMatch(className::startsWith);
}
}

0 comments on commit 7f8ef7f

Please sign in to comment.