fix: ensures that properties are runtime properties are filtered (#209)

closes: #CVE-2024-10451

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-11-18 05:32:48 -05:00 committed by GitHub
parent 7bdc16f029
commit 13833fd221
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 52 additions and 28 deletions

View File

@ -168,7 +168,13 @@ public class Picocli {
}
if (currentSpec != null) {
CommandLine commandLine = currentSpec.commandLine();
addCommandOptions(cliArgs, currentSpec.commandLine());
if (commandLine != null && commandLine.getCommand() instanceof AbstractCommand ac) {
// set current parsed command
Environment.setParsedCommand(ac);
}
}
if (isRebuildCheck()) {
@ -719,12 +725,9 @@ public class Picocli {
}
private static void addCommandOptions(List<String> cliArgs, CommandLine command) {
if (command != null && command.getCommand() instanceof AbstractCommand ac) {
if (command != null && command.getCommand() instanceof AbstractCommand) {
IncludeOptions options = getIncludeOptions(cliArgs, command.getCommand(), command.getCommandName());
// set current parsed command
Environment.setParsedCommand(ac);
if (!options.includeBuildTime && !options.includeRuntime) {
return;
}

View File

@ -21,7 +21,6 @@ import static org.keycloak.config.ClassLoaderOptions.QUARKUS_REMOVED_ARTIFACTS_P
import static org.keycloak.quarkus.runtime.Environment.getHomePath;
import static org.keycloak.quarkus.runtime.Environment.isDevProfile;
import static org.keycloak.quarkus.runtime.cli.Picocli.println;
import static org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource.getAllCliArgs;
import io.quarkus.runtime.LaunchMode;
import org.keycloak.config.OptionCategory;
@ -111,8 +110,13 @@ public final class Build extends AbstractCommand implements Runnable {
}
private void exitWithErrorIfDevProfileIsSetAndNotStartDev() {
if (Environment.isDevProfile() && !getAllCliArgs().contains(StartDev.NAME)) {
executionError(spec.commandLine(), Messages.devProfileNotAllowedError(NAME));
if (Environment.isDevProfile()) {
String cmd = Environment.getParsedCommand().map(AbstractCommand::getName).orElse(getName());
// we allow start-dev, and import|export|bootstrap-admin --profile=dev
// but not start --profile=dev, nor build --profile=dev
if (Start.NAME.equals(cmd) || Build.NAME.equals(cmd)) {
executionError(spec.commandLine(), Messages.devProfileNotAllowedError(cmd));
}
}
}

View File

@ -37,7 +37,6 @@ import io.smallrye.config.ConfigValue;
import org.keycloak.config.DeprecatedMetadata;
import org.keycloak.config.Option;
import org.keycloak.config.OptionBuilder;
import org.keycloak.config.OptionCategory;
import org.keycloak.quarkus.runtime.cli.PropertyException;
import org.keycloak.quarkus.runtime.cli.ShortErrorMessageHandler;
@ -50,23 +49,6 @@ import org.keycloak.utils.StringUtil;
public class PropertyMapper<T> {
static PropertyMapper<?> IDENTITY = new PropertyMapper<>(
new OptionBuilder<>(null, String.class).build(),
null,
() -> false,
"",
null,
null,
null,
false,
null,
null) {
@Override
public ConfigValue getConfigValue(String name, ConfigSourceInterceptorContext context) {
return context.proceed(name);
}
};
private final Option<T> option;
private final String to;
private BooleanSupplier enabled;

View File

@ -13,8 +13,8 @@ import org.keycloak.quarkus.runtime.cli.PropertyException;
import org.keycloak.quarkus.runtime.cli.command.AbstractCommand;
import org.keycloak.quarkus.runtime.cli.command.Build;
import org.keycloak.quarkus.runtime.cli.command.ShowConfig;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.DisabledMappersInterceptor;
import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource;
import java.util.ArrayList;
@ -37,7 +37,7 @@ import static org.keycloak.quarkus.runtime.Environment.isRebuildCheck;
import static org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider.isKeyStoreConfigSource;
public final class PropertyMappers {
public static final String KC_SPI_PREFIX = "kc.spi";
public static String VALUE_MASK = "*******";
private static MappersConfig MAPPERS;
@ -74,7 +74,22 @@ public final class PropertyMappers {
}
public static ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
return getMapperOrDefault(name, PropertyMapper.IDENTITY).getConfigValue(name, context);
PropertyMapper<?> mapper = getMapper(name);
// during re-aug do not resolve the server runtime properties and avoid they included by quarkus in the default value config source
if ((isRebuild() || Environment.isRebuildCheck()) && isKeycloakRuntime(name, mapper)) {
return ConfigValue.builder().withName(name).build();
}
if (mapper == null) {
return context.proceed(name);
}
return mapper.getConfigValue(name, context);
}
private static boolean isKeycloakRuntime(String name, PropertyMapper<?> mapper) {
if (mapper == null) {
return name.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK) && !isSpiBuildTimeProperty(name);
}
return mapper.isRunTime();
}
public static boolean isBuildTimeProperty(String name) {

View File

@ -238,4 +238,24 @@ public class StartCommandDistTest {
cliResult.assertError("File specified via '--config-file' or '-cf' option does not exist.");
cliResult.assertError(String.format("Try '%s --help' for more information on the available options.", KeycloakDistribution.SCRIPT_CMD));
}
@RawDistOnly(reason = "Containers are immutable")
@Test
void testRuntimeValuesAreNotCaptured(KeycloakDistribution dist) {
// confirm that the invalid value prevents startup - if this passes, then we need to use a different
// spi provider
CLIResult cliResult = dist.run("start", "--spi-events-listener-jboss-logging-success-level=invalid", "--http-enabled", "true", "--hostname-strict", "false");
cliResult.assertError("Failed to start quarkus");
// if there was no auto-build use an explicit build to potentially capture the runtime default
if (!cliResult.getOutput().contains("Server configuration updated and persisted")) {
cliResult = dist.run("build", "--spi-events-listener-jboss-logging-success-level=invalid");
cliResult.assertBuild();
}
// the invalid value should not be the default
cliResult = dist.run("start", "--http-enabled", "true", "--hostname-strict", "false");
cliResult.assertNoBuild();
cliResult.assertStarted();
}
}