From 75ef2b8d60e7d67f859b79fe712c8ae7b2e861d8 Mon Sep 17 00:00:00 2001 From: Bernd Ahlers Date: Tue, 6 Feb 2024 17:38:16 +0100 Subject: [PATCH] Restrict classes allowed for cluster config and event types (#18165) (#18179) * Restrict classes allowed for cluster config and event types (#18165) Add a new safe_classes configuration option to restrict the classes allowed to be used as cluster config and event types. The configuration option allows to specify a comma-separated set of prefixes matched against the fully qualified class name. For now, the default value for the configuration is org.graylog.,org.graylog2., which will allow all classes that Graylog maintains. This should work out of the box for almost all setups. Changing the default value might only be necessary if external plugins require cluster config or event types outside the "org.graylog." or "org.graylog2." namespaces. If that is the case, the configuration setting can be adjusted to cover this use case, e.b. by setting it to safe_classes = org.graylog.,org.graylog2.,custom.plugin.namespace. if said classes are located within the custom.plugin.namespace package. Refs: https://github.com/Graylog2/graylog2-server/security/advisories/GHSA-p6gg-5hf4-4rgj (cherry picked from commit 813203263b06dda18e2aed68ae92b34277f904b4) * Use javax.inject.Inject instead of jakarta.inject.Inject * Use javax.ws.rs instead of jakarta.ws.rs --------- Co-authored-by: Othello Maurer --- changelog/unreleased/pr-18165.toml | 4 + .../org/graylog/datanode/Configuration.java | 9 ++ .../main/java/org/graylog2/Configuration.java | 27 ++++++ .../cluster/ClusterConfigServiceImpl.java | 23 ++++- .../events/ClusterEventPeriodical.java | 38 +++++--- .../system/ClusterConfigResource.java | 11 ++- .../RestrictedChainingClassLoader.java | 61 ++++++++++++ .../org/graylog2/security/SafeClasses.java | 52 ++++++++++ .../UnsafeClassLoadingAttemptException.java | 29 ++++++ .../views/search/views/ViewServiceTest.java | 5 +- .../cluster/ClusterConfigServiceImplTest.java | 22 ++++- .../events/ClusterEventPeriodicalTest.java | 61 +++++++++++- .../indexset/MongoIndexSetServiceTest.java | 6 +- ...5163900_MoveIndexSetDefaultConfigTest.java | 10 +- ...50100_FixAlertConditionsMigrationTest.java | 7 +- .../system/ClusterConfigResourceTest.java | 94 +++++++++++++++++++ 16 files changed, 429 insertions(+), 30 deletions(-) create mode 100644 changelog/unreleased/pr-18165.toml create mode 100644 graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java create mode 100644 graylog2-server/src/main/java/org/graylog2/security/SafeClasses.java create mode 100644 graylog2-server/src/main/java/org/graylog2/security/UnsafeClassLoadingAttemptException.java create mode 100644 graylog2-server/src/test/java/org/graylog2/rest/resources/system/ClusterConfigResourceTest.java diff --git a/changelog/unreleased/pr-18165.toml b/changelog/unreleased/pr-18165.toml new file mode 100644 index 000000000000..5254c051e097 --- /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 1fdce5e5d455..f96ffd9c63a6 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.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; @@ -30,6 +31,7 @@ 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 safeClasses = Set.of("org.graylog.", "org.graylog2."); + public Integer getStaleLeaderTimeout() { return staleLeaderTimeout; } diff --git a/graylog2-server/src/main/java/org/graylog2/Configuration.java b/graylog2-server/src/main/java/org/graylog2/Configuration.java index 404523e91300..1b4611231e7b 100644 --- a/graylog2-server/src/main/java/org/graylog2/Configuration.java +++ b/graylog2-server/src/main/java/org/graylog2/Configuration.java @@ -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 safeClasses = Set.of("org.graylog.", "org.graylog2."); + public boolean maintainsStreamAwareFieldTypes() { return streamAwareFieldTypes; } @@ -425,6 +435,10 @@ public Duration getFailureHandlingShutdownAwait() { return failureHandlingShutdownAwait; } + public Set 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> { + @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)); + } + } + } } 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 ed04a6fe091a..e9bbde9040c3 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.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 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); @@ -174,10 +187,12 @@ public Set> 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); } } } 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 0d47f9309527..4a12530cb354 100644 --- a/graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java +++ b/graylog2-server/src/main/java/org/graylog2/events/ClusterEventPeriodical.java @@ -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 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; } } 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 813bf771127c..c666ccd0634a 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 @@ -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()); } } 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 000000000000..7eddaad8759a --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog2/security/RestrictedChainingClassLoader.java @@ -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 + * . + */ +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) + ); + } + } + +} 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 000000000000..40776408f718 --- /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 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 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 000000000000..0d73c707fd9a --- /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 84f1f1c02dc5..40637096c992 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 @@ -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( 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 0f1176d5406b..b89cbac91b22 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.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; 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 31705740479e..b84f42dee6bf 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.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(); 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 3894dbe0e899..9ed939b01183 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.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 79c1fc15ae2f..6a066d3b5630 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.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 661cec23f001..ab2f44af5a36 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.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 000000000000..c4832895e0c7 --- /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 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"); + } +}