From 9cd618f276b85fd5d24eeda11aaac3b1608c49d7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:54:55 -0500 Subject: [PATCH] feat(baseuri): add baseuri-range config property to control servers the Agent will register with (#243) (#244) (cherry picked from commit e02ba921e0c1a539156d7c19bcbcc3d88e03f8a4) Co-authored-by: Andrew Azores --- README.md | 1 + src/main/java/io/cryostat/agent/Agent.java | 19 ++ .../java/io/cryostat/agent/ConfigModule.java | 74 ++++++++ .../io/cryostat/agent/util/StringUtils.java | 4 + .../META-INF/microprofile-config.properties | 1 + .../java/io/cryostat/agent/URIRangeTest.java | 172 ++++++++++++++++++ 6 files changed, 271 insertions(+) create mode 100644 src/test/java/io/cryostat/agent/URIRangeTest.java diff --git a/README.md b/README.md index cc134611..a12c0d8d 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,7 @@ Below is a list of configuration properties that can be used to influence how `c and how it advertises itself to a Cryostat server instance. Required properties are indicated with a checked box. - [x] `cryostat.agent.baseuri` [`java.net.URI`]: the URL location of the Cryostat server backend that this agent advertises itself to. +- [x] `cryostat.agent.baseuri-range` [`String`]: a `String` representing the `io.cryostat.agent.ConfigModule.UriRange` enum level that restricts the acceptable hosts specified in the `cryostat.agent.baseuri` property. This is used to control the server locations that this Cryostat Agent instance is willing to register itself with. Default `dns_local`, which means any IP or hostname that is or resolves to `localhost`, a link-local IP address, an IP address from a private range block, or a hostname ending in `.local` will be accepted. If a `cryostat.agent.baseuri` is specified with a host outside of this range then the Agent will refuse to start. Acceptable values are: `loopback`, `link_local`, `site_local`, `dns_local`, and `public`. Each higher/more relaxed level implies that each lower level is also acceptable. - [x] `cryostat.agent.callback` [`java.net.URI`]: a URL pointing back to this agent, ex. `"https://12.34.56.78:1234/"`. Cryostat will use this URL to perform health checks and request updates from the agent. This reflects the externally-visible IP address/hostname and port where this application and agent can be found. - [ ] `cryostat.agent.api.writes-enabled` [`boolean`]: Control whether the agent accepts "write" or mutating operations on its HTTP API. Requests for remote operations such as dynamically starting Flight Recordings will be rejected unless this is set. Default `false`. - [ ] `cryostat.agent.instance-id` [`String`]: a unique ID for this agent instance. This will be used to uniquely identify the agent in the Cryostat discovery database, as well as to unambiguously match its encrypted stored credentials. The default is a random UUID string. It is not recommended to override this value. diff --git a/src/main/java/io/cryostat/agent/Agent.java b/src/main/java/io/cryostat/agent/Agent.java index d8714b74..df4aa396 100644 --- a/src/main/java/io/cryostat/agent/Agent.java +++ b/src/main/java/io/cryostat/agent/Agent.java @@ -15,6 +15,7 @@ */ package io.cryostat.agent; +import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -28,6 +29,7 @@ import javax.inject.Named; import javax.inject.Singleton; +import io.cryostat.agent.ConfigModule.URIRange; import io.cryostat.agent.harvest.Harvester; import io.cryostat.agent.triggers.TriggerEvaluator; @@ -46,6 +48,17 @@ public static void main(String[] args) { AgentExitHandler agentExitHandler = null; try { final Client client = DaggerAgent_Client.builder().build(); + + URI baseUri = client.baseUri(); + URIRange uriRange = client.uriRange(); + if (!uriRange.validate(baseUri)) { + throw new IllegalArgumentException( + String.format( + "cryostat.agent.baseuri of \"%s\" is unacceptable with URI range" + + " \"%s\"", + baseUri, uriRange)); + } + Registration registration = client.registration(); Harvester harvester = client.harvester(); WebServer webServer = client.webServer(); @@ -141,6 +154,12 @@ public static void premain(String args) { @Singleton @Component(modules = {MainModule.class}) interface Client { + @Named(ConfigModule.CRYOSTAT_AGENT_BASEURI) + URI baseUri(); + + @Named(ConfigModule.CRYOSTAT_AGENT_BASEURI_RANGE) + URIRange uriRange(); + WebServer webServer(); Registration registration(); diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java index 570475d8..bce67b8f 100644 --- a/src/main/java/io/cryostat/agent/ConfigModule.java +++ b/src/main/java/io/cryostat/agent/ConfigModule.java @@ -21,10 +21,14 @@ import java.util.Arrays; import java.util.List; import java.util.UUID; +import java.util.function.Predicate; +import java.util.stream.Collectors; import javax.inject.Named; import javax.inject.Singleton; +import io.cryostat.agent.util.StringUtils; + import dagger.Module; import dagger.Provides; import org.eclipse.microprofile.config.Config; @@ -38,6 +42,7 @@ public abstract class ConfigModule { private static final Logger log = LoggerFactory.getLogger(ConfigModule.class); public static final String CRYOSTAT_AGENT_INSTANCE_ID = "cryostat.agent.instance-id"; + public static final String CRYOSTAT_AGENT_BASEURI_RANGE = "cryostat.agent.baseuri-range"; public static final String CRYOSTAT_AGENT_BASEURI = "cryostat.agent.baseuri"; public static final String CRYOSTAT_AGENT_CALLBACK = "cryostat.agent.callback"; public static final String CRYOSTAT_AGENT_REALM = "cryostat.agent.realm"; @@ -97,6 +102,12 @@ public static Config provideConfig() { return ConfigProvider.getConfig(); } + @Provides + @Named(CRYOSTAT_AGENT_BASEURI_RANGE) + public static URIRange provideUriRange(Config config) { + return URIRange.fromString(config.getValue(CRYOSTAT_AGENT_BASEURI_RANGE, String.class)); + } + @Provides @Singleton @Named(CRYOSTAT_AGENT_BASEURI) @@ -307,4 +318,67 @@ public static List provideCryostatSmartTriggerDefinitions(Config config) public static long provideCryostatSmartTriggerEvaluationPeriodMs(Config config) { return config.getValue(CRYOSTAT_AGENT_SMART_TRIGGER_EVALUATION_PERIOD_MS, long.class); } + + public enum URIRange { + LOOPBACK(u -> check(u, u2 -> true, InetAddress::isLoopbackAddress)), + LINK_LOCAL( + u -> + check( + u, + u2 -> StringUtils.isNotBlank(u2.getHost()), + InetAddress::isLinkLocalAddress)), + SITE_LOCAL( + u -> + check( + u, + u2 -> StringUtils.isNotBlank(u2.getHost()), + InetAddress::isSiteLocalAddress)), + DNS_LOCAL( + u -> + StringUtils.isNotBlank(u.getHost()) + && (u.getHost().endsWith(".local") + || u.getHost().endsWith(".localhost"))), + PUBLIC(u -> true), + ; + + private URIRange(Predicate fn) { + this.fn = fn; + } + + private final Predicate fn; + + private static boolean check(URI uri, Predicate f1, Predicate f2) { + try { + return f1.test(uri) && f2.test(InetAddress.getByName(uri.getHost())); + } catch (UnknownHostException uhe) { + log.error("Failed to resolve host", uhe); + return false; + } + } + + private boolean test(URI uri) { + return fn.test(uri); + } + + public boolean validate(URI uri) { + List ranges = + List.of(URIRange.values()).stream() + .filter(r -> r.ordinal() <= this.ordinal()) + .collect(Collectors.toList()); + boolean match = false; + for (URIRange range : ranges) { + match |= range.test(uri); + } + return match; + } + + public static URIRange fromString(String s) { + for (URIRange r : URIRange.values()) { + if (r.name().equalsIgnoreCase(s)) { + return r; + } + } + return SITE_LOCAL; + } + } } diff --git a/src/main/java/io/cryostat/agent/util/StringUtils.java b/src/main/java/io/cryostat/agent/util/StringUtils.java index a8f76f8f..aae1778e 100644 --- a/src/main/java/io/cryostat/agent/util/StringUtils.java +++ b/src/main/java/io/cryostat/agent/util/StringUtils.java @@ -21,4 +21,8 @@ private StringUtils() {} public static boolean isBlank(String s) { return s == null || s.isBlank(); } + + public static boolean isNotBlank(String s) { + return !isBlank(s); + } } diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index e09dbb68..dc89508b 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -1,4 +1,5 @@ cryostat.agent.app.name=cryostat-agent +cryostat.agent.baseuri-range=dns_local cryostat.agent.baseuri= cryostat.agent.api.writes-enabled=false diff --git a/src/test/java/io/cryostat/agent/URIRangeTest.java b/src/test/java/io/cryostat/agent/URIRangeTest.java new file mode 100644 index 00000000..b74baf48 --- /dev/null +++ b/src/test/java/io/cryostat/agent/URIRangeTest.java @@ -0,0 +1,172 @@ +/* + * Copyright The Cryostat Authors. + * + * 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 io.cryostat.agent; + +import java.net.URI; +import java.net.URISyntaxException; + +import io.cryostat.agent.ConfigModule.URIRange; + +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +// FIXME we actually perform a DNS lookup to validate the URI. Unit tests should not do actual DNS +// resolution but instead be able to validate what the host symbolically represents. +public class URIRangeTest { + + @ParameterizedTest + @CsvSource({ + "127.0.0.1, true", + "localhost, true", + "svc.localhost, true", + "169.254.0.0, false", + "169.254.10.5, false", + "168.254.1.1, false", + "10.0.0.10, false", + "10.0.10.10, false", + "10.10.10.10, false", + "11.10.10.10, false", + "172.16.0.0, false", + "172.16.10.5, false", + "172.17.10.5, false", + "172.100.10.5, false", + "192.168.1.1, false", + "192.168.2.1, false", + "192.169.2.1, false", + "svc.local, false", + "example.svc.cluster.local, false", + "example.com, false" + }) + public void testLoopbackRange(String s, String v) throws URISyntaxException { + test(URIRange.LOOPBACK, s, v); + } + + @ParameterizedTest + @CsvSource({ + "127.0.0.1, true", + "localhost, true", + "svc.localhost, true", + "169.254.0.0, true", + "169.254.10.5, true", + "168.254.1.1, false", + "10.0.0.10, false", + "10.0.10.10, false", + "10.10.10.10, false", + "11.10.10.10, false", + "172.16.0.0, false", + "172.16.10.5, false", + "172.17.10.5, false", + "172.100.10.5, false", + "192.168.1.1, false", + "192.168.2.1, false", + "192.169.2.1, false", + "svc.local, false", + "example.svc.cluster.local, false", + "example.com, false" + }) + public void testLinkLocalRange(String s, String v) throws URISyntaxException { + test(URIRange.LINK_LOCAL, s, v); + } + + @ParameterizedTest + @CsvSource({ + "127.0.0.1, true", + "localhost, true", + "svc.localhost, true", + "169.254.0.0, true", + "169.254.10.5, true", + "168.254.1.1, false", + "10.0.0.10, true", + "10.0.10.10, true", + "10.10.10.10, true", + "11.10.10.10, false", + "172.16.0.0, true", + "172.16.10.5, true", + "172.17.10.5, true", + "172.100.10.5, false", + "192.168.1.1, true", + "192.168.2.1, true", + "192.170.2.1, false", + "svc.local, false", + "example.svc.cluster.local, false", + "example.com, false" + }) + public void testSiteLocalRange(String s, String v) throws URISyntaxException { + test(URIRange.SITE_LOCAL, s, v); + } + + @ParameterizedTest + @CsvSource({ + "127.0.0.1, true", + "localhost, true", + "svc.localhost, true", + "169.254.0.0, true", + "169.254.10.5, true", + "168.254.1.1, false", + "10.0.0.10, true", + "10.0.10.10, true", + "10.10.10.10, true", + "11.10.10.10, false", + "172.16.0.0, true", + "172.16.10.5, true", + "172.17.10.5, true", + "172.100.10.5, false", + "192.168.1.1, true", + "192.168.2.1, true", + "192.169.2.1, false", + "svc.local, true", + "example.svc.cluster.local, true", + "example.com, false" + }) + public void testDnsLocalRange(String s, String v) throws URISyntaxException { + test(URIRange.DNS_LOCAL, s, v); + } + + @ParameterizedTest + @CsvSource({ + "127.0.0.1, true", + "localhost, true", + "svc.localhost, true", + "169.254.0.0, true", + "169.254.10.5, true", + "168.254.1.1, true", + "10.0.0.10, true", + "10.0.10.10, true", + "10.10.10.10, true", + "11.10.10.10, true", + "172.16.0.0, true", + "172.16.10.5, true", + "172.17.10.5, true", + "172.100.10.5, true", + "192.168.1.1, true", + "192.168.2.1, true", + "192.169.2.1, true", + "svc.local, true", + "example.svc.cluster.local, true", + "example.com, true" + }) + public void testPublicRange(String s, String v) throws URISyntaxException { + test(URIRange.PUBLIC, s, v); + } + + private void test(URIRange range, String s, String v) throws URISyntaxException { + boolean expected = Boolean.parseBoolean(v); + URI u = new URI(String.format("http://%s:1234", s)); + MatcherAssert.assertThat(s, range.validate(u), Matchers.is(expected)); + } +}