Validate project-id returned from metadata server (#2822)

yihanzhen · web-flow · commit 536af840f936 · 2018-02-01T09:17:58.000-08:00
* Validate project-id returned from metadata server to handle cases in
which a descriptive failure html page is returned instead of the
projectid itself from server when projects are not running in google cloud machines.

* update comments

* Validate project-id returned from metadata server to handle cases in
which a descriptive failure html page is returned instead of the
projectid itself from server when projects are not running in google cloud machines.

* update comments

* fix code review problems

* Fix code review problems

* fix code review problems

* fix useless parentheses
diff --git a/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java
@@ -452,7 +452,8 @@ private static String getAppEngineProjectIdFromMetadataServer() throws IOExcepti
             .setReadTimeout(500)
             .setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google"));
     HttpResponse response = request.execute();
-    return response.parseAsString();
+    String projectId = response.parseAsString();
+    return projectId != null && isValidProjectId(projectId)? projectId : null;
   }
 
   protected static String getServiceAccountProjectId() {
@@ -476,6 +477,29 @@ static String getServiceAccountProjectId(String credentialsPath) {
     return project;
   }
 
+  /* 
+   * Returns true if the projectId is valid. This method checks whether the projectId
+   * contains only lowercase letters, digits and hyphens, starts with a lowercase letter
+   * and does not end with a hyphen, but does not check the length of projectId. This
+   * method is primarily used to protect against DNS hijacking.
+   */
+  static boolean isValidProjectId(String projectId) {   
+    for (char c : projectId.toCharArray()) {
+      if (!isLowerCase(c) && !isDigit(c) && c != '-') {
+        return false;
+      }
+    }
+    return projectId.length() > 0 && isLowerCase(projectId.charAt(0))
+      && !projectId.endsWith("-");
+  }
+
+  private static boolean isLowerCase(char c) {
+    return c >= 'a' && c <= 'z';
+  }
+
+  private static boolean isDigit(char c) {
+    return c >= '0' && c <= '9';
+  }
 
   /**
    * Returns a Service object for the current service. For instance, when using Google Cloud
diff --git a/google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java b/google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
@@ -23,6 +23,7 @@
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static com.google.common.truth.Truth.assertThat;
 
 import com.google.api.core.ApiClock;
 import com.google.api.core.CurrentMillisClock;
@@ -313,4 +314,13 @@ public void testGetServiceAccountProjectId_nonExistentFile() throws Exception {
 
     assertNull(ServiceOptions.getServiceAccountProjectId(credentialsFile.getPath()));
   }
+
+  @Test
+  public void testValidateProjectId() throws Exception {
+    assertThat(ServiceOptions.isValidProjectId("abc-123")).isTrue();
+    assertThat(ServiceOptions.isValidProjectId("abc-123-ab")).isTrue();
+    assertThat(ServiceOptions.isValidProjectId("abc=123")).isFalse();
+    assertThat(ServiceOptions.isValidProjectId("abc123-")).isFalse();
+    assertThat(ServiceOptions.isValidProjectId("1abc-23")).isFalse();
+  }
 }
