Skip to content

Commit

Permalink
feat: disable dynamic code loading properties by default (#2606)
Browse files Browse the repository at this point in the history
Disable Connection URL properties that dynamically invoke code by
default. These properties can now only be used if the corresponding
System property has been enabled. This gives users control over whether
they want to enable these properties in their applications or not.

You should only enable the use of these properties as long as untrusted
end users cannot dynamically set these. That is; If your application is
a public service that allows end users to set a Connection URL, then you
should not enable the use of these properties, as it would allow a user
to try to specify a credentials or channel provider class that is not a
valid provider. The constructor of the selected class would in that case
still be invoked.
  • Loading branch information
olavloite committed Sep 11, 2023
1 parent 76fbae4 commit d855ebb
Show file tree
Hide file tree
Showing 3 changed files with 298 additions and 139 deletions.
Expand Up @@ -36,6 +36,7 @@
import com.google.cloud.spanner.SpannerOptions;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions;
import java.io.IOException;
Expand Down Expand Up @@ -205,8 +206,14 @@ public String[] getValidValues() {
public static final String CREDENTIALS_PROPERTY_NAME = "credentials";
/** Name of the 'encodedCredentials' connection property. */
public static final String ENCODED_CREDENTIALS_PROPERTY_NAME = "encodedCredentials";

public static final String ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY =
"ENABLE_ENCODED_CREDENTIALS";
/** Name of the 'credentialsProvider' connection property. */
public static final String CREDENTIALS_PROVIDER_PROPERTY_NAME = "credentialsProvider";

public static final String ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY =
"ENABLE_CREDENTIALS_PROVIDER";
/**
* OAuth token to use for authentication. Cannot be used in combination with a credentials file.
*/
Expand All @@ -219,6 +226,8 @@ public String[] getValidValues() {
public static final String NUM_CHANNELS_PROPERTY_NAME = "numChannels";
/** Name of the 'channelProvider' connection property. */
public static final String CHANNEL_PROVIDER_PROPERTY_NAME = "channelProvider";

public static final String ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY = "ENABLE_CHANNEL_PROVIDER";
/** Custom user agent string is only for other Google libraries. */
private static final String USER_AGENT_PROPERTY_NAME = "userAgent";
/** Query optimizer version to use for a connection. */
Expand Down Expand Up @@ -248,6 +257,19 @@ public String[] getValidValues() {
public static final String MAX_PARTITIONED_PARALLELISM_PROPERTY_NAME =
"maxPartitionedParallelism";

private static final String GUARDED_CONNECTION_PROPERTY_ERROR_MESSAGE =
"%s can only be used if the system property %s has been set to true. "
+ "Start the application with the JVM command line option -D%s=true";

private static String generateGuardedConnectionPropertyError(
String systemPropertyName, String connectionPropertyName) {
return String.format(
GUARDED_CONNECTION_PROPERTY_ERROR_MESSAGE,
connectionPropertyName,
systemPropertyName,
systemPropertyName);
}

/** All valid connection properties. */
public static final Set<ConnectionProperty> VALID_PROPERTIES =
Collections.unmodifiableSet(
Expand Down Expand Up @@ -846,40 +868,64 @@ static boolean parseRetryAbortsInternally(String uri) {

@VisibleForTesting
static @Nullable String parseEncodedCredentials(String uri) {
return parseUriProperty(uri, ENCODED_CREDENTIALS_PROPERTY_NAME);
String encodedCredentials = parseUriProperty(uri, ENCODED_CREDENTIALS_PROPERTY_NAME);
checkGuardedProperty(
encodedCredentials,
ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
ENCODED_CREDENTIALS_PROPERTY_NAME);
return encodedCredentials;
}

@VisibleForTesting
static @Nullable CredentialsProvider parseCredentialsProvider(String uri) {
String name = parseUriProperty(uri, CREDENTIALS_PROVIDER_PROPERTY_NAME);
if (name != null) {
String credentialsProviderName = parseUriProperty(uri, CREDENTIALS_PROVIDER_PROPERTY_NAME);
checkGuardedProperty(
credentialsProviderName,
ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY,
CREDENTIALS_PROVIDER_PROPERTY_NAME);
if (!Strings.isNullOrEmpty(credentialsProviderName)) {
try {
Class<? extends CredentialsProvider> clazz =
(Class<? extends CredentialsProvider>) Class.forName(name);
(Class<? extends CredentialsProvider>) Class.forName(credentialsProviderName);
Constructor<? extends CredentialsProvider> constructor = clazz.getDeclaredConstructor();
return constructor.newInstance();
} catch (ClassNotFoundException classNotFoundException) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
"Unknown or invalid CredentialsProvider class name: " + name,
"Unknown or invalid CredentialsProvider class name: " + credentialsProviderName,
classNotFoundException);
} catch (NoSuchMethodException noSuchMethodException) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
"Credentials provider " + name + " does not have a public no-arg constructor.",
"Credentials provider "
+ credentialsProviderName
+ " does not have a public no-arg constructor.",
noSuchMethodException);
} catch (InvocationTargetException
| InstantiationException
| IllegalAccessException exception) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
"Failed to create an instance of " + name + ": " + exception.getMessage(),
"Failed to create an instance of "
+ credentialsProviderName
+ ": "
+ exception.getMessage(),
exception);
}
}
return null;
}

private static void checkGuardedProperty(
String value, String systemPropertyName, String connectionPropertyName) {
if (!Strings.isNullOrEmpty(value)
&& !Boolean.parseBoolean(System.getProperty(systemPropertyName))) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
generateGuardedConnectionPropertyError(systemPropertyName, connectionPropertyName));
}
}

@VisibleForTesting
static @Nullable String parseOAuthToken(String uri) {
String value = parseUriProperty(uri, OAUTH_TOKEN_PROPERTY_NAME);
Expand Down Expand Up @@ -907,6 +953,8 @@ static String parseNumChannels(String uri) {
@VisibleForTesting
static String parseChannelProvider(String uri) {
String value = parseUriProperty(uri, CHANNEL_PROVIDER_PROPERTY_NAME);
checkGuardedProperty(
value, ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY, CHANNEL_PROVIDER_PROPERTY_NAME);
return value != null ? value : DEFAULT_CHANNEL_PROVIDER;
}

Expand Down

0 comments on commit d855ebb

Please sign in to comment.