Merge "Allow CORS based on site.allowOriginRegex"

Documentation/config-gerrit.txt

@@ -3452,6 +3452,15 @@ By default, unset, so no Expiry-Date header is generated.
 [[site]]
 === Section site
 
+[[site.allowOriginRegex]]site.allowOriginRegex::
++
+List of regular expressions matching origins that should be permitted
+to use the Gerrit REST API to read content. These should be trusted
+applications as the sites may be able to use the user's credentials.
+Only applies to GET and HEAD requests.
++
+By default, unset, denying all cross-origin requests.
+
 [[site.refreshHeaderFooter]]site.refreshHeaderFooter::
 +
 If true the server checks the site header, footer and CSS files for

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance;
 
 import com.google.common.base.Preconditions;
 
+import org.apache.http.Header;
 import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.RawParseUtils;
 
@@ -52,7 +53,12 @@ public class HttpResponse {
   }
 
   public String getContentType() {
-    return response.getFirstHeader("X-FYI-Content-Type").getValue();
+    return getHeader("X-FYI-Content-Type");
+  }
+
+  public String getHeader(String name) {
+    Header hdr = response.getFirstHeader(name);
+    return hdr != null ? hdr.getValue() : null;
   }
 
   public boolean hasContent() {

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpSession.java

@@ -37,7 +37,11 @@ public class HttpSession {
             account.username, account.httpPassword);
   }
 
-  protected RestResponse execute(Request request) throws IOException {
+  public String url() {
+    return url;
+  }
+
+  public RestResponse execute(Request request) throws IOException {
     return new RestResponse(executor.execute(request).returnResponse());
   }
 }

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/RestSession.java

@@ -45,7 +45,7 @@ public class RestSession extends HttpSession {
         new BasicHeader(HttpHeaders.ACCEPT, "application/json"));
   }
 
-  private RestResponse getWithHeader(String endPoint, Header header)
+  public RestResponse getWithHeader(String endPoint, Header header)
       throws IOException {
     Request get = Request.Get(url + "/a" + endPoint);
     if (header != null) {

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java

@@ -0,0 +1,160 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest.change;
+
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
+import static com.google.common.net.HttpHeaders.ORIGIN;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.testutil.ConfigSuite;
+
+import org.apache.http.Header;
+import org.apache.http.client.fluent.Request;
+import org.apache.http.message.BasicHeader;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class CorsIT extends AbstractDaemonTest {
+  @ConfigSuite.Default
+  public static Config allowExampleDotCom() {
+    Config cfg = new Config();
+    cfg.setStringList(
+        "site", null, "allowOriginRegex",
+        ImmutableList.of(
+            "https?://(.+[.])?example[.]com",
+            "http://friend[.]ly"));
+    return cfg;
+  }
+
+  @Test
+  public void origin() throws Exception {
+    Result change = createChange();
+
+    String url = "/changes/" + change.getChangeId() + "/detail";
+    RestResponse r = adminRestSession.get(url);
+    r.assertOK();
+    assertThat(r.getHeader(ACCESS_CONTROL_ALLOW_ORIGIN)).isNull();
+    assertThat(r.getHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS)).isNull();
+
+    check(url, true, "http://example.com");
+    check(url, true, "https://sub.example.com");
+    check(url, true, "http://friend.ly");
+
+    check(url, false, "http://evil.attacker");
+    check(url, false, "http://friendsly");
+  }
+
+  @Test
+  public void putWithOriginRefused() throws Exception {
+    Result change = createChange();
+    String origin = "http://example.com";
+    RestResponse r = adminRestSession.putWithHeader(
+        "/changes/" + change.getChangeId() + "/topic",
+        new BasicHeader(ORIGIN, origin),
+        "A");
+    r.assertOK();
+    checkCors(r, false, origin);
+  }
+
+  @Test
+  public void preflightOk() throws Exception {
+    Result change = createChange();
+
+    String origin = "http://example.com";
+    Request req = Request.Options(adminRestSession.url()
+        + "/a/changes/" + change.getChangeId() + "/detail");
+    req.addHeader(ORIGIN, origin);
+    req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
+    req.addHeader(ACCESS_CONTROL_REQUEST_HEADERS, "X-Requested-With");
+
+    RestResponse res = adminRestSession.execute(req);
+    res.assertOK();
+    checkCors(res, true, origin);
+  }
+
+  @Test
+  public void preflightBadOrigin() throws Exception {
+    Result change = createChange();
+
+    Request req = Request.Options(adminRestSession.url()
+        + "/a/changes/" + change.getChangeId() + "/detail");
+    req.addHeader(ORIGIN, "http://evil.attacker");
+    req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
+
+    adminRestSession.execute(req).assertBadRequest();
+  }
+
+  @Test
+  public void preflightBadMethod() throws Exception {
+    Result change = createChange();
+
+    for (String method : new String[] {"POST", "PUT", "DELETE", "PATCH"}) {
+      Request req = Request.Options(adminRestSession.url()
+          + "/a/changes/" + change.getChangeId() + "/detail");
+      req.addHeader(ORIGIN, "http://example.com");
+      req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, method);
+      adminRestSession.execute(req).assertBadRequest();
+    }
+  }
+
+  @Test
+  public void preflightBadHeader() throws Exception {
+    Result change = createChange();
+
+    Request req = Request.Options(adminRestSession.url()
+        + "/a/changes/" + change.getChangeId() + "/detail");
+    req.addHeader(ORIGIN, "http://example.com");
+    req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
+    req.addHeader(ACCESS_CONTROL_REQUEST_HEADERS, "X-Gerrit-Auth");
+
+    adminRestSession.execute(req).assertBadRequest();
+  }
+
+  private RestResponse check(String url, boolean accept, String origin)
+      throws Exception {
+    Header hdr = new BasicHeader(ORIGIN, origin);
+    RestResponse r = adminRestSession.getWithHeader(url, hdr);
+    r.assertOK();
+    checkCors(r, accept, origin);
+    return r;
+  }
+
+  private void checkCors(RestResponse r, boolean accept, String origin) {
+    String allowOrigin = r.getHeader(ACCESS_CONTROL_ALLOW_ORIGIN);
+    String allowCred = r.getHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS);
+    String allowMethods = r.getHeader(ACCESS_CONTROL_ALLOW_METHODS);
+    String allowHeaders = r.getHeader(ACCESS_CONTROL_ALLOW_HEADERS);
+    if (accept) {
+      assertThat(allowOrigin).isEqualTo(origin);
+      assertThat(allowCred).isEqualTo("true");
+      assertThat(allowMethods).isEqualTo("GET, OPTIONS");
+      assertThat(allowHeaders).isEqualTo("X-Requested-With");
+    } else {
+      assertThat(allowOrigin).isNull();
+      assertThat(allowCred).isNull();
+      assertThat(allowMethods).isNull();
+      assertThat(allowHeaders).isNull();
+    }
+  }
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -15,6 +15,14 @@
 package com.google.gerrit.httpd.restapi;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
+import static com.google.common.net.HttpHeaders.ORIGIN;
+import static com.google.common.net.HttpHeaders.VARY;
 import static java.math.RoundingMode.CEILING;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -35,9 +43,12 @@ import static javax.servlet.http.HttpServletResponse.SC_PRECONDITION_FAILED;
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
+import com.google.common.base.Predicates;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -85,6 +96,7 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.OptionUtil;
 import com.google.gerrit.server.OutputFormat;
 import com.google.gerrit.server.account.CapabilityUtils;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.util.http.RequestUtil;
 import com.google.gson.ExclusionStrategy;
 import com.google.gson.FieldAttributes;
@@ -103,6 +115,7 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.util.Providers;
 
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.util.TemporaryBuffer;
 import org.eclipse.jgit.util.TemporaryBuffer.Heap;
 import org.slf4j.Logger;
@@ -131,6 +144,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
 import java.util.zip.GZIPOutputStream;
 
 import javax.servlet.ServletException;
@@ -150,6 +164,9 @@ public class RestApiServlet extends HttpServlet {
   // HTTP 422 Unprocessable Entity.
   // TODO: Remove when HttpServletResponse.SC_UNPROCESSABLE_ENTITY is available
   private static final int SC_UNPROCESSABLE_ENTITY = 422;
+  private static final String X_REQUESTED_WITH = "X-Requested-With";
+  private static final ImmutableSet<String> ALLOWED_CORS_REQUEST_HEADERS =
+      ImmutableSet.of(X_REQUESTED_WITH);
 
   private static final int HEAP_EST_SIZE = 10 * 8 * 1024; // Presize 10 blocks.
 
@@ -174,18 +191,29 @@ public class RestApiServlet extends HttpServlet {
     final Provider<ParameterParser> paramParser;
     final AuditService auditService;
     final RestApiMetrics metrics;
+    final Pattern allowOrigin;
 
     @Inject
     Globals(Provider<CurrentUser> currentUser,
         DynamicItem<WebSession> webSession,
         Provider<ParameterParser> paramParser,
         AuditService auditService,
-        RestApiMetrics metrics) {
+        RestApiMetrics metrics,
+        @GerritServerConfig Config cfg) {
       this.currentUser = currentUser;
       this.webSession = webSession;
       this.paramParser = paramParser;
       this.auditService = auditService;
       this.metrics = metrics;
+      allowOrigin = makeAllowOrigin(cfg);
+    }
+
+    private static Pattern makeAllowOrigin(Config cfg) {
+      String[] allow = cfg.getStringList("site", null, "allowOriginRegex");
+      if (allow.length > 0) {
+        return Pattern.compile(Joiner.on('|').join(allow));
+      }
+      return null;
     }
   }
 
@@ -222,6 +250,11 @@ public class RestApiServlet extends HttpServlet {
     ViewData viewData = null;
 
     try {
+      if (isCorsPreflight(req)) {
+        doCorsPreflight(req, res);
+        return;
+      }
+      checkCors(req, res);
       checkUserSession(req);
 
       List<IdString> path = splitPath(req);
@@ -232,7 +265,7 @@ public class RestApiServlet extends HttpServlet {
       viewData = new ViewData(null, null);
 
       if (path.isEmpty()) {
-        if (isGetOrHead(req)) {
+        if (isRead(req)) {
           viewData = new ViewData(null, rc.list());
         } else if (rc instanceof AcceptsPost && "POST".equals(req.getMethod())) {
           @SuppressWarnings("unchecked")
@@ -273,7 +306,7 @@ public class RestApiServlet extends HttpServlet {
             (RestCollection<RestResource, RestResource>) viewData.view;
 
         if (path.isEmpty()) {
-          if (isGetOrHead(req)) {
+          if (isRead(req)) {
             viewData = new ViewData(null, c.list());
           } else if (c instanceof AcceptsPost && "POST".equals(req.getMethod())) {
             @SuppressWarnings("unchecked")
@@ -330,7 +363,7 @@ public class RestApiServlet extends HttpServlet {
         return;
       }
 
-      if (viewData.view instanceof RestReadView<?> && isGetOrHead(req)) {
+      if (viewData.view instanceof RestReadView<?> && isRead(req)) {
         result = ((RestReadView<RestResource>) viewData.view).apply(rsrc);
       } else if (viewData.view instanceof RestModifyView<?, ?>) {
         @SuppressWarnings("unchecked")
@@ -428,6 +461,72 @@ public class RestApiServlet extends HttpServlet {
     }
   }
 
+  private void checkCors(HttpServletRequest req, HttpServletResponse res) {
+    String origin = req.getHeader(ORIGIN);
+    if (isRead(req)
+        && !Strings.isNullOrEmpty(origin)
+        && isOriginAllowed(origin)) {
+      res.addHeader(VARY, ORIGIN);
+      setCorsHeaders(res, origin);
+    }
+  }
+
+  private static boolean isCorsPreflight(HttpServletRequest req) {
+    return "OPTIONS".equals(req.getMethod())
+        && !Strings.isNullOrEmpty(req.getHeader(ORIGIN))
+        && !Strings.isNullOrEmpty(req.getHeader(ACCESS_CONTROL_REQUEST_METHOD));
+  }
+
+  private void doCorsPreflight(HttpServletRequest req,
+      HttpServletResponse res) throws BadRequestException {
+    CacheHeaders.setNotCacheable(res);
+    res.setHeader(VARY, Joiner.on(", ").join(ImmutableList.of(
+        ORIGIN,
+        ACCESS_CONTROL_REQUEST_METHOD)));
+
+    String origin = req.getHeader(ORIGIN);
+    if (Strings.isNullOrEmpty(origin) || !isOriginAllowed(origin)) {
+      throw new BadRequestException("CORS not allowed");
+    }
+
+    String method = req.getHeader(ACCESS_CONTROL_REQUEST_METHOD);
+    if (!"GET".equals(method) && !"HEAD".equals(method)) {
+      throw new BadRequestException(method + " not allowed in CORS");
+    }
+
+    String headers = req.getHeader(ACCESS_CONTROL_REQUEST_HEADERS);
+    if (headers != null) {
+      res.addHeader(VARY, ACCESS_CONTROL_REQUEST_HEADERS);
+      String badHeader = Iterables.getFirst(
+          Iterables.filter(
+              Splitter.on(',').trimResults().split(headers),
+              Predicates.not(Predicates.in(ALLOWED_CORS_REQUEST_HEADERS))),
+          null);
+      if (badHeader != null) {
+        throw new BadRequestException(badHeader + " not allowed in CORS");
+      }
+    }
+
+    res.setStatus(SC_OK);
+    setCorsHeaders(res, origin);
+    res.setContentType("text/plain");
+    res.setContentLength(0);
+  }
+
+  private void setCorsHeaders(HttpServletResponse res, String origin) {
+    res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
+    res.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
+    res.setHeader(ACCESS_CONTROL_ALLOW_METHODS, "GET, OPTIONS");
+    res.setHeader(
+        ACCESS_CONTROL_ALLOW_HEADERS,
+        Joiner.on(", ").join(ALLOWED_CORS_REQUEST_HEADERS));
+  }
+
+  private boolean isOriginAllowed(String origin) {
+    return globals.allowOrigin != null
+        && globals.allowOrigin.matcher(origin).matches();
+  }
+
   private static String messageOr(Throwable t, String defaultMessage) {
     if (!Strings.isNullOrEmpty(t.getMessage())) {
       return t.getMessage();
@@ -438,7 +537,7 @@ public class RestApiServlet extends HttpServlet {
   @SuppressWarnings({"unchecked", "rawtypes"})
   private static boolean notModified(HttpServletRequest req, RestResource rsrc,
       RestView<RestResource> view) {
-    if (!isGetOrHead(req)) {
+    if (!isRead(req)) {
       return false;
     }
 
@@ -469,7 +568,7 @@ public class RestApiServlet extends HttpServlet {
   private static <R extends RestResource> void configureCaching(
       HttpServletRequest req, HttpServletResponse res, R rsrc,
       RestView<R> view, CacheControl c) {
-    if (isGetOrHead(req)) {
+    if (isRead(req)) {
       switch (c.getType()) {
         case NONE:
         default:
@@ -972,25 +1071,20 @@ public class RestApiServlet extends HttpServlet {
   private void checkUserSession(HttpServletRequest req)
       throws AuthException {
     CurrentUser user = globals.currentUser.get();
-    if (isStateChange(req)) {
-      if (user instanceof AnonymousUser) {
-        throw new AuthException("Authentication required");
-      } else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) {
-        throw new AuthException("Invalid authentication method. In order to authenticate, "
-            + "prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).");
-      }
+    if (isRead(req)) {
+      user.setAccessPath(AccessPath.REST_API);
+    } else if (user instanceof AnonymousUser) {
+      throw new AuthException("Authentication required");
+    } else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) {
+      throw new AuthException("Invalid authentication method. In order to authenticate, "
+          + "prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).");
     }
-    user.setAccessPath(AccessPath.REST_API);
   }
 
-  private static boolean isGetOrHead(HttpServletRequest req) {
+  private static boolean isRead(HttpServletRequest req) {
     return "GET".equals(req.getMethod()) || "HEAD".equals(req.getMethod());
   }
 
-  private static boolean isStateChange(HttpServletRequest req) {
-    return !isGetOrHead(req);
-  }
-
   private void checkRequiresCapability(ViewData viewData) throws AuthException {
     CapabilityUtils.checkRequiresCapability(globals.currentUser,
         viewData.pluginName, viewData.view.getClass());
@@ -1029,7 +1123,7 @@ public class RestApiServlet extends HttpServlet {
 
   static long replyText(@Nullable HttpServletRequest req,
       HttpServletResponse res, String text) throws IOException {
-    if ((req == null || isGetOrHead(req)) && isMaybeHTML(text)) {
+    if ((req == null || isRead(req)) && isMaybeHTML(text)) {
       return replyJson(req, res, ImmutableMultimap.of("pp", "0"), new JsonPrimitive(text));
     }
     if (!text.endsWith("\n")) {