summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarcin Czech <maczech@gmail.com>2020-06-15 17:36:18 +0200
committerMarcin Czech <maczech@gmail.com>2020-06-17 11:18:52 +0200
commit559ea2b49f348d009287bef8f26f6f5e29971c4a (patch)
tree7b3db796db160c541552ea37108298ad08ea907c
parent56918394544b6207a626cb0938d0bba4927a9fa4 (diff)
Set X-Frame-Options header to avoid clickjacking
Add HTTP filter which is applied to all HTTP responses. Based on gerrit.canLoadInIFrame and gerrit.xframeOption properties filter adds the X-Frame-Options HTTP response header. The X-Frame-Options HTTP response header can be used to indicate whether or not a browser should be allowed to render a page in a <frame>, <iframe>, <embed> or <object>. Gerrit can use this to avoid click-jacking attacks, by ensuring that the content is not embedded into other sites. Bug: Issue 12926 Change-Id: If3f6a770332ade9924b3d1a20c092637c9380e0c
-rw-r--r--Documentation/config-gerrit.txt19
-rw-r--r--java/com/google/gerrit/httpd/AllRequestFilter.java8
-rw-r--r--java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java59
-rw-r--r--javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java169
4 files changed, 254 insertions, 1 deletions
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 531bb94f7c..006af8607c 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2086,6 +2086,25 @@ Setting this option to true will prevent this behavior.
+
By default false.
+[[gerrit.xframeOption]]gerrit.xframeOption::
++
+Add link:https://tools.ietf.org/html/rfc7034[`X-Frame-Options`] header to all HTTP
+responses. The `X-Frame-Options` HTTP response header can be used to indicate
+whether or not a browser should be allowed to render a page in a
+`<frame>`, `<iframe>`, `<embed>` or `<object>`.
++
+Available values:
++
+1. ALLOW - The page can be displayed in a frame.
+2. SAMEORIGIN - The page can only be displayed in a frame on the same origin as the page itself.
++
+If link:#gerrit.canLoadInIFrame is set to false this option is ignored and the
+`X-Frame-Options` header is always set to `DENY`.
+Setting this option to `ALLOW` will cause the `X-Frame-Options` header to be omitted
+the the page can be displayed in a frame.
++
+By default SAMEORIGIN.
+
[[gerrit.cdnPath]]gerrit.cdnPath::
+
Path prefix for PolyGerrit's static resources if using a CDN.
diff --git a/java/com/google/gerrit/httpd/AllRequestFilter.java b/java/com/google/gerrit/httpd/AllRequestFilter.java
index 9d171d5a54..1c3cafea04 100644
--- a/java/com/google/gerrit/httpd/AllRequestFilter.java
+++ b/java/com/google/gerrit/httpd/AllRequestFilter.java
@@ -18,6 +18,8 @@ import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.plugins.Plugin;
import com.google.gerrit.server.plugins.StopPluginListener;
import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Scopes;
import com.google.inject.Singleton;
import com.google.inject.internal.UniqueAnnotations;
import com.google.inject.servlet.ServletModule;
@@ -32,11 +34,15 @@ import javax.servlet.ServletResponse;
/** Filters all HTTP requests passing through the server. */
public abstract class AllRequestFilter implements Filter {
- public static ServletModule module() {
+ public static Module module() {
return new ServletModule() {
@Override
protected void configureServlets() {
DynamicSet.setOf(binder(), AllRequestFilter.class);
+ DynamicSet.bind(binder(), AllRequestFilter.class)
+ .to(AllowRenderInFrameFilter.class)
+ .in(Scopes.SINGLETON);
+
filter("/*").through(FilterProxy.class);
bind(StopPluginListener.class)
diff --git a/java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java b/java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java
new file mode 100644
index 0000000000..b414aad7c1
--- /dev/null
+++ b/java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java
@@ -0,0 +1,59 @@
+// Copyright (C) 2020 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.httpd;
+
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.lib.Config;
+
+public class AllowRenderInFrameFilter extends AllRequestFilter {
+ static final String X_FRAME_OPTIONS_HEADER_NAME = "X-Frame-Options";
+
+ public static enum XFrameOption {
+ ALLOW,
+ SAMEORIGIN;
+ }
+
+ private final String xframeOptionString;
+ private final boolean skipXFrameOption;
+
+ @Inject
+ public AllowRenderInFrameFilter(@GerritServerConfig Config cfg) {
+ XFrameOption xframeOption =
+ cfg.getEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
+ boolean canLoadInIFrame = cfg.getBoolean("gerrit", "canLoadInIFrame", false);
+ xframeOptionString = canLoadInIFrame ? xframeOption.name() : "DENY";
+
+ skipXFrameOption = xframeOption.equals(XFrameOption.ALLOW) && canLoadInIFrame;
+ }
+
+ @Override
+ public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+ if (skipXFrameOption) {
+ chain.doFilter(request, response);
+ } else {
+ HttpServletResponse httpResponse = (HttpServletResponse) response;
+ httpResponse.addHeader(X_FRAME_OPTIONS_HEADER_NAME, xframeOptionString);
+ chain.doFilter(request, httpResponse);
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java b/javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java
new file mode 100644
index 0000000000..0d6d482032
--- /dev/null
+++ b/javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java
@@ -0,0 +1,169 @@
+// Copyright (C) 2020 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.httpd;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.httpd.AllowRenderInFrameFilter.X_FRAME_OPTIONS_HEADER_NAME;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.easymock.EasyMock.expectLastCall;
+
+import com.google.gerrit.httpd.AllowRenderInFrameFilter.XFrameOption;
+import com.google.gerrit.testing.GerritBaseTests;
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.easymock.EasyMockSupport;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Test;
+
+public class AllowRenderInFrameFilterTest extends GerritBaseTests {
+
+ Config cfg;
+ ServletRequest request;
+ HttpServletResponse response;
+ FilterChain filterChain;
+
+ EasyMockSupport ems = new EasyMockSupport();
+
+ @Before
+ public void setup() throws IOException, ServletException {
+ cfg = new Config();
+ request = ems.createMock(ServletRequest.class);
+ response = ems.createMock(HttpServletResponse.class);
+ filterChain = ems.createMock(FilterChain.class);
+ ems.resetAll();
+ // we want to make sure that doFilter is always called
+ filterChain.doFilter(request, response);
+ }
+
+ @Test
+ public void shouldDenyInFrameRenderingWhenCanRenderInFrameIsFalse()
+ throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", false);
+
+ response.addHeader(X_FRAME_OPTIONS_HEADER_NAME, "DENY");
+ expectLastCall().times(1);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldDenyInFrameRenderingWhenCanRenderInFrameIsFalseAndXFormOptionIsSAMEORIGIN()
+ throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", false);
+ cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
+
+ response.addHeader(X_FRAME_OPTIONS_HEADER_NAME, "DENY");
+ expectLastCall().times(1);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldDenyInFrameRenderingWhenCanRenderInFrameIsFalseAndXFormOptionIsALLOW()
+ throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", false);
+ cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.ALLOW);
+
+ response.addHeader(X_FRAME_OPTIONS_HEADER_NAME, "DENY");
+ expectLastCall().times(1);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldRestrictAccessToSAMEORIGINWhenCanRenderInFrameIsTrue()
+ throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+
+ response.addHeader(X_FRAME_OPTIONS_HEADER_NAME, "SAMEORIGIN");
+ expectLastCall().times(1);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldSkipHeaderWhenCanRenderInFrameIsTrueAndXFormOptionIsALLOW()
+ throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+ cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.ALLOW);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldRestrictAccessToSAMEORIGINWhenCanRenderInFrameIsTrueAndXFormOptionIsSAMEORIGIN()
+ throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+ cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
+
+ response.addHeader(X_FRAME_OPTIONS_HEADER_NAME, "SAMEORIGIN");
+ expectLastCall().times(1);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldIgnoreXFrameOriginCaseSensitivity() throws IOException, ServletException {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+ cfg.setString("gerrit", null, "xframeOption", "sameOrigin");
+
+ response.addHeader(X_FRAME_OPTIONS_HEADER_NAME, "SAMEORIGIN");
+ expectLastCall().times(1);
+ ems.replayAll();
+
+ AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+ objectUnderTest.doFilter(request, response, filterChain);
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void shouldThrowExceptionWhenUnknownXFormOptionValue() {
+ cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+ cfg.setString("gerrit", null, "xframeOption", "unsupported value");
+
+ IllegalArgumentException e =
+ assertThrows(IllegalArgumentException.class, () -> new AllowRenderInFrameFilter(cfg));
+ assertThat(e).hasMessageThat().contains("gerrit.xframeOption=unsupported value");
+ }
+}