Skip to content

Commit

Permalink
Enforce allow list for JDBC properties by default (apache#11063)
Browse files Browse the repository at this point in the history
* Enforce allow list for JDBC properties by default

* fix tests
  • Loading branch information
jihoonson authored Apr 7, 2021
1 parent 053af68 commit cc12a57
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 12 deletions.
4 changes: 2 additions & 2 deletions docs/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ the [HTTP input source](../ingestion/native-batch.md#http-input-source) and the
|`druid.ingestion.http.allowedProtocols`|List of protocols|Allowed protocols for the HTTP input source and HTTP firehose.|["http", "https"]|


### Ingestion Security Configuration
### External Data Access Security Configuration

#### JDBC Connections to External Databases

Expand All @@ -551,7 +551,7 @@ These properties do not apply to metadata storage connections.

|Property|Possible Values|Description|Default|
|--------|---------------|-----------|-------|
|`druid.access.jdbc.enforceAllowedProperties`|Boolean|When true, Druid applies `druid.access.jdbc.allowedProperties` to JDBC connections starting with `jdbc:postgresql:` or `jdbc:mysql:`. When false, Druid allows any kind of JDBC connections without JDBC property validation. This config is deprecated and will be removed in a future release.|false|
|`druid.access.jdbc.enforceAllowedProperties`|Boolean|When true, Druid applies `druid.access.jdbc.allowedProperties` to JDBC connections starting with `jdbc:postgresql:` or `jdbc:mysql:`. When false, Druid allows any kind of JDBC connections without JDBC property validation. This config is for backward compatibility especially during upgrades since enforcing allow list can break existing ingestion jobs or lookups based on JDBC. This config is deprecated and will be removed in a future release.|true|
|`druid.access.jdbc.allowedProperties`|List of JDBC properties|Defines a list of allowed JDBC properties. Druid always enforces the list for all JDBC connections starting with `jdbc:postgresql:` or `jdbc:mysql:` if `druid.access.jdbc.enforceAllowedProperties` is set to true.<br/><br/>This option is tested against MySQL connector 5.1.48 and PostgreSQL connector 42.2.14. Other connector versions might not work.|["useSSL", "requireSSL", "ssl", "sslmode"]|
|`druid.access.jdbc.allowUnknownJdbcUrlFormat`|Boolean|When false, Druid only accepts JDBC connections starting with `jdbc:postgresql:` or `jdbc:mysql:`. When true, Druid allows JDBC connections to any kind of database, but only enforces `druid.access.jdbc.allowedProperties` for PostgreSQL and MySQL.|true|

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ public Set<String> getAllowedProperties()
{
return ImmutableSet.of("user", "nonenone");
}

@Override
public boolean isEnforceAllowedProperties()
{
return false;
}
};

new MySQLFirehoseDatabaseConnector(
Expand All @@ -205,13 +211,12 @@ public String getConnectURI()
}
};

MySQLFirehoseDatabaseConnector connector = new MySQLFirehoseDatabaseConnector(
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(StringUtils.format("Invalid URL format for MySQL: [%s]", url));
new MySQLFirehoseDatabaseConnector(
connectorConfig,
new JdbcAccessSecurityConfig()
);
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(StringUtils.format("Invalid URL format for MySQL: [%s]", url));
connector.findPropertyKeysFromConnectURL(url);
}

private static JdbcAccessSecurityConfig newSecurityConfigEnforcingAllowList(Set<String> allowedProperties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,12 @@ public Set<String> getAllowedProperties()
{
return ImmutableSet.of("user", "nonenone");
}

@Override
public boolean isEnforceAllowedProperties()
{
return false;
}
};

new PostgresqlFirehoseDatabaseConnector(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,12 @@ public class JdbcAccessSecurityConfig
@JsonProperty
private boolean allowUnknownJdbcUrlFormat = true;

// Enforcing allow list check can break rolling upgrade. This is not good for patch releases
// and is why this config is added. However, from the security point of view, this config
// should be always enabled in production to secure your cluster. As a result, this config
// is deprecated and will be removed in the near future.
// This config is for compatibility as enforcing allow list can break existing ingestion jobs or lookups.
// However, from the security point of view, this config should be always enabled in production to secure
// your cluster. As a result, this config is deprecated and will be removed in a future release.
@Deprecated
@JsonProperty
private boolean enforceAllowedProperties = false;
private boolean enforceAllowedProperties = true;

@JsonIgnore
public Set<String> getSystemPropertyPrefixes()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void testSecurityConfigDefault()
securityConfig.getAllowedProperties()
);
Assert.assertTrue(securityConfig.isAllowUnknownJdbcUrlFormat());
Assert.assertFalse(securityConfig.isEnforceAllowedProperties());
Assert.assertTrue(securityConfig.isEnforceAllowedProperties());
}

@Test
Expand Down

0 comments on commit cc12a57

Please sign in to comment.