graylog2-server vulnerable to instantiation of arbitrary classes triggered by API request
Description
Graylog is a free and open log management platform. Starting in version 2.0.0 and prior to versions 5.1.11 and 5.2.4, arbitrary classes can be loaded and instantiated using a HTTP PUT request to the /api/system/cluster_config/ endpoint. Graylog's cluster config system uses fully qualified class names as config keys. To validate the existence of the requested class before using them, Graylog loads the class using the class loader. If a user with the appropriate permissions performs the request, arbitrary classes with 1-arg String constructors can be instantiated. This will execute arbitrary code that is run during class instantiation. In the specific use case of java.io.File, the behavior of the internal web-server stack will lead to information exposure by including the entire file content in the response to the REST request. Versions 5.1.11 and 5.2.4 contain a fix for this issue.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
org.graylog2:graylog2-serverMaven | >= 2.0.0, < 5.1.11 | 5.1.11 |
org.graylog2:graylog2-serverMaven | >= 5.2.0-alpha.1, < 5.2.4 | 5.2.4 |
Affected products
1- Range: >= 2.0.0, < 5.1.11
Patches
27f8ef7fa8edfRestrict classes allowed for cluster config and event types (#18165) (#18180)
17 files changed · +430 −30
changelog/unreleased/pr-18165.toml+4 −0 added@@ -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"]
data-node/src/main/java/org/graylog/datanode/Configuration.java+9 −0 modified@@ -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; @@ -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; @@ -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 @@ -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; }
graylog2-server/src/main/java/org/graylog2/cluster/ClusterConfigServiceImpl.java+19 −4 modified@@ -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; @@ -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); @@ -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); } } }
graylog2-server/src/main/java/org/graylog2/Configuration.java+27 −0 modified@@ -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; @@ -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. @@ -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; @@ -450,6 +460,10 @@ public Period getMinimumAutoRefreshInterval() { return minimumAutoRefreshInterval; } + public Set<String> getSafeClasses() { + return safeClasses; + } + public FieldValueSuggestionMode getFieldValueSuggestionMode() { return fieldValueSuggestionMode; } @@ -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)); + } + } + } }
graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java+27 −11 modified@@ -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; @@ -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); @@ -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; } }
graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.java+7 −4 modified@@ -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; @@ -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); @@ -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()); } }
graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java+61 −0 added@@ -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) + ); + } + } + +}
graylog2-server/src/main/java/org/graylog2/security/SafeClasses.java+52 −0 added@@ -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); + } +}
graylog2-server/src/main/java/org/graylog2/security/UnsafeClassLoadingAttemptException.java+29 −0 added@@ -0,0 +1,29 @@ +/* + * 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; + +/** + * Exception indicating an attempt to load a class that is not considered safe because it's fully qualified class name + * did not start with any of the prefixes configured in {@link Configuration#getSafeClasses()} + */ +public class UnsafeClassLoadingAttemptException extends Exception { + public UnsafeClassLoadingAttemptException(String message) { + super(message); + } +}
graylog2-server/src/test/java/org/graylog2/cluster/ClusterConfigServiceImplTest.java+20 −2 modified@@ -30,6 +30,8 @@ import org.graylog2.plugin.cluster.ClusterConfigService; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.joda.time.DateTime; @@ -78,7 +80,8 @@ public void setUpService() throws Exception { provider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader(new ChainingClassLoader(getClass().getClassLoader()), + SafeClasses.allGraylogInternal()), clusterEventBus ); } @@ -401,7 +404,7 @@ public void listIgnoresInvalidClasses() throws Exception { .add("last_updated_by", "ID") .get()); collection.save(new BasicDBObjectBuilder() - .add("type", "invalid.ClassName") + .add("type", "org.graylog.invalid.ClassName") .add("payload", Collections.emptyMap()) .add("last_updated", TIME.toString()) .add("last_updated_by", "ID") @@ -413,6 +416,21 @@ public void listIgnoresInvalidClasses() throws Exception { .containsOnly(CustomConfig.class); } + @Test + public void listIgnoresUnsafeClasses() throws Exception { + @SuppressWarnings("deprecation") + final DBCollection collection = mongoConnection.getDatabase().getCollection(COLLECTION_NAME); + collection.save(new BasicDBObjectBuilder() + .add("type", "java.io.File") + .add("payload", "/etc/passwd") + .add("last_updated", TIME.toString()) + .add("last_updated_by", "ID") + .get()); + + assertThat(collection.count()).isOne(); + assertThat(clusterConfigService.list()).hasSize(0); + } + public static class ClusterConfigChangedEventHandler { public volatile ClusterConfigChangedEvent event;
graylog2-server/src/test/java/org/graylog2/events/ClusterEventPeriodicalTest.java+59 −2 modified@@ -32,6 +32,8 @@ import org.graylog2.database.MongoConnection; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.graylog2.system.debug.DebugEvent; @@ -49,6 +51,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.assertThat; @@ -87,7 +90,9 @@ public void setUpService() throws Exception { provider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader(new ChainingClassLoader(getClass().getClassLoader()), + new SafeClasses(Set.of( + SimpleEvent.class.getName(), DebugEvent.class.getName(), Safe.class.getName()))), serverEventBus, clusterEventBus ); @@ -311,7 +316,7 @@ public void localNodeIsMarkedAsHavingConsumedEvent() { DBObject dbObject = collection.findOne(); - assertThat(((BasicDBList)dbObject.get("consumers")).toArray()).isEqualTo(new String[] { nodeId.getNodeId() }); + assertThat(((BasicDBList) dbObject.get("consumers")).toArray()).isEqualTo(new String[]{nodeId.getNodeId()}); } @Test @@ -342,6 +347,58 @@ public void localEventIsNotProcessedFromDB() { verify(clusterEventBus, never()).post(any()); } + private static volatile String constructorArgument; + + public static class Unsafe { + public Unsafe(String param) { + constructorArgument = param; + } + } + + public static class Safe { + public Safe(String param) { + constructorArgument = param; + } + } + + @Test + public void testInstantiatesSafeEventClass() { + DBObject event = new BasicDBObjectBuilder() + .add("timestamp", TIME.getMillis()) + .add("producer", "TEST-PRODUCER") + .add("consumers", Collections.emptyList()) + .add("event_class", "org.graylog2.events.ClusterEventPeriodicalTest$Safe") + .add("payload", "this-is-safe") + .get(); + + @SuppressWarnings("deprecation") + final DBCollection collection = mongoConnection.getDatabase().getCollection(ClusterEventPeriodical.COLLECTION_NAME); + collection.save(event); + + constructorArgument = null; + clusterEventPeriodical.run(); + assertThat(constructorArgument).isEqualTo("this-is-safe"); + } + + @Test + public void testIgnoresUnsafeEventClass() { + DBObject event = new BasicDBObjectBuilder() + .add("timestamp", TIME.getMillis()) + .add("producer", "TEST-PRODUCER") + .add("consumers", Collections.emptyList()) + .add("event_class", "org.graylog2.events.ClusterEventPeriodicalTest$Unsafe") + .add("payload", "this-is-unsafe") + .get(); + + @SuppressWarnings("deprecation") + final DBCollection collection = mongoConnection.getDatabase().getCollection(ClusterEventPeriodical.COLLECTION_NAME); + collection.save(event); + + constructorArgument = null; + clusterEventPeriodical.run(); + assertThat(constructorArgument).isNull(); + } + public static class SimpleEventHandler { final AtomicInteger invocations = new AtomicInteger();
graylog2-server/src/test/java/org/graylog2/indexer/indexset/MongoIndexSetServiceTest.java+5 −1 modified@@ -34,6 +34,8 @@ import org.graylog2.plugin.cluster.ClusterConfigService; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.graylog2.streams.StreamService; @@ -78,7 +80,9 @@ public class MongoIndexSetServiceTest { public void setUp() throws Exception { clusterEventBus = new ClusterEventBus(); clusterConfigService = new ClusterConfigServiceImpl(objectMapperProvider, mongodb.mongoConnection(), - nodeId, new ChainingClassLoader(getClass().getClassLoader()), clusterEventBus); + nodeId, new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), + clusterEventBus); indexSetService = new MongoIndexSetService(mongodb.mongoConnection(), objectMapperProvider, streamService, clusterConfigService, clusterEventBus); }
graylog2-server/src/test/java/org/graylog2/migrations/V20161215163900_MoveIndexSetDefaultConfigTest.java+7 −3 modified@@ -29,13 +29,14 @@ import org.graylog2.migrations.V20161215163900_MoveIndexSetDefaultConfig.MigrationCompleted; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -70,8 +71,11 @@ public class V20161215163900_MoveIndexSetDefaultConfigTest { @Before public void setUp() throws Exception { this.clusterConfigService = spy(new ClusterConfigServiceImpl(objectMapperProvider, - mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), new ClusterEventBus())); + mongodb.mongoConnection(), + nodeId, + new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), + new ClusterEventBus())); this.collection = mongodb.mongoConnection().getMongoDatabase().getCollection("index_sets");
graylog2-server/src/test/java/org/graylog2/migrations/V20170110150100_FixAlertConditionsMigrationTest.java+5 −2 modified@@ -32,12 +32,13 @@ import org.graylog2.migrations.V20170110150100_FixAlertConditionsMigration.MigrationCompleted; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -74,7 +75,9 @@ public class V20170110150100_FixAlertConditionsMigrationTest { public void setUp() throws Exception { this.clusterConfigService = spy(new ClusterConfigServiceImpl(objectMapperProvider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), new ClusterEventBus())); + new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), + new ClusterEventBus())); final MongoConnection mongoConnection = spy(mongodb.mongoConnection()); final MongoDatabase mongoDatabase = spy(mongoConnection.getMongoDatabase());
graylog2-server/src/test/java/org/graylog2/rest/resources/system/ClusterConfigResourceTest.java+94 −0 added@@ -0,0 +1,94 @@ +/* + * 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.rest.resources.system; + +import org.glassfish.jersey.message.internal.FileProvider; +import org.graylog2.plugin.cluster.ClusterConfigService; +import org.graylog2.plugin.validate.ClusterConfigValidatorService; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; +import org.graylog2.shared.bindings.providers.ObjectMapperProvider; +import org.graylog2.shared.plugins.ChainingClassLoader; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import javax.ws.rs.BadRequestException; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.graylog2.shared.utilities.StringUtils.f; + +@ExtendWith(MockitoExtension.class) +class ClusterConfigResourceTest { + @Mock + private ClusterConfigService clusterConfigService; + + @Mock + private ClusterConfigValidatorService clusterConfigValidatorService; + + @Test + void putClassConsideredUnsafe(@TempDir Path tmpDir) throws IOException { + final Path file = tmpDir.resolve("secrets.txt"); + Files.writeString(file, "secret content"); + + final ClusterConfigResource resource = new ClusterConfigResource(clusterConfigService, + new RestrictedChainingClassLoader(new ChainingClassLoader(this.getClass().getClassLoader()), + SafeClasses.allGraylogInternal()), + new ObjectMapperProvider().get(), + clusterConfigValidatorService + ); + + assertThatThrownBy(() -> resource.update("java.io.File", + new ByteArrayInputStream(f("\"%s\"", file.toAbsolutePath()).getBytes(StandardCharsets.UTF_8)))) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining("Prevented loading of unsafe class"); + } + + /** + * Proof of concept to show what would happen if we'd allow problematic classes + */ + @Test + void putClassConsideredSafe(@TempDir Path tmpDir) throws IOException { + final Path file = tmpDir.resolve("secrets.txt"); + Files.writeString(file, "secret content"); + + final ClusterConfigResource resource = new ClusterConfigResource(clusterConfigService, + new RestrictedChainingClassLoader(new ChainingClassLoader(this.getClass().getClassLoader()), + new SafeClasses(Set.of("java.io.File"))), + new ObjectMapperProvider().get(), + clusterConfigValidatorService); + + // Simulate how jersey would serialize a File object + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + new FileProvider().writeTo((File) resource.update("java.io.File", + new ByteArrayInputStream(f("\"%s\"", file.toAbsolutePath()).getBytes(StandardCharsets.UTF_8))).getEntity(), null, null, null, null, null, out); + final String content = out.toString(StandardCharsets.UTF_8); + + assertThat(content).isEqualTo("secret content"); + } +}
graylog2-server/src/test/java/org/graylog/plugins/views/search/views/ViewServiceTest.java+4 −1 modified@@ -31,6 +31,8 @@ import org.graylog2.plugin.system.SimpleNodeId; import org.graylog2.search.SearchQueryField; import org.graylog2.search.SearchQueryParser; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.junit.After; @@ -64,7 +66,8 @@ public void setUp() throws Exception { objectMapperProvider, mongodb.mongoConnection(), new SimpleNodeId("5ca1ab1e-0000-4000-a000-000000000000"), - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), new ClusterEventBus() ); this.dbService = new ViewService(
pom.xml+1 −0 modified@@ -753,6 +753,7 @@ com.google.inject.Singleton com.google.inject.Provider com.google.inject.name.Named + jakarta.inject.** ]]> </signatures> </configuration>
75ef2b8d60e7Restrict classes allowed for cluster config and event types (#18165) (#18179)
16 files changed · +429 −30
changelog/unreleased/pr-18165.toml+4 −0 added@@ -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"]
data-node/src/main/java/org/graylog/datanode/Configuration.java+9 −0 modified@@ -23,13 +23,15 @@ 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.validators.PositiveIntegerValidator; import com.github.joschi.jadconfig.validators.StringNotBlankValidator; import com.github.joschi.jadconfig.validators.URIAbsoluteValidator; import com.google.common.annotations.VisibleForTesting; import com.google.common.net.HostAndPort; import com.google.common.net.InetAddresses; import org.graylog.datanode.configuration.BaseConfiguration; +import org.graylog2.Configuration.SafeClassesValidator; import org.graylog2.plugin.Tools; import org.joda.time.DateTimeZone; import org.slf4j.Logger; @@ -48,6 +50,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; /** * Helper class to hold configuration of Graylog @@ -124,6 +127,12 @@ public class Configuration extends BaseConfiguration { @Parameter(value = "user_password_bcrypt_salt_size", validators = PositiveIntegerValidator.class) private int userPasswordBCryptSaltSize = 10; + /** + * 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 Integer getStaleLeaderTimeout() { return staleLeaderTimeout; }
graylog2-server/src/main/java/org/graylog2/cluster/ClusterConfigServiceImpl.java+19 −4 modified@@ -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; @@ -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); @@ -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); } } }
graylog2-server/src/main/java/org/graylog2/Configuration.java+27 −0 modified@@ -27,6 +27,7 @@ import com.github.joschi.jadconfig.validators.PositiveIntegerValidator; import com.github.joschi.jadconfig.validators.PositiveLongValidator; import com.github.joschi.jadconfig.validators.StringNotBlankValidator; +import org.apache.commons.lang3.StringUtils; import org.graylog2.cluster.leader.AutomaticLeaderElectionService; import org.graylog2.cluster.leader.LeaderElectionMode; import org.graylog2.cluster.leader.LeaderElectionService; @@ -44,11 +45,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 BaseConfiguration { + public static final String SAFE_CLASSES = "safe_classes"; /** * Deprecated! Use isLeader() instead. @@ -210,6 +214,12 @@ public class Configuration extends BaseConfiguration { @Parameter(value = "lock_service_lock_ttl", converter = JavaDurationConverter.class) private java.time.Duration lockServiceLockTTL = MongoLockService.MIN_LOCK_TTL; + /** + * 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."); + public boolean maintainsStreamAwareFieldTypes() { return streamAwareFieldTypes; } @@ -425,6 +435,10 @@ public Duration getFailureHandlingShutdownAwait() { return failureHandlingShutdownAwait; } + public Set<String> getSafeClasses() { + return safeClasses; + } + /** * This is needed for backwards compatibility. The setting in TLSProtocolsConfiguration should be used instead. */ @@ -528,4 +542,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)); + } + } + } }
graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java+27 −11 modified@@ -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; @@ -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); @@ -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; } }
graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.java+7 −4 modified@@ -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; @@ -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); @@ -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()); } }
graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java+61 −0 added@@ -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) + ); + } + } + +}
graylog2-server/src/main/java/org/graylog2/security/SafeClasses.java+52 −0 added@@ -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); + } +}
graylog2-server/src/main/java/org/graylog2/security/UnsafeClassLoadingAttemptException.java+29 −0 added@@ -0,0 +1,29 @@ +/* + * 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; + +/** + * Exception indicating an attempt to load a class that is not considered safe because it's fully qualified class name + * did not start with any of the prefixes configured in {@link Configuration#getSafeClasses()} + */ +public class UnsafeClassLoadingAttemptException extends Exception { + public UnsafeClassLoadingAttemptException(String message) { + super(message); + } +}
graylog2-server/src/test/java/org/graylog2/cluster/ClusterConfigServiceImplTest.java+20 −2 modified@@ -30,6 +30,8 @@ import org.graylog2.plugin.cluster.ClusterConfigService; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.joda.time.DateTime; @@ -78,7 +80,8 @@ public void setUpService() throws Exception { provider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader(new ChainingClassLoader(getClass().getClassLoader()), + SafeClasses.allGraylogInternal()), clusterEventBus ); } @@ -401,7 +404,7 @@ public void listIgnoresInvalidClasses() throws Exception { .add("last_updated_by", "ID") .get()); collection.save(new BasicDBObjectBuilder() - .add("type", "invalid.ClassName") + .add("type", "org.graylog.invalid.ClassName") .add("payload", Collections.emptyMap()) .add("last_updated", TIME.toString()) .add("last_updated_by", "ID") @@ -413,6 +416,21 @@ public void listIgnoresInvalidClasses() throws Exception { .containsOnly(CustomConfig.class); } + @Test + public void listIgnoresUnsafeClasses() throws Exception { + @SuppressWarnings("deprecation") + final DBCollection collection = mongoConnection.getDatabase().getCollection(COLLECTION_NAME); + collection.save(new BasicDBObjectBuilder() + .add("type", "java.io.File") + .add("payload", "/etc/passwd") + .add("last_updated", TIME.toString()) + .add("last_updated_by", "ID") + .get()); + + assertThat(collection.count()).isOne(); + assertThat(clusterConfigService.list()).hasSize(0); + } + public static class ClusterConfigChangedEventHandler { public volatile ClusterConfigChangedEvent event;
graylog2-server/src/test/java/org/graylog2/events/ClusterEventPeriodicalTest.java+59 −2 modified@@ -32,6 +32,8 @@ import org.graylog2.database.MongoConnection; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.graylog2.system.debug.DebugEvent; @@ -49,6 +51,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.assertThat; @@ -87,7 +90,9 @@ public void setUpService() throws Exception { provider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader(new ChainingClassLoader(getClass().getClassLoader()), + new SafeClasses(Set.of( + SimpleEvent.class.getName(), DebugEvent.class.getName(), Safe.class.getName()))), serverEventBus, clusterEventBus ); @@ -311,7 +316,7 @@ public void localNodeIsMarkedAsHavingConsumedEvent() { DBObject dbObject = collection.findOne(); - assertThat(((BasicDBList)dbObject.get("consumers")).toArray()).isEqualTo(new String[] { nodeId.getNodeId() }); + assertThat(((BasicDBList) dbObject.get("consumers")).toArray()).isEqualTo(new String[]{nodeId.getNodeId()}); } @Test @@ -342,6 +347,58 @@ public void localEventIsNotProcessedFromDB() { verify(clusterEventBus, never()).post(any()); } + private static volatile String constructorArgument; + + public static class Unsafe { + public Unsafe(String param) { + constructorArgument = param; + } + } + + public static class Safe { + public Safe(String param) { + constructorArgument = param; + } + } + + @Test + public void testInstantiatesSafeEventClass() { + DBObject event = new BasicDBObjectBuilder() + .add("timestamp", TIME.getMillis()) + .add("producer", "TEST-PRODUCER") + .add("consumers", Collections.emptyList()) + .add("event_class", "org.graylog2.events.ClusterEventPeriodicalTest$Safe") + .add("payload", "this-is-safe") + .get(); + + @SuppressWarnings("deprecation") + final DBCollection collection = mongoConnection.getDatabase().getCollection(ClusterEventPeriodical.COLLECTION_NAME); + collection.save(event); + + constructorArgument = null; + clusterEventPeriodical.run(); + assertThat(constructorArgument).isEqualTo("this-is-safe"); + } + + @Test + public void testIgnoresUnsafeEventClass() { + DBObject event = new BasicDBObjectBuilder() + .add("timestamp", TIME.getMillis()) + .add("producer", "TEST-PRODUCER") + .add("consumers", Collections.emptyList()) + .add("event_class", "org.graylog2.events.ClusterEventPeriodicalTest$Unsafe") + .add("payload", "this-is-unsafe") + .get(); + + @SuppressWarnings("deprecation") + final DBCollection collection = mongoConnection.getDatabase().getCollection(ClusterEventPeriodical.COLLECTION_NAME); + collection.save(event); + + constructorArgument = null; + clusterEventPeriodical.run(); + assertThat(constructorArgument).isNull(); + } + public static class SimpleEventHandler { final AtomicInteger invocations = new AtomicInteger();
graylog2-server/src/test/java/org/graylog2/indexer/indexset/MongoIndexSetServiceTest.java+5 −1 modified@@ -34,6 +34,8 @@ import org.graylog2.plugin.cluster.ClusterConfigService; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.graylog2.streams.StreamService; @@ -78,7 +80,9 @@ public class MongoIndexSetServiceTest { public void setUp() throws Exception { clusterEventBus = new ClusterEventBus(); clusterConfigService = new ClusterConfigServiceImpl(objectMapperProvider, mongodb.mongoConnection(), - nodeId, new ChainingClassLoader(getClass().getClassLoader()), clusterEventBus); + nodeId, new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), + clusterEventBus); indexSetService = new MongoIndexSetService(mongodb.mongoConnection(), objectMapperProvider, streamService, clusterConfigService, clusterEventBus); }
graylog2-server/src/test/java/org/graylog2/migrations/V20161215163900_MoveIndexSetDefaultConfigTest.java+7 −3 modified@@ -29,13 +29,14 @@ import org.graylog2.migrations.V20161215163900_MoveIndexSetDefaultConfig.MigrationCompleted; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -70,8 +71,11 @@ public class V20161215163900_MoveIndexSetDefaultConfigTest { @Before public void setUp() throws Exception { this.clusterConfigService = spy(new ClusterConfigServiceImpl(objectMapperProvider, - mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), new ClusterEventBus())); + mongodb.mongoConnection(), + nodeId, + new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), + new ClusterEventBus())); this.collection = mongodb.mongoConnection().getMongoDatabase().getCollection("index_sets");
graylog2-server/src/test/java/org/graylog2/migrations/V20170110150100_FixAlertConditionsMigrationTest.java+5 −2 modified@@ -32,12 +32,13 @@ import org.graylog2.migrations.V20170110150100_FixAlertConditionsMigration.MigrationCompleted; import org.graylog2.plugin.system.NodeId; import org.graylog2.plugin.system.SimpleNodeId; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -74,7 +75,9 @@ public class V20170110150100_FixAlertConditionsMigrationTest { public void setUp() throws Exception { this.clusterConfigService = spy(new ClusterConfigServiceImpl(objectMapperProvider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), new ClusterEventBus())); + new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), + new ClusterEventBus())); final MongoConnection mongoConnection = spy(mongodb.mongoConnection()); final MongoDatabase mongoDatabase = spy(mongoConnection.getMongoDatabase());
graylog2-server/src/test/java/org/graylog2/rest/resources/system/ClusterConfigResourceTest.java+94 −0 added@@ -0,0 +1,94 @@ +/* + * 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.rest.resources.system; + +import org.glassfish.jersey.message.internal.FileProvider; +import org.graylog2.plugin.cluster.ClusterConfigService; +import org.graylog2.plugin.validate.ClusterConfigValidatorService; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; +import org.graylog2.shared.bindings.providers.ObjectMapperProvider; +import org.graylog2.shared.plugins.ChainingClassLoader; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import javax.ws.rs.BadRequestException; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.graylog2.shared.utilities.StringUtils.f; + +@ExtendWith(MockitoExtension.class) +class ClusterConfigResourceTest { + @Mock + private ClusterConfigService clusterConfigService; + + @Mock + private ClusterConfigValidatorService clusterConfigValidatorService; + + @Test + void putClassConsideredUnsafe(@TempDir Path tmpDir) throws IOException { + final Path file = tmpDir.resolve("secrets.txt"); + Files.writeString(file, "secret content"); + + final ClusterConfigResource resource = new ClusterConfigResource(clusterConfigService, + new RestrictedChainingClassLoader(new ChainingClassLoader(this.getClass().getClassLoader()), + SafeClasses.allGraylogInternal()), + new ObjectMapperProvider().get(), + clusterConfigValidatorService + ); + + assertThatThrownBy(() -> resource.update("java.io.File", + new ByteArrayInputStream(f("\"%s\"", file.toAbsolutePath()).getBytes(StandardCharsets.UTF_8)))) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining("Prevented loading of unsafe class"); + } + + /** + * Proof of concept to show what would happen if we'd allow problematic classes + */ + @Test + void putClassConsideredSafe(@TempDir Path tmpDir) throws IOException { + final Path file = tmpDir.resolve("secrets.txt"); + Files.writeString(file, "secret content"); + + final ClusterConfigResource resource = new ClusterConfigResource(clusterConfigService, + new RestrictedChainingClassLoader(new ChainingClassLoader(this.getClass().getClassLoader()), + new SafeClasses(Set.of("java.io.File"))), + new ObjectMapperProvider().get(), + clusterConfigValidatorService); + + // Simulate how jersey would serialize a File object + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + new FileProvider().writeTo((File) resource.update("java.io.File", + new ByteArrayInputStream(f("\"%s\"", file.toAbsolutePath()).getBytes(StandardCharsets.UTF_8))).getEntity(), null, null, null, null, null, out); + final String content = out.toString(StandardCharsets.UTF_8); + + assertThat(content).isEqualTo("secret content"); + } +}
graylog2-server/src/test/java/org/graylog/plugins/views/search/views/ViewServiceTest.java+4 −1 modified@@ -29,6 +29,8 @@ import org.graylog2.plugin.system.SimpleNodeId; import org.graylog2.search.SearchQueryField; import org.graylog2.search.SearchQueryParser; +import org.graylog2.security.RestrictedChainingClassLoader; +import org.graylog2.security.SafeClasses; import org.graylog2.shared.bindings.providers.ObjectMapperProvider; import org.graylog2.shared.plugins.ChainingClassLoader; import org.junit.After; @@ -60,7 +62,8 @@ public void setUp() throws Exception { objectMapperProvider, mongodb.mongoConnection(), new SimpleNodeId("5ca1ab1e-0000-4000-a000-000000000000"), - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader( + new ChainingClassLoader(getClass().getClassLoader()), SafeClasses.allGraylogInternal()), new ClusterEventBus() ); this.dbService = new ViewService(
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
6- github.com/advisories/GHSA-p6gg-5hf4-4rgjghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2024-24824ghsaADVISORY
- github.com/Graylog2/graylog2-server/blob/e458db8bf4f789d4d19f1b37f0263f910c8d036c/graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.javaghsax_refsource_MISCWEB
- github.com/Graylog2/graylog2-server/commit/75ef2b8d60e7d67f859b79fe712c8ae7b2e861d8ghsax_refsource_MISCWEB
- github.com/Graylog2/graylog2-server/commit/7f8ef7fa8edf493106d5ef6f777d4da02c5194d9ghsax_refsource_MISCWEB
- github.com/Graylog2/graylog2-server/security/advisories/GHSA-p6gg-5hf4-4rgjghsax_refsource_CONFIRMWEB
News mentions
0No linked articles in our index yet.