Skip to content

Commit d855ebb

Browse files
authored
feat: disable dynamic code loading properties by default (googleapis#2606)
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.
1 parent 76fbae4 commit d855ebb

File tree

3 files changed

+298
-139
lines changed

3 files changed

+298
-139
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java

+55-7
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.google.cloud.spanner.SpannerOptions;
3737
import com.google.common.annotations.VisibleForTesting;
3838
import com.google.common.base.Preconditions;
39+
import com.google.common.base.Strings;
3940
import com.google.common.collect.Sets;
4041
import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions;
4142
import java.io.IOException;
@@ -205,8 +206,14 @@ public String[] getValidValues() {
205206
public static final String CREDENTIALS_PROPERTY_NAME = "credentials";
206207
/** Name of the 'encodedCredentials' connection property. */
207208
public static final String ENCODED_CREDENTIALS_PROPERTY_NAME = "encodedCredentials";
209+
210+
public static final String ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY =
211+
"ENABLE_ENCODED_CREDENTIALS";
208212
/** Name of the 'credentialsProvider' connection property. */
209213
public static final String CREDENTIALS_PROVIDER_PROPERTY_NAME = "credentialsProvider";
214+
215+
public static final String ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY =
216+
"ENABLE_CREDENTIALS_PROVIDER";
210217
/**
211218
* OAuth token to use for authentication. Cannot be used in combination with a credentials file.
212219
*/
@@ -219,6 +226,8 @@ public String[] getValidValues() {
219226
public static final String NUM_CHANNELS_PROPERTY_NAME = "numChannels";
220227
/** Name of the 'channelProvider' connection property. */
221228
public static final String CHANNEL_PROVIDER_PROPERTY_NAME = "channelProvider";
229+
230+
public static final String ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY = "ENABLE_CHANNEL_PROVIDER";
222231
/** Custom user agent string is only for other Google libraries. */
223232
private static final String USER_AGENT_PROPERTY_NAME = "userAgent";
224233
/** Query optimizer version to use for a connection. */
@@ -248,6 +257,19 @@ public String[] getValidValues() {
248257
public static final String MAX_PARTITIONED_PARALLELISM_PROPERTY_NAME =
249258
"maxPartitionedParallelism";
250259

260+
private static final String GUARDED_CONNECTION_PROPERTY_ERROR_MESSAGE =
261+
"%s can only be used if the system property %s has been set to true. "
262+
+ "Start the application with the JVM command line option -D%s=true";
263+
264+
private static String generateGuardedConnectionPropertyError(
265+
String systemPropertyName, String connectionPropertyName) {
266+
return String.format(
267+
GUARDED_CONNECTION_PROPERTY_ERROR_MESSAGE,
268+
connectionPropertyName,
269+
systemPropertyName,
270+
systemPropertyName);
271+
}
272+
251273
/** All valid connection properties. */
252274
public static final Set<ConnectionProperty> VALID_PROPERTIES =
253275
Collections.unmodifiableSet(
@@ -846,40 +868,64 @@ static boolean parseRetryAbortsInternally(String uri) {
846868

847869
@VisibleForTesting
848870
static @Nullable String parseEncodedCredentials(String uri) {
849-
return parseUriProperty(uri, ENCODED_CREDENTIALS_PROPERTY_NAME);
871+
String encodedCredentials = parseUriProperty(uri, ENCODED_CREDENTIALS_PROPERTY_NAME);
872+
checkGuardedProperty(
873+
encodedCredentials,
874+
ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
875+
ENCODED_CREDENTIALS_PROPERTY_NAME);
876+
return encodedCredentials;
850877
}
851878

852879
@VisibleForTesting
853880
static @Nullable CredentialsProvider parseCredentialsProvider(String uri) {
854-
String name = parseUriProperty(uri, CREDENTIALS_PROVIDER_PROPERTY_NAME);
855-
if (name != null) {
881+
String credentialsProviderName = parseUriProperty(uri, CREDENTIALS_PROVIDER_PROPERTY_NAME);
882+
checkGuardedProperty(
883+
credentialsProviderName,
884+
ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY,
885+
CREDENTIALS_PROVIDER_PROPERTY_NAME);
886+
if (!Strings.isNullOrEmpty(credentialsProviderName)) {
856887
try {
857888
Class<? extends CredentialsProvider> clazz =
858-
(Class<? extends CredentialsProvider>) Class.forName(name);
889+
(Class<? extends CredentialsProvider>) Class.forName(credentialsProviderName);
859890
Constructor<? extends CredentialsProvider> constructor = clazz.getDeclaredConstructor();
860891
return constructor.newInstance();
861892
} catch (ClassNotFoundException classNotFoundException) {
862893
throw SpannerExceptionFactory.newSpannerException(
863894
ErrorCode.INVALID_ARGUMENT,
864-
"Unknown or invalid CredentialsProvider class name: " + name,
895+
"Unknown or invalid CredentialsProvider class name: " + credentialsProviderName,
865896
classNotFoundException);
866897
} catch (NoSuchMethodException noSuchMethodException) {
867898
throw SpannerExceptionFactory.newSpannerException(
868899
ErrorCode.INVALID_ARGUMENT,
869-
"Credentials provider " + name + " does not have a public no-arg constructor.",
900+
"Credentials provider "
901+
+ credentialsProviderName
902+
+ " does not have a public no-arg constructor.",
870903
noSuchMethodException);
871904
} catch (InvocationTargetException
872905
| InstantiationException
873906
| IllegalAccessException exception) {
874907
throw SpannerExceptionFactory.newSpannerException(
875908
ErrorCode.INVALID_ARGUMENT,
876-
"Failed to create an instance of " + name + ": " + exception.getMessage(),
909+
"Failed to create an instance of "
910+
+ credentialsProviderName
911+
+ ": "
912+
+ exception.getMessage(),
877913
exception);
878914
}
879915
}
880916
return null;
881917
}
882918

919+
private static void checkGuardedProperty(
920+
String value, String systemPropertyName, String connectionPropertyName) {
921+
if (!Strings.isNullOrEmpty(value)
922+
&& !Boolean.parseBoolean(System.getProperty(systemPropertyName))) {
923+
throw SpannerExceptionFactory.newSpannerException(
924+
ErrorCode.FAILED_PRECONDITION,
925+
generateGuardedConnectionPropertyError(systemPropertyName, connectionPropertyName));
926+
}
927+
}
928+
883929
@VisibleForTesting
884930
static @Nullable String parseOAuthToken(String uri) {
885931
String value = parseUriProperty(uri, OAUTH_TOKEN_PROPERTY_NAME);
@@ -907,6 +953,8 @@ static String parseNumChannels(String uri) {
907953
@VisibleForTesting
908954
static String parseChannelProvider(String uri) {
909955
String value = parseUriProperty(uri, CHANNEL_PROVIDER_PROPERTY_NAME);
956+
checkGuardedProperty(
957+
value, ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY, CHANNEL_PROVIDER_PROPERTY_NAME);
910958
return value != null ? value : DEFAULT_CHANNEL_PROVIDER;
911959
}
912960

0 commit comments

Comments
 (0)