Skip to content

Commit 3f65378

Browse files
committed
Require explict user-consent to enable HadoopFileIO
Using `HadoopFileIO` in Polaris can enable "hidden features" that users are likely not aware of. This change requires users to manually update the configuration to be able to use `HadoopFileIO` in way that highlights the consequences of enabling it. This PR updates Polaris in multiple ways: * The default of `SUPPORTED_CATALOG_STORAGE_TYPES` is changed to not include the `FILE` storage type. * Respect the `ALLOW_SPECIFYING_FILE_IO_IMPL` configuration on namespaces, tables and views to prevent setting an `io-impl` value for anything but one of the configured, supported storage-types. * Unify validation code in a new class `IcebergPropertiesValidation`. * Using `FILE` or `HadoopFileIO` now _also_ requires the explicit configuration `ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS=true`. * Added production readiness checks that trigger when `ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS` is `true` or `SUPPORTED_CATALOG_STORAGE_TYPES` contains `FILE` (defaults and per-realm). * The two new readiness checks are considered _severe_. Severe readiness-errors prevent the server from starting up - unless the user explicitly configured `polaris.readiness.ignore-security-issues=true`. Log messages and configuration options explicitly use "clear" phrases highlighting the consequences. With these changes it is intentionally extremely difficult to start Polaris with HadoopFileIO. People who work around all these safety nets must have realized that what they are doing. A lot of the test code relies on `FILE`/`HadoopFileIO`, those tests got all the configurations to let those tests continue to work as they are, bypassing the added security safeguards.
1 parent 4d0f258 commit 3f65378

26 files changed

+557
-120
lines changed

plugins/spark/v3.5/regtests/docker-compose.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ services:
2828
POLARIS_BOOTSTRAP_CREDENTIALS: POLARIS,root,secret
2929
quarkus.log.file.enable: "false"
3030
quarkus.otel.sdk.disabled: "true"
31+
polaris.features.defaults."ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS": "true"
32+
polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES": "[\"FILE\",\"S3\",\"GCS\",\"AZURE\"]"
33+
polaris.readiness.ignore-security-issues: "true"
3134
healthcheck:
3235
test: ["CMD", "curl", "http://localhost:8182/q/health"]
3336
interval: 10s

polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,7 @@ public static void enforceFeatureEnabledOrThrow(
146146
List.of(
147147
StorageConfigInfo.StorageTypeEnum.S3.name(),
148148
StorageConfigInfo.StorageTypeEnum.AZURE.name(),
149-
StorageConfigInfo.StorageTypeEnum.GCS.name(),
150-
StorageConfigInfo.StorageTypeEnum.FILE.name()))
149+
StorageConfigInfo.StorageTypeEnum.GCS.name()))
151150
.buildFeatureConfiguration();
152151

153152
public static final FeatureConfiguration<Boolean> CLEANUP_ON_NAMESPACE_DROP =
@@ -234,4 +233,38 @@ public static void enforceFeatureEnabledOrThrow(
234233
.description("If true, the policy-store endpoints are enabled")
235234
.defaultValue(true)
236235
.buildFeatureConfiguration();
236+
237+
public static final FeatureConfiguration<Boolean> ALLOW_SPECIFYING_FILE_IO_IMPL =
238+
PolarisConfiguration.<Boolean>builder()
239+
.key("ALLOW_SPECIFYING_FILE_IO_IMPL")
240+
.description(
241+
"Config key for whether to allow setting the FILE_IO_IMPL using catalog properties. "
242+
+ "Must only be enabled in dev/test environments, never in production systems.")
243+
.defaultValue(false)
244+
.buildFeatureConfiguration();
245+
246+
public static final FeatureConfiguration<Boolean>
247+
ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS =
248+
PolarisConfiguration.<Boolean>builder()
249+
.key("ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS")
250+
.description(
251+
"Allow usage of FileIO implementations that are considered insecure. "
252+
+ "Enabling this setting exposes the service to SEVERE security risks, including "
253+
+ "denial of service, corruption, data loss and more!"
254+
+ "This must NEVER be set to 'true' for anything except tests! ")
255+
.defaultValue(false)
256+
.buildFeatureConfiguration();
257+
258+
public static final FeatureConfiguration<Boolean> INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST =
259+
PolarisConfiguration.<Boolean>builder()
260+
.key("INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST")
261+
.description(
262+
"Config key for initializing a default \"catalogFileIO\" that is available either via "
263+
+ "getIo() or for any TableOperations/ViewOperations instantiated, via ops.io() "
264+
+ "before entity-specific FileIO initialization is triggered for any such operations. "
265+
+ "Typically this should only be used in test scenarios where a PolarisIcebergCatalog "
266+
+ "instance is used for both the \"client-side\" and \"server-side\" logic instead of "
267+
+ "being access through a REST layer.")
268+
.defaultValue(false)
269+
.buildFeatureConfiguration();
237270
}

polaris-core/src/main/java/org/apache/polaris/core/config/ProductionReadinessCheck.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,20 @@ default boolean ready() {
4848
interface Error {
4949

5050
static Error of(String message, String offendingProperty) {
51-
return ImmutableError.of(message, offendingProperty);
51+
return ImmutableError.of(message, offendingProperty, false);
52+
}
53+
54+
static Error ofSevere(String message, String offendingProperty) {
55+
return ImmutableError.of(message, offendingProperty, true);
5256
}
5357

5458
@Value.Parameter(order = 1)
5559
String message();
5660

5761
@Value.Parameter(order = 2)
5862
String offendingProperty();
63+
64+
@Value.Parameter(order = 3)
65+
boolean severe();
5966
}
6067
}

quarkus/defaults/src/main/resources/application-it.properties

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@ polaris.features.defaults."ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING"=false
3535
polaris.features.defaults."ALLOW_EXTERNAL_METADATA_FILE_LOCATION"=false
3636
polaris.features.defaults."ALLOW_OVERLAPPING_CATALOG_URLS"=true
3737
polaris.features.defaults."ALLOW_SPECIFYING_FILE_IO_IMPL"=true
38+
polaris.features.defaults."ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS"=true
3839
polaris.features.defaults."ALLOW_WILDCARD_LOCATION"=true
3940
polaris.features.defaults."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=true
4041
polaris.features.defaults."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_it"=true
4142
polaris.features.defaults."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true
4243
polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["FILE","S3","GCS","AZURE"]
4344
polaris.features.defaults."ENABLE_CATALOG_FEDERATION"=true
45+
polaris.readiness.ignore-security-issues=true
4446

4547
polaris.realm-context.realms=POLARIS,OTHER
4648

quarkus/defaults/src/main/resources/application.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ polaris.realm-context.header-name=Polaris-Realm
109109
polaris.realm-context.require-header=false
110110

111111
polaris.features.defaults."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=false
112-
polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE","FILE"]
112+
polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE"]
113113
# polaris.features.defaults."ENABLE_CATALOG_FEDERATION"=true
114114

115115
# realm overrides

quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java

Lines changed: 113 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
*/
1919
package org.apache.polaris.service.quarkus.config;
2020

21+
import static java.lang.String.format;
22+
23+
import com.fasterxml.jackson.databind.ObjectMapper;
2124
import jakarta.enterprise.context.ApplicationScoped;
2225
import jakarta.enterprise.event.Observes;
2326
import jakarta.enterprise.event.Startup;
@@ -27,12 +30,15 @@
2730
import java.nio.charset.StandardCharsets;
2831
import java.util.ArrayList;
2932
import java.util.List;
33+
import org.apache.polaris.core.config.FeatureConfiguration;
3034
import org.apache.polaris.core.config.ProductionReadinessCheck;
3135
import org.apache.polaris.core.config.ProductionReadinessCheck.Error;
3236
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
3337
import org.apache.polaris.service.auth.AuthenticationRealmConfiguration.TokenBrokerConfiguration.RSAKeyPairConfiguration;
3438
import org.apache.polaris.service.auth.AuthenticationRealmConfiguration.TokenBrokerConfiguration.SymmetricKeyConfiguration;
3539
import org.apache.polaris.service.auth.AuthenticationType;
40+
import org.apache.polaris.service.catalog.validation.IcebergPropertiesValidation;
41+
import org.apache.polaris.service.config.FeaturesConfiguration;
3642
import org.apache.polaris.service.context.DefaultRealmContextResolver;
3743
import org.apache.polaris.service.context.RealmContextResolver;
3844
import org.apache.polaris.service.context.TestRealmContextResolver;
@@ -42,6 +48,7 @@
4248
import org.eclipse.microprofile.config.ConfigValue;
4349
import org.slf4j.Logger;
4450
import org.slf4j.LoggerFactory;
51+
import org.slf4j.event.Level;
4552

4653
@ApplicationScoped
4754
public class ProductionReadinessChecks {
@@ -55,26 +62,53 @@ public class ProductionReadinessChecks {
5562
*/
5663
private static final String WARNING_SIGN_UTF_8 = "\u0000\u26A0\uFE0F";
5764

65+
private static final String SEVERE_SIGN_UTF_8 = "\u0000\uD83D\uDED1";
66+
5867
/** A simple warning sign displayed when the character set is not UTF-8. */
5968
private static final String WARNING_SIGN_PLAIN = "!!!";
6069

70+
private static final String SEVERE_SIGN_PLAIN = "***STOP***";
71+
6172
public void warnOnFailedChecks(
62-
@Observes Startup event, Instance<ProductionReadinessCheck> checks) {
73+
@Observes Startup event,
74+
Instance<ProductionReadinessCheck> checks,
75+
QuarkusReadinessConfiguration config) {
6376
List<Error> errors = checks.stream().flatMap(check -> check.getErrors().stream()).toList();
6477
if (!errors.isEmpty()) {
65-
String warning =
66-
Charset.defaultCharset().equals(StandardCharsets.UTF_8)
67-
? WARNING_SIGN_UTF_8
68-
: WARNING_SIGN_PLAIN;
69-
LOGGER.warn("{} Production readiness checks failed! Check the warnings below.", warning);
78+
var utf8 = Charset.defaultCharset().equals(StandardCharsets.UTF_8);
79+
var warning = utf8 ? WARNING_SIGN_UTF_8 : WARNING_SIGN_PLAIN;
80+
var severe = utf8 ? SEVERE_SIGN_UTF_8 : SEVERE_SIGN_PLAIN;
81+
var hasSevere = errors.stream().anyMatch(Error::severe);
82+
LOGGER
83+
.makeLoggingEventBuilder(hasSevere ? Level.ERROR : Level.WARN)
84+
.log(
85+
"{} Production readiness checks failed! Check the warnings below.",
86+
hasSevere ? severe : warning);
7087
errors.forEach(
7188
error ->
72-
LOGGER.warn(
73-
"- {} Offending configuration option: '{}'.",
74-
error.message(),
75-
error.offendingProperty()));
76-
LOGGER.warn(
77-
"Refer to https://polaris.apache.org/in-dev/unreleased/configuring-polaris-for-production for more information.");
89+
LOGGER
90+
.makeLoggingEventBuilder(error.severe() ? Level.ERROR : Level.WARN)
91+
.log(
92+
"- {} {} Offending configuration option: '{}'.",
93+
error.severe() ? severe : warning,
94+
error.message(),
95+
error.offendingProperty()));
96+
LOGGER
97+
.makeLoggingEventBuilder(hasSevere ? Level.ERROR : Level.WARN)
98+
.log(
99+
"Refer to https://polaris.apache.org/in-dev/unreleased/configuring-polaris-for-production for more information.");
100+
101+
if (hasSevere) {
102+
if (!config.ignoreSecurityIssues()) {
103+
throw new IllegalStateException(
104+
"Severe production readiness issues detected, startup aborted!");
105+
}
106+
LOGGER.error(
107+
"{} severe production readiness issues detected, but user explicitly requested startup by setting "
108+
+ "polaris.readiness.ignore-security-issues=true and accepts the risk of denial-of-service, "
109+
+ "data-loss, corruption and others !",
110+
severe);
111+
}
78112
}
79113
}
80114

@@ -164,4 +198,71 @@ public ProductionReadinessCheck checkRealmResolver(Config config, RealmContextRe
164198
private static String authRealmSegment(String realm) {
165199
return realm.equals(QuarkusAuthenticationConfiguration.DEFAULT_REALM_KEY) ? "" : realm + ".";
166200
}
201+
202+
@Produces
203+
public ProductionReadinessCheck checkInsecureStorageSettings(
204+
FeaturesConfiguration featureConfiguration) {
205+
var insecure = FeatureConfiguration.ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS;
206+
207+
var errors = new ArrayList<Error>();
208+
if (Boolean.parseBoolean(featureConfiguration.defaults().get(insecure.key))) {
209+
errors.add(
210+
Error.ofSevere(
211+
"Must not enable a configuration that exposes known and severe security risks: "
212+
+ insecure.description,
213+
format("polaris.features.defaults.\"%s\"", insecure.key)));
214+
}
215+
216+
featureConfiguration
217+
.realmOverrides()
218+
.forEach(
219+
(realmId, overrides) -> {
220+
if (Boolean.parseBoolean(overrides.overrides().get(insecure.key))) {
221+
errors.add(
222+
Error.ofSevere(
223+
"Must not enable a configuration that exposes known and severe security risks: "
224+
+ insecure.description,
225+
format(
226+
"polaris.features.realm-overrides.\"%s\".overrides.\"%s\"",
227+
realmId, insecure.key)));
228+
}
229+
});
230+
231+
var storageTypes = FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES;
232+
var mapper = new ObjectMapper();
233+
var defaults = featureConfiguration.parseDefaults(mapper);
234+
var realmOverrides = featureConfiguration.parseRealmOverrides(mapper);
235+
@SuppressWarnings("unchecked")
236+
var supported = (List<String>) defaults.getOrDefault(storageTypes.key, List.of());
237+
supported.stream()
238+
.filter(n -> !IcebergPropertiesValidation.safeStorageType(n))
239+
.forEach(
240+
t ->
241+
errors.add(
242+
Error.ofSevere(
243+
format(
244+
"The storage type '%s' is considered insecure and to expose the service to severe security ricks!",
245+
t),
246+
format("polaris.features.defaults.\"%s\"", storageTypes.key))));
247+
realmOverrides.forEach(
248+
(realmId, overrides) -> {
249+
@SuppressWarnings("unchecked")
250+
var s = (List<String>) overrides.getOrDefault(storageTypes.key, List.of());
251+
s.stream()
252+
.filter(n -> !IcebergPropertiesValidation.safeStorageType(n))
253+
.forEach(
254+
t ->
255+
errors.add(
256+
Error.ofSevere(
257+
format(
258+
"The storage type '%s' is considered insecure and to expose the service to severe security ricks!",
259+
t),
260+
format(
261+
"polaris.features.realm-overrides.\"%s\".overrides.\"%s\"",
262+
realmId, storageTypes.key))));
263+
});
264+
return errors.isEmpty()
265+
? ProductionReadinessCheck.OK
266+
: ProductionReadinessCheck.of(errors.toArray(new Error[0]));
267+
}
167268
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.polaris.service.quarkus.config;
20+
21+
import io.quarkus.runtime.annotations.StaticInitSafe;
22+
import io.smallrye.config.ConfigMapping;
23+
import io.smallrye.config.WithDefault;
24+
25+
@StaticInitSafe
26+
@ConfigMapping(prefix = "polaris.readiness")
27+
public interface QuarkusReadinessConfiguration {
28+
29+
/**
30+
* Setting this to {@code true} means that Polaris will start up even if severe security risks
31+
* have been detected, accepting the risk of denial-of-service, data-loss, corruption and other
32+
* risks.
33+
*/
34+
@WithDefault("false")
35+
boolean ignoreSecurityIssues();
36+
}

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,13 @@ public Map<String, String> getConfigOverrides() {
113113
return Map.of(
114114
"polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"",
115115
"true",
116+
"polaris.features.defaults.\"ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS\"",
117+
"true",
116118
"polaris.features.defaults.\"ALLOW_EXTERNAL_METADATA_FILE_LOCATION\"",
119+
"true",
120+
"polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
121+
"[\"FILE\",\"S3\"]",
122+
"polaris.readiness.ignore-security-issues",
117123
"true");
118124
}
119125
}
@@ -220,9 +226,16 @@ public void before(TestInfo testInfo) {
220226

221227
Map<String, Object> configMap =
222228
Map.of(
223-
"ALLOW_SPECIFYING_FILE_IO_IMPL", true,
224-
"ALLOW_EXTERNAL_METADATA_FILE_LOCATION", true,
225-
"ENABLE_GENERIC_TABLES", true);
229+
"ALLOW_SPECIFYING_FILE_IO_IMPL",
230+
true,
231+
"ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS",
232+
true,
233+
"ALLOW_EXTERNAL_METADATA_FILE_LOCATION",
234+
true,
235+
"SUPPORTED_CATALOG_STORAGE_TYPES",
236+
List.of("FILE", "S3"),
237+
"ENABLE_GENERIC_TABLES",
238+
true);
226239
polarisContext =
227240
new PolarisCallContext(
228241
managerFactory.getOrCreateSessionSupplier(realmContext).get(),

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.assertj.core.api.Assertions.assertThat;
2525

2626
import jakarta.ws.rs.core.Response;
27+
import java.util.List;
2728
import java.util.Map;
2829
import java.util.UUID;
2930
import java.util.stream.Stream;
@@ -73,10 +74,25 @@ private int createTable(TestServices services, String location) {
7374

7475
static Stream<Arguments> testTableLocationRestrictions() {
7576
Map<String, Object> laxServices =
76-
Map.of("ALLOW_UNSTRUCTURED_TABLE_LOCATION", "true", "ALLOW_TABLE_LOCATION_OVERLAP", "true");
77+
Map.of(
78+
"ALLOW_UNSTRUCTURED_TABLE_LOCATION",
79+
"true",
80+
"ALLOW_TABLE_LOCATION_OVERLAP",
81+
"true",
82+
"ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS",
83+
"true",
84+
"SUPPORTED_CATALOG_STORAGE_TYPES",
85+
List.of("FILE", "S3"));
7786
Map<String, Object> strictServices =
7887
Map.of(
79-
"ALLOW_UNSTRUCTURED_TABLE_LOCATION", "false", "ALLOW_TABLE_LOCATION_OVERLAP", "false");
88+
"ALLOW_UNSTRUCTURED_TABLE_LOCATION",
89+
"false",
90+
"ALLOW_TABLE_LOCATION_OVERLAP",
91+
"false",
92+
"ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS",
93+
"true",
94+
"SUPPORTED_CATALOG_STORAGE_TYPES",
95+
List.of("FILE", "S3"));
8096
Map<String, Object> laxCatalog =
8197
Map.of(
8298
ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(),

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,14 @@ public Map<String, String> getConfigOverrides() {
106106
return Map.of(
107107
"polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"",
108108
"true",
109+
"polaris.features.defaults.\"ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS\"",
110+
"true",
109111
"polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"",
110112
"true",
111113
"polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
112-
"[\"FILE\"]");
114+
"[\"FILE\"]",
115+
"polaris.readiness.ignore-security-issues",
116+
"true");
113117
}
114118
}
115119

0 commit comments

Comments
 (0)