Skip to content

Commit

Permalink
fix: only allow user and service account credentials (#2603)
Browse files Browse the repository at this point in the history
When running in authentication mode, PGAdapter accepted a credentials
file as a password. The credentials file was not checked before handed
over to the auth libraries to check that it was one of the supported
types. This check now happens before handing it off for further parsing.
  • Loading branch information
olavloite authored Dec 6, 2024
1 parent 13644f1 commit 632e3c7
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
import static com.google.cloud.spanner.pgadapter.wireprotocol.StartupMessage.DATABASE_KEY;
import static com.google.cloud.spanner.pgadapter.wireprotocol.StartupMessage.createConnectionAndSendStartupMessage;

import com.google.api.client.json.GenericJson;
import com.google.api.client.json.JsonFactory;
import com.google.api.client.json.JsonObjectParser;
import com.google.api.client.json.gson.GsonFactory;
import com.google.api.client.util.PemReader;
import com.google.api.client.util.PemReader.Section;
import com.google.api.client.util.Strings;
Expand All @@ -38,6 +42,7 @@
import java.nio.charset.StandardCharsets;
import java.text.MessageFormat;
import java.util.Map;
import java.util.Objects;

/**
* PGAdapter will convert a password message into gRPC authentication in the following ways:
Expand Down Expand Up @@ -89,7 +94,7 @@ protected void sendPayload() throws Exception {
.setHints(
"PGAdapter expects credentials to be one of the following:\n"
+ "1. Username contains the fixed string 'oauth2' and the password field contains a valid OAuth2 token.\n"
+ "2. Username contains any string and the password field contains the JSON payload of a credentials file (e.g. a service account file).\n"
+ "2. Username contains any string and the password field contains the JSON payload of a service account or user account credentials file. Note: Only user accounts and service accounts are supported.\n"
+ "3. Username contains the email address of a service account and the password contains the corresponding private key for the service account.")
.setSQLState(SQLState.InvalidPassword)
.setSeverity(Severity.ERROR)
Expand Down Expand Up @@ -143,15 +148,32 @@ private Credentials checkCredentials(String username, String password) {

// Try to parse the password field as a JSON string that contains a credentials object.
try {
return GoogleCredentials.fromStream(
new ByteArrayInputStream(password.getBytes(StandardCharsets.UTF_8)));
// Only try to parse credentials that we know and trust.
if (isValidCredentialsType(password)) {
return GoogleCredentials.fromStream(
new ByteArrayInputStream(password.getBytes(StandardCharsets.UTF_8)));
}
} catch (IOException ioException) {
// Ignore and fallthrough..
// Ignore and fallthrough.
}

return null;
}

static boolean isValidCredentialsType(String credentials) throws IOException {
JsonFactory jsonFactory = GsonFactory.getDefaultInstance();
JsonObjectParser parser = new JsonObjectParser(jsonFactory);
GenericJson fileContents =
parser.parseAndClose(new StringReader(credentials), GenericJson.class);

String fileType = (String) fileContents.get("type");
if (fileType == null) {
throw new IOException("Error reading credentials from password, 'type' field not specified.");
}
return Objects.equals("authorized_user", fileType)
|| Objects.equals("service_account", fileType);
}

@Override
protected String getMessageName() {
return "Password Exchange";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class AuthMockServerTest extends AbstractMockServerTest {
"ERROR: Invalid credentials received.\n"
+ " Hint: PGAdapter expects credentials to be one of the following:\n"
+ "1. Username contains the fixed string 'oauth2' and the password field contains a valid OAuth2 token.\n"
+ "2. Username contains any string and the password field contains the JSON payload of a credentials file (e.g. a service account file).\n"
+ "2. Username contains any string and the password field contains the JSON payload of a service account or user account credentials file. Note: Only user accounts and service accounts are supported.\n"
+ "3. Username contains the email address of a service account and the password contains the corresponding private key for the service account.";

@BeforeClass
Expand Down Expand Up @@ -102,13 +102,7 @@ public void testConnectFailsWithEmptyPassword() {
PSQLException exception =
assertThrows(
PSQLException.class, () -> DriverManager.getConnection(createUrl(), "oauth2", ""));
assertEquals(
"ERROR: Invalid credentials received.\n"
+ " Hint: PGAdapter expects credentials to be one of the following:\n"
+ "1. Username contains the fixed string 'oauth2' and the password field contains a valid OAuth2 token.\n"
+ "2. Username contains any string and the password field contains the JSON payload of a credentials file (e.g. a service account file).\n"
+ "3. Username contains the email address of a service account and the password contains the corresponding private key for the service account.",
exception.getMessage());
assertEquals(CREDENTIALS_ERROR, exception.getMessage());
}

@Test
Expand Down Expand Up @@ -181,6 +175,20 @@ public void testConnectWithPrivateKey() throws Exception {
+ " \"client_x509_cert_url\": \"https://www.googleapis.com/robot/v1/metadata/x509/invalid.com\"\n"
+ "}\n";

private static final String INVALID_CREDENTIALS_FILE =
"{\n"
+ " \"type\": \"external_account\",\n"
+ " \"project_id\": \"pgadapter-mock-server-test-project\",\n"
+ " \"private_key_id\": \"random-key-id\",\n"
+ " \"private_key\": \"%s\",\n"
+ " \"client_email\": \"pgadapter-test-account@pgadapter-mock-server-test-project.iam.invalid.com\",\n"
+ " \"client_id\": \"123456789\",\n"
+ " \"auth_uri\": \"https://accounts.google.com/o/oauth2/auth\",\n"
+ " \"token_uri\": \"https://oauth2.googleapis.com/token\",\n"
+ " \"auth_provider_x509_cert_url\": \"https://www.googleapis.com/oauth2/v1/certs\",\n"
+ " \"client_x509_cert_url\": \"https://www.googleapis.com/robot/v1/metadata/x509/invalid.com\"\n"
+ "}\n";

@Test
public void testConnectWithCredentialsFile() throws Exception {
// The username is ignored by PGAdapter when a credentials file is used.
Expand Down Expand Up @@ -208,6 +216,18 @@ public void testConnectWithCredentialsFile() throws Exception {
assertEquals(password, passwordMessage.getPassword());
}

@Test
public void testConnectWithInvalidCredentialsFile() throws Exception {
// The username is ignored by PGAdapter when a credentials file is used.
String username = "whatever";
String password = String.format(INVALID_CREDENTIALS_FILE, generateRandomPrivateKey());
PSQLException exception =
assertThrows(
PSQLException.class,
() -> DriverManager.getConnection(createUrl(), username, password));
assertEquals(CREDENTIALS_ERROR, exception.getMessage());
}

private String generateRandomPrivateKey() throws NoSuchAlgorithmException, IOException {
final KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");
generator.initialize(2048, null);
Expand Down

0 comments on commit 632e3c7

Please sign in to comment.