diff --git a/changelog/unreleased/pr-18165.toml b/changelog/unreleased/pr-18165.toml new file mode 100644 index 0000000000..5254c051e0 --- /dev/null +++ b/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"] diff --git a/data-node/src/main/java/org/graylog/datanode/Configuration.java b/data-node/src/main/java/org/graylog/datanode/Configuration.java index 2c7601f5b0..34926091f7 100644 --- a/data-node/src/main/java/org/graylog/datanode/Configuration.java +++ b/data-node/src/main/java/org/graylog/datanode/Configuration.java @@ -23,6 +23,7 @@ import com.github.joschi.jadconfig.Validator; 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; @@ -31,6 +32,7 @@ import com.google.common.annotations.VisibleForTesting; 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; @@ -50,6 +52,7 @@ import java.nio.file.Paths; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; /** * Helper class to hold configuration of Graylog @@ -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 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"; diff --git a/graylog2-server/src/main/java/org/graylog2/Configuration.java b/graylog2-server/src/main/java/org/graylog2/Configuration.java index 42a1ba3adb..428fde6cd4 100644 --- a/graylog2-server/src/main/java/org/graylog2/Configuration.java +++ b/graylog2-server/src/main/java/org/graylog2/Configuration.java @@ -28,6 +28,7 @@ import com.github.joschi.jadconfig.validators.PositiveIntegerValidator; 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; @@ -50,11 +51,14 @@ import java.nio.file.Paths; 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. @@ -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 safeClasses = Set.of("org.graylog.", "org.graylog2."); + @Parameter(value = "field_value_suggestion_mode", required = true, converter = FieldValueSuggestionModeConverter.class) private FieldValueSuggestionMode fieldValueSuggestionMode = FieldValueSuggestionMode.ON; @@ -457,6 +467,10 @@ public class Configuration extends CaConfiguration { return minimumAutoRefreshInterval; } + public Set getSafeClasses() { + return safeClasses; + } + public FieldValueSuggestionMode getFieldValueSuggestionMode() { return fieldValueSuggestionMode; } @@ -573,6 +587,19 @@ public class Configuration extends CaConfiguration { } } + public static class SafeClassesValidator implements Validator> { + @Override + public void validate(String name, Set 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 diff --git a/graylog2-server/src/main/java/org/graylog2/cluster/ClusterConfigServiceImpl.java b/graylog2-server/src/main/java/org/graylog2/cluster/ClusterConfigServiceImpl.java index 74c7d1fe93..8734560da2 100644 --- a/graylog2-server/src/main/java/org/graylog2/cluster/ClusterConfigServiceImpl.java +++ b/graylog2-server/src/main/java/org/graylog2/cluster/ClusterConfigServiceImpl.java @@ -27,6 +27,9 @@ import org.graylog2.database.MongoConnection; 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; @@ -53,23 +56,33 @@ public class ClusterConfigServiceImpl implements ClusterConfigService { private final JacksonDBCollection 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 dbCollection, final NodeId nodeId, final ObjectMapper objectMapper, - final ChainingClassLoader chainingClassLoader, + final RestrictedChainingClassLoader chainingClassLoader, final EventBus clusterEventBus) { this.nodeId = checkNotNull(nodeId); this.dbCollection = checkNotNull(dbCollection); @@ -175,10 +188,12 @@ public class ClusterConfigServiceImpl implements ClusterConfigService { 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); } } } diff --git a/graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java b/graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java index 06bd5f8318..6dbc39fe4c 100644 --- a/graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java +++ b/graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java @@ -28,10 +28,14 @@ import com.mongodb.DBCollection; 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; @@ -41,8 +45,6 @@ import org.mongojack.JacksonDBCollection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import jakarta.inject.Inject; - import java.util.Collections; import static com.google.common.base.Preconditions.checkNotNull; @@ -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 dbCollection, final NodeId nodeId, final ObjectMapper objectMapper, - final ChainingClassLoader chainingClassLoader, + final RestrictedChainingClassLoader chainingClassLoader, final EventBus serverEventBus, final ClusterEventBus clusterEventBus) { this.nodeId = checkNotNull(nodeId); @@ -205,15 +220,15 @@ public class ClusterEventPeriodical extends Periodical { 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; } } diff --git a/graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.java b/graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.java index 6995191056..a3107cd111 100644 --- a/graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.java +++ b/graylog2-server/src/main/java/org/graylog2/rest/resources/system/ClusterConfigResource.java @@ -47,7 +47,8 @@ import org.graylog2.plugin.validate.ClusterConfigValidatorService; 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; @@ -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); @@ -209,9 +210,11 @@ public class ClusterConfigResource extends RestResource { @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()); } } diff --git a/graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java b/graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java new file mode 100644 index 0000000000..752463ce4f --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java @@ -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 + * . + */ +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) + ); + } + } + +} diff --git a/graylog2-server/src/main/java/org/graylog2/security/SafeClasses.java b/graylog2-server/src/main/java/org/graylog2/security/SafeClasses.java new file mode 100644 index 0000000000..bc1fd3148f --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog2/security/SafeClasses.java @@ -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 + * . + */ +package org.graylog2.security; + +import jakarta.inject.Inject; +import jakarta.inject.Named; +import jakarta.inject.Singleton; +import org.graylog2.Configuration; + +import javax.annotation.Nonnull; +import java.util.Objects; +import java.util.Set; + +/** + * Adds a safety net for class loading. + */ +@Singleton +public class SafeClasses { + private final Set prefixes; + + public static SafeClasses allGraylogInternal() { + return new SafeClasses(Set.of("org.graylog.", "org.graylog2.")); + } + + @Inject + public SafeClasses(@Named(Configuration.SAFE_CLASSES) @Nonnull Set 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); + } +} diff --git a/graylog2-server/src/main/java/org/graylog2/security/UnsafeClassLoadingAttemptException.java b/graylog2-server/src/main/java/org/graylog2/security/UnsafeClassLoadingAttemptException.java new file mode 100644 index 0000000000..0d73c707fd --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog2/security/UnsafeClassLoadingAttemptException.java @@ -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 + * . + */ +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); + } +} diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/search/views/ViewServiceTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/search/views/ViewServiceTest.java index c1112df2d9..2d4776397f 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/search/views/ViewServiceTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/search/views/ViewServiceTest.java @@ -31,6 +31,8 @@ import org.graylog2.events.ClusterEventBus; 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 class ViewServiceTest { 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( diff --git a/graylog2-server/src/test/java/org/graylog2/cluster/ClusterConfigServiceImplTest.java b/graylog2-server/src/test/java/org/graylog2/cluster/ClusterConfigServiceImplTest.java index 0f1176d540..b89cbac91b 100644 --- a/graylog2-server/src/test/java/org/graylog2/cluster/ClusterConfigServiceImplTest.java +++ b/graylog2-server/src/test/java/org/graylog2/cluster/ClusterConfigServiceImplTest.java @@ -30,6 +30,8 @@ import org.graylog2.events.ClusterEventBus; 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 class ClusterConfigServiceImplTest { provider, mongodb.mongoConnection(), nodeId, - new ChainingClassLoader(getClass().getClassLoader()), + new RestrictedChainingClassLoader(new ChainingClassLoader(getClass().getClassLoader()), + SafeClasses.allGraylogInternal()), clusterEventBus ); } @@ -401,7 +404,7 @@ public class ClusterConfigServiceImplTest { .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 class ClusterConfigServiceImplTest { .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; diff --git a/graylog2-server/src/test/java/org/graylog2/events/ClusterEventPeriodicalTest.java b/graylog2-server/src/test/java/org/graylog2/events/ClusterEventPeriodicalTest.java index 3170574047..b84f42dee6 100644 --- a/graylog2-server/src/test/java/org/graylog2/events/ClusterEventPeriodicalTest.java +++ b/graylog2-server/src/test/java/org/graylog2/events/ClusterEventPeriodicalTest.java @@ -32,6 +32,8 @@ import org.graylog2.bindings.providers.MongoJackObjectMapperProvider; 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 org.mockito.junit.MockitoRule; 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 class ClusterEventPeriodicalTest { 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 class ClusterEventPeriodicalTest { 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 class ClusterEventPeriodicalTest { 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(); diff --git a/graylog2-server/src/test/java/org/graylog2/indexer/indexset/MongoIndexSetServiceTest.java b/graylog2-server/src/test/java/org/graylog2/indexer/indexset/MongoIndexSetServiceTest.java index 3894dbe0e8..9ed939b011 100644 --- a/graylog2-server/src/test/java/org/graylog2/indexer/indexset/MongoIndexSetServiceTest.java +++ b/graylog2-server/src/test/java/org/graylog2/indexer/indexset/MongoIndexSetServiceTest.java @@ -34,6 +34,8 @@ import org.graylog2.indexer.rotation.strategies.MessageCountRotationStrategyConf 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); } diff --git a/graylog2-server/src/test/java/org/graylog2/migrations/V20161215163900_MoveIndexSetDefaultConfigTest.java b/graylog2-server/src/test/java/org/graylog2/migrations/V20161215163900_MoveIndexSetDefaultConfigTest.java index 79c1fc15ae..6a066d3b56 100644 --- a/graylog2-server/src/test/java/org/graylog2/migrations/V20161215163900_MoveIndexSetDefaultConfigTest.java +++ b/graylog2-server/src/test/java/org/graylog2/migrations/V20161215163900_MoveIndexSetDefaultConfigTest.java @@ -29,13 +29,14 @@ import org.graylog2.indexer.indexset.DefaultIndexSetConfig; 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"); diff --git a/graylog2-server/src/test/java/org/graylog2/migrations/V20170110150100_FixAlertConditionsMigrationTest.java b/graylog2-server/src/test/java/org/graylog2/migrations/V20170110150100_FixAlertConditionsMigrationTest.java index 661cec23f0..ab2f44af5a 100644 --- a/graylog2-server/src/test/java/org/graylog2/migrations/V20170110150100_FixAlertConditionsMigrationTest.java +++ b/graylog2-server/src/test/java/org/graylog2/migrations/V20170110150100_FixAlertConditionsMigrationTest.java @@ -32,12 +32,13 @@ import org.graylog2.events.ClusterEventBus; 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()); diff --git a/graylog2-server/src/test/java/org/graylog2/rest/resources/system/ClusterConfigResourceTest.java b/graylog2-server/src/test/java/org/graylog2/rest/resources/system/ClusterConfigResourceTest.java new file mode 100644 index 0000000000..af01de619b --- /dev/null +++ b/graylog2-server/src/test/java/org/graylog2/rest/resources/system/ClusterConfigResourceTest.java @@ -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 + * . + */ +package org.graylog2.rest.resources.system; + +import jakarta.ws.rs.BadRequestException; +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 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"); + } +}