GERONIMO-6655 rework security to tolerate by default local calls
diff --git a/README.adoc b/README.adoc
index d3a7030..fc305ce 100644
--- a/README.adoc
+++ b/README.adoc
@@ -30,3 +30,33 @@
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.
diff --git a/geronimo-metrics-common/pom.xml b/geronimo-metrics-common/pom.xml
index 92e243a..1b7d394 100644
--- a/geronimo-metrics-common/pom.xml
+++ b/geronimo-metrics-common/pom.xml
@@ -34,4 +34,13 @@
<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>
diff --git a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
index c2fdec4..4293fd5 100644
--- a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
+++ b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
@@ -32,8 +32,11 @@
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 @@
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 @@
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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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
index 0000000..d7d364f
--- /dev/null
+++ b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidator.java
@@ -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
index 0000000..c2e0aef
--- /dev/null
+++ b/geronimo-metrics-common/src/test/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/SecurityValidatorTest.java
@@ -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;
+ }
+ }
+}
diff --git a/geronimo-metrics/pom.xml b/geronimo-metrics/pom.xml
index a29b50a..258312b 100644
--- a/geronimo-metrics/pom.xml
+++ b/geronimo-metrics/pom.xml
@@ -107,9 +107,6 @@
<environmentVariables>
<MP_METRICS_TAGS>tier=integration</MP_METRICS_TAGS>
</environmentVariables>
- <systemPropertyVariables>
- <geronimo.metrics.jaxrs.activated>true</geronimo.metrics.jaxrs.activated>
- </systemPropertyVariables>
</configuration>
</plugin>
</plugins>
diff --git a/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java b/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
index 0d79b23..74a7f5f 100644
--- a/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
+++ b/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
@@ -86,7 +86,7 @@
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();
}
}
diff --git a/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/jaxrs/CdiMetricsEndpoints.java b/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/jaxrs/CdiMetricsEndpoints.java
index 1e9a0e6..62b9284 100644
--- a/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/jaxrs/CdiMetricsEndpoints.java
+++ b/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/jaxrs/CdiMetricsEndpoints.java
@@ -43,7 +43,7 @@
private MetricRegistry applicationRegistry;
@PostConstruct
- private void init() {
+ protected void init() {
setApplicationRegistry(applicationRegistry);
setBaseRegistry(baseRegistry);
setVendorRegistry(vendorRegistry);
