GERONIMO-6655 rework security to tolerate by default local calls
authorRomain Manni-Bucau <rmannibucau@apache.org>
Sat, 27 Oct 2018 16:28:00 +0000 (18:28 +0200)
committerRomain Manni-Bucau <rmannibucau@apache.org>
Sat, 27 Oct 2018 16:28:00 +0000 (18:28 +0200)
README.adoc
geronimo-metrics-common/pom.xml
geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidator.java [new file with mode: 0644]
geronimo-metrics-common/src/test/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidatorTest.java [new file with mode: 0644]
geronimo-metrics/pom.xml
geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/jaxrs/CdiMetricsEndpoints.java

index d3a7030..fc305ce 100644 (file)
@@ -30,3 +30,33 @@ TIP: since it is a properties format the `:` must be escaped.
 The specific key `geronimo.metrics.filter.prefix` can take
 a list (comma separated values) of metrics prefixes to filter (whitelist)
 exported metrics.
+
+== Controlling the metrics endpoints exposure
+
+To activate the metrics endpoints you have to set the system property `geronimo.metrics.jaxrs.activated` to true
+or configure either a role or host supported for the endpoint.
+
+The role validation will use the JAX-RS `SecurityContext`.
+It relies on the system property `geronimo.metrics.jaxrs.acceptedRoles` and it takes a comma separated list of roles.
+At least one must match to let the request pass, if none is set this validation is ignored.
+Note that a request without a principal will lead to a HTTP 401 whereas a request with a principal but not the right role will issue a HTTP 403.
+
+The host validation will use the JAX-RS `UriInfo#getRequestUri`.
+It relies on the system property `geronimo.metrics.jaxrs.acceptedHosts` and it takes a comma separated list of roles.
+At least one must match to let the request pass, if none is set this validation is ignored.
+The `<local>` value is an alias for `127.x.y.z` or `1::x` IP or `localhost`.
+
+Configuration example:
+
+[source]
+----
+-Dgeronimo.metrics.jaxrs.acceptedRoles=ops \
+-Dgeronimo.metrics.jaxrs.acceptedHosts=my.remote.host
+----
+
+IMPORTANT: the default is `geronimo.metrics.jaxrs.acceptedHosts=<local>` but you can disable the endpoints using `geronimo.metrics.jaxrs.activated=false`.
+
+=== Security
+
+IMPORTANT: default will allow all local calls, this means that if you are behind a proxy which does not propagate the request URI properly
+your `/metrics` endpoints will be public.
index 92e243a..1b7d394 100644 (file)
   <properties>
     <geronimo-metrics.Automatic-Module-Name>org.apache.geronimo.microprofile.metrics.common</geronimo-metrics.Automatic-Module-Name>
   </properties>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.cxf</groupId>
+      <artifactId>cxf-rt-frontend-jaxrs</artifactId>
+      <version>3.2.6</version>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
 </project>
index c2fdec4..4293fd5 100644 (file)
@@ -32,8 +32,11 @@ import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
 import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import javax.ws.rs.core.SecurityContext;
+import javax.ws.rs.core.UriInfo;
 
 import org.apache.geronimo.microprofile.metrics.common.prometheus.PrometheusFormatter;
 import org.eclipse.microprofile.metrics.Counter;
@@ -48,7 +51,13 @@ public class MetricsEndpoints {
     private MetricRegistry vendorRegistry;
     private MetricRegistry applicationRegistry;
 
-    private final PrometheusFormatter prometheus = new PrometheusFormatter().enableOverriding();
+    private SecurityValidator securityValidator = new SecurityValidator() {
+        {
+            init();
+        }
+    };
+
+    private PrometheusFormatter prometheus = new PrometheusFormatter().enableOverriding();
 
     public void setBaseRegistry(final MetricRegistry baseRegistry) {
         this.baseRegistry = baseRegistry;
@@ -62,9 +71,19 @@ public class MetricsEndpoints {
         this.applicationRegistry = applicationRegistry;
     }
 
+    public void setSecurityValidator(final SecurityValidator securityValidator) {
+        this.securityValidator = securityValidator;
+    }
+
+    public void setPrometheus(final PrometheusFormatter prometheus) {
+        this.prometheus = prometheus;
+    }
+
     @GET
     @Produces(MediaType.APPLICATION_JSON)
-    public Object getJson() {
+    public Object getJson(@Context final SecurityContext securityContext,
+                          @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         return Stream.of(MetricRegistry.Type.values())
                 .collect(toMap(MetricRegistry.Type::getName, it -> findRegistry(it.getName()).getMetrics().entrySet().stream()
                         .collect(toMap(Map.Entry::getKey, m -> map(m.getValue())))));
@@ -72,7 +91,9 @@ public class MetricsEndpoints {
 
     @GET
     @Produces(MediaType.TEXT_PLAIN)
-    public String getText() {
+    public String getText(@Context final SecurityContext securityContext,
+                          @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         return Stream.of(MetricRegistry.Type.values())
                 .map(type -> {
                     final MetricRegistry metricRegistry = findRegistry(type.getName());
@@ -85,7 +106,10 @@ public class MetricsEndpoints {
     @GET
     @Path("{registry}")
     @Produces(MediaType.APPLICATION_JSON)
-    public Object getJson(@PathParam("registry") final String registry) {
+    public Object getJson(@PathParam("registry") final String registry,
+                          @Context final SecurityContext securityContext,
+                          @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         return findRegistry(registry).getMetrics().entrySet().stream()
                 .collect(toMap(Map.Entry::getKey, it -> map(it.getValue())));
     }
@@ -93,7 +117,10 @@ public class MetricsEndpoints {
     @GET
     @Path("{registry}")
     @Produces(MediaType.TEXT_PLAIN)
-    public String getText(@PathParam("registry") final String registry) {
+    public String getText(@PathParam("registry") final String registry,
+                          @Context final SecurityContext securityContext,
+                          @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         final MetricRegistry metricRegistry = findRegistry(registry);
         return prometheus.toText(metricRegistry, registry, metricRegistry.getMetrics()).toString();
     }
@@ -102,7 +129,10 @@ public class MetricsEndpoints {
     @Path("{registry}/{metric}")
     @Produces(MediaType.APPLICATION_JSON)
     public Object getJson(@PathParam("registry") final String registry,
-                          @PathParam("metric") final String name) {
+                          @PathParam("metric") final String name,
+                          @Context final SecurityContext securityContext,
+                          @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         return singleEntry(name, findRegistry(registry));
     }
 
@@ -110,7 +140,10 @@ public class MetricsEndpoints {
     @Path("{registry}/{metric}")
     @Produces(MediaType.TEXT_PLAIN)
     public String getText(@PathParam("registry") final String registry,
-                          @PathParam("metric") final String name) {
+                          @PathParam("metric") final String name,
+                          @Context final SecurityContext securityContext,
+                          @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         final MetricRegistry metricRegistry = findRegistry(registry);
         return prometheus.toText(
                 metricRegistry, registry,
@@ -122,7 +155,10 @@ public class MetricsEndpoints {
     @Path("{registry}/{metric}")
     @Produces(MediaType.APPLICATION_JSON)
     public Object getMetadata(@PathParam("registry") final String registry,
-                          @PathParam("metric") final String name) {
+                              @PathParam("metric") final String name,
+                              @Context final SecurityContext securityContext,
+                              @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         return ofNullable(findRegistry(registry).getMetadata().get(name))
                 .map(metric -> singletonMap(name, mapMeta(metric)))
                 .orElse(emptyMap());
@@ -131,7 +167,10 @@ public class MetricsEndpoints {
     @OPTIONS
     @Path("{registry}")
     @Produces(MediaType.APPLICATION_JSON)
-    public Object getMetadata(@PathParam("registry") final String registry) {
+    public Object getMetadata(@PathParam("registry") final String registry,
+                              @Context final SecurityContext securityContext,
+                              @Context final UriInfo uriInfo) {
+        securityValidator.checkSecurity(securityContext, uriInfo);
         return findRegistry(registry).getMetadata().entrySet().stream()
                 .collect(toMap(Map.Entry::getKey, e -> mapMeta(e.getValue())));
     }
diff --git a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidator.java b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidator.java
new file mode 100644 (file)
index 0000000..d7d364f
--- /dev/null
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.geronimo.microprofile.metrics.common.jaxrs;
+
+import static java.util.Collections.singletonList;
+import static java.util.Optional.ofNullable;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toList;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Stream;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.SecurityContext;
+import javax.ws.rs.core.UriInfo;
+
+// default let it pass locally (127.*, localhost or 1::*),
+// this matches prometheus use case in general
+// WARNING: ensure you accept it is public or if you are behind a proxy that you get the right hostname!
+public class SecurityValidator {
+    private static final Predicate<String> LOCAL_MATCHER = it ->
+            it.startsWith("127.") || it.startsWith("1::") || "localhost".equals(it);
+
+    private List<Predicate<String>> acceptedHosts;
+    private List<String> acceptedRoles;
+
+    public void init() {
+        acceptedHosts = config("geronimo.metrics.jaxrs.acceptedHosts", value -> {
+            if ("<local>".equals(value)) {
+                return LOCAL_MATCHER;
+            }
+            return (Predicate<String>) value::equals;
+        }).orElse(singletonList(LOCAL_MATCHER));
+        acceptedRoles = config("geronimo.metrics.jaxrs.acceptedRoles", identity()).orElse(null);
+    }
+
+    public void checkSecurity(final SecurityContext securityContext, final UriInfo uriInfo) {
+        if (acceptedHosts != null && uriInfo != null) {
+            final String host = uriInfo.getRequestUri().getHost();
+            if (host == null || acceptedHosts.stream().noneMatch(it -> it.test(host))) {
+                throw new WebApplicationException(Response.Status.NOT_FOUND);
+            }
+        }
+        if (!hasValidRole(securityContext)) {
+            if (securityContext == null || securityContext.getUserPrincipal() == null) {
+                throw new WebApplicationException(Response.Status.UNAUTHORIZED);
+            }
+            throw new WebApplicationException(Response.Status.FORBIDDEN);
+        }
+    }
+
+    private boolean hasValidRole(final SecurityContext securityContext) {
+        return acceptedRoles == null || (securityContext != null &&
+                securityContext.getUserPrincipal() != null &&
+                acceptedRoles.stream().anyMatch(securityContext::isUserInRole));
+    }
+
+    private <T> Optional<List<T>> config(final String key, final Function<String, T> mapper) {
+        return ofNullable(config(key))
+                .map(value -> Stream.of(value.split(","))
+                        .map(String::trim)
+                        .filter(it -> !it.isEmpty())
+                        .map(mapper)
+                        .collect(toList()));
+    }
+
+    protected String config(final String key) {
+        return System.getProperty(key);
+    }
+}
diff --git a/geronimo-metrics-common/src/test/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidatorTest.java b/geronimo-metrics-common/src/test/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidatorTest.java
new file mode 100644 (file)
index 0000000..c2e0aef
--- /dev/null
@@ -0,0 +1,277 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.geronimo.microprofile.metrics.common.jaxrs;
+
+import java.net.URI;
+import java.security.Principal;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.PathSegment;
+import javax.ws.rs.core.SecurityContext;
+import javax.ws.rs.core.UriBuilder;
+import javax.ws.rs.core.UriInfo;
+
+import com.sun.org.apache.regexp.internal.RE;
+
+import org.junit.Test;
+
+public class SecurityValidatorTest {
+    private static final SecurityContext ANONYMOUS = new SecurityContext() {
+        @Override
+        public Principal getUserPrincipal() {
+            return null;
+        }
+
+        @Override
+        public boolean isUserInRole(final String role) {
+            return false;
+        }
+
+        @Override
+        public boolean isSecure() {
+            return false;
+        }
+
+        @Override
+        public String getAuthenticationScheme() {
+            return null;
+        }
+    };
+    private static final SecurityContext LOGGED_NO_ROLE = new SecurityContext() {
+        @Override
+        public Principal getUserPrincipal() {
+            return () -> "somebody";
+        }
+
+        @Override
+        public boolean isUserInRole(final String role) {
+            return false;
+        }
+
+        @Override
+        public boolean isSecure() {
+            return false;
+        }
+
+        @Override
+        public String getAuthenticationScheme() {
+            return null;
+        }
+    };
+    private static final SecurityContext ADMIN = new SecurityContext() {
+        @Override
+        public Principal getUserPrincipal() {
+            return () -> "somebody";
+        }
+
+        @Override
+        public boolean isUserInRole(final String role) {
+            return "admin".equals(role);
+        }
+
+        @Override
+        public boolean isSecure() {
+            return false;
+        }
+
+        @Override
+        public String getAuthenticationScheme() {
+            return null;
+        }
+    };
+    private static final UriInfo REMOTE = uri("http://geronimo.somewhere");
+    private static final UriInfo LOCALHOST = uri("http://localhost");
+
+    @Test
+    public void localValid() {
+        new SecurityValidator() {{
+            init();
+        }}.checkSecurity(ANONYMOUS, LOCALHOST);
+    }
+
+    @Test(expected = WebApplicationException.class)
+    public void remoteInvalid() {
+        new SecurityValidator() {{
+            init();
+        }}.checkSecurity(ANONYMOUS, REMOTE);
+    }
+
+    @Test
+    public void roleValid() {
+        new SecurityValidator() {
+            {
+                init();
+            }
+
+            @Override
+            protected String config(final String key) {
+                return key.endsWith("acceptedRoles") ? "admin" : null;
+            }
+        }.checkSecurity(ADMIN, LOCALHOST);
+    }
+
+    @Test(expected = WebApplicationException.class)
+    public void roleAnonymousInvalid() {
+        new SecurityValidator() {
+            {
+                init();
+            }
+
+            @Override
+            protected String config(final String key) {
+                return key.endsWith("acceptedRoles") ? "admin" : null;
+            }
+        }.checkSecurity(ANONYMOUS, LOCALHOST);
+    }
+
+    @Test(expected = WebApplicationException.class)
+    public void roleLoggedButInvalid() {
+        new SecurityValidator() {
+            {
+                init();
+            }
+
+            @Override
+            protected String config(final String key) {
+                return key.endsWith("acceptedRoles") ? "admin" : null;
+            }
+        }.checkSecurity(LOGGED_NO_ROLE, LOCALHOST);
+    }
+
+    @Test
+    public void roleAndHostValid() {
+        new SecurityValidator() {
+            {
+                init();
+            }
+
+            @Override
+            protected String config(final String key) {
+                return key.endsWith("acceptedRoles") ? "admin" : "geronimo.somewhere";
+            }
+        }.checkSecurity(ADMIN, REMOTE);
+    }
+
+    private static UriInfo uri(final String request) {
+        return new UriInfoMock(request);
+    }
+
+    private static class UriInfoMock implements UriInfo {
+        private final URI request;
+
+        private UriInfoMock(final String request) {
+            this.request = URI.create(request);
+        }
+
+        @Override
+        public String getPath() {
+            return null;
+        }
+
+        @Override
+        public String getPath(boolean decode) {
+            return null;
+        }
+
+        @Override
+        public List<PathSegment> getPathSegments() {
+            return null;
+        }
+
+        @Override
+        public List<PathSegment> getPathSegments(boolean decode) {
+            return null;
+        }
+
+        @Override
+        public URI getRequestUri() {
+            return request;
+        }
+
+        @Override
+        public UriBuilder getRequestUriBuilder() {
+            return null;
+        }
+
+        @Override
+        public URI getAbsolutePath() {
+            return null;
+        }
+
+        @Override
+        public UriBuilder getAbsolutePathBuilder() {
+            return null;
+        }
+
+        @Override
+        public URI getBaseUri() {
+            return null;
+        }
+
+        @Override
+        public UriBuilder getBaseUriBuilder() {
+            return null;
+        }
+
+        @Override
+        public MultivaluedMap<String, String> getPathParameters() {
+            return null;
+        }
+
+        @Override
+        public MultivaluedMap<String, String> getPathParameters(boolean decode) {
+            return null;
+        }
+
+        @Override
+        public MultivaluedMap<String, String> getQueryParameters() {
+            return null;
+        }
+
+        @Override
+        public MultivaluedMap<String, String> getQueryParameters(boolean decode) {
+            return null;
+        }
+
+        @Override
+        public List<String> getMatchedURIs() {
+            return null;
+        }
+
+        @Override
+        public List<String> getMatchedURIs(boolean decode) {
+            return null;
+        }
+
+        @Override
+        public List<Object> getMatchedResources() {
+            return null;
+        }
+
+        @Override
+        public URI resolve(URI uri) {
+            return null;
+        }
+
+        @Override
+        public URI relativize(URI uri) {
+            return null;
+        }
+    }
+}
index a29b50a..258312b 100644 (file)
           <environmentVariables>
             <MP_METRICS_TAGS>tier=integration</MP_METRICS_TAGS>
           </environmentVariables>
-          <systemPropertyVariables>
-            <geronimo.metrics.jaxrs.activated>true</geronimo.metrics.jaxrs.activated>
-          </systemPropertyVariables>
         </configuration>
       </plugin>
     </plugins>
index 0d79b23..74a7f5f 100644 (file)
@@ -86,7 +86,7 @@ public class MetricsExtension implements Extension {
     private final Collection<CreationalContext<?>> creationalContexts = new ArrayList<>();
 
     void letOtherExtensionsUseRegistries(@Observes final ProcessAnnotatedType<CdiMetricsEndpoints> processAnnotatedType) {
-        if (!Boolean.getBoolean("geronimo.metrics.jaxrs.activated")) {
+        if ("false".equalsIgnoreCase(System.getProperty("geronimo.metrics.jaxrs.activated"))) { // default is secured so deploy
             processAnnotatedType.veto();
         }
     }
index 1e9a0e6..62b9284 100644 (file)
@@ -43,7 +43,7 @@ public class CdiMetricsEndpoints extends MetricsEndpoints {
     private MetricRegistry applicationRegistry;
 
     @PostConstruct
-    private void init() {
+    protected void init() {
         setApplicationRegistry(applicationRegistry);
         setBaseRegistry(baseRegistry);
         setVendorRegistry(vendorRegistry);