From b8a5d0a13900c255cbd35adace79ac6dcbed6587 Mon Sep 17 00:00:00 2001 From: Paul King Date: Tue, 18 Aug 2026 12:26:12 +1000 Subject: [PATCH] GROOVY-12266: Add a policy for Grape resolver roots using a plaintext protocol Neither engine checked the scheme of a resolver root, so a @GrabResolver naming an http:// repository silently fetched artifacts that anyone on the path could read or replace, and placed that repository first in lookup order. Both engines default to an https central repository, so this only arose for explicitly configured resolvers. Add -Dgroovy.grape.insecureProtocolPolicy, taking fail, warn or ignore and defaulting to warn, so that the resolver is reported but still added. The values mirror Maven's checksum-policy vocabulary. An unrecognised value falls back to warn rather than ignore, reported once, so that a typo cannot silently disable the check. The check lives in the Grape facade rather than in either engine because every documented route to adding a resolver -- @GrabResolver, the grape command line tool, and Grape.addResolver -- passes through it, so the Ivy and Maven engines are covered by one implementation and behave alike. Roots naming a loopback host are exempt under every policy since they do not cross a network, and each distinct root is reported at most once. Classification is an allow-list of known-plaintext schemes, currently http and ftp, so that encrypted transports such as s3 and gs are not reported falsely; file: roots are never insecure, as they cross no network and a network-mounted one cannot be told apart from a local one by inspecting the URI. Integrity for those repositories is checksum verification's job, which applies to every transport rather than only to remote ones. --- src/main/java/groovy/grape/Grape.java | 157 ++++++++++++++ .../GrapeInsecureResolverRootTest.groovy | 193 ++++++++++++++++++ 2 files changed, 350 insertions(+) create mode 100644 src/test/groovy/groovy/grape/GrapeInsecureResolverRootTest.groovy diff --git a/src/main/java/groovy/grape/Grape.java b/src/main/java/groovy/grape/Grape.java index 654696c241c..a4c41f4f676 100644 --- a/src/main/java/groovy/grape/Grape.java +++ b/src/main/java/groovy/grape/Grape.java @@ -21,12 +21,18 @@ import org.codehaus.groovy.tools.GrapeUtil; import java.net.URI; +import java.net.URISyntaxException; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static java.lang.System.Logger.Level.DEBUG; import static java.lang.System.Logger.Level.ERROR; @@ -51,6 +57,16 @@ public class Grape { * Argument key for additional system properties. */ public static final String SYSTEM_PROPERTIES_SETTING = "systemProperties"; + /** + * System property selecting what happens when a resolver root uses a plaintext protocol: + * {@code fail} to reject the resolver, {@code warn} (the default) to log a warning and add + * it anyway, or {@code ignore} to skip the check. An unrecognised value is treated as + * {@code warn}. Values mirror Maven's checksum-policy vocabulary. + */ + public static final String INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY = "groovy.grape.insecureProtocolPolicy"; + private static final String INSECURE_PROTOCOL_POLICY_FAIL = "fail"; + private static final String INSECURE_PROTOCOL_POLICY_WARN = "warn"; + private static final String INSECURE_PROTOCOL_POLICY_IGNORE = "ignore"; private static final String GRAPE_IMPL_SYSTEM_PROPERTY = "groovy.grape.impl"; private static final String DEFAULT_GRAPE_ENGINE = "groovy.grape.ivy.GrapeIvy"; private static final URI[] EMPTY_URI_ARRAY = new URI[0]; @@ -59,6 +75,12 @@ public class Grape { private static boolean enableGrapes = Boolean.parseBoolean(System.getProperty("groovy.grape.enable", "true")); private static boolean enableAutoDownload = Boolean.parseBoolean(System.getProperty("groovy.grape.autoDownload", "true")); private static boolean disableChecksums = Boolean.parseBoolean(System.getProperty("groovy.grape.disableChecksums", "false")); + /** Resolver roots already reported as plaintext, so each is warned about only once. */ + private static final Set WARNED_INSECURE_ROOTS = ConcurrentHashMap.newKeySet(); + /** Unrecognised insecure-protocol policy values already reported. */ + private static final Set WARNED_POLICY_VALUES = ConcurrentHashMap.newKeySet(); + /** Dotted-quad IPv4 literal, capturing the first octet. */ + private static final Pattern IPV4_LITERAL = Pattern.compile("(\\d{1,3})(?:\\.\\d{1,3}){3}"); /** * Lazily created grape engine instance. */ @@ -380,11 +402,18 @@ public static Map[] listDependencies(ClassLoader cl) { /** * Adds a resolver to the shared grape engine. + *

+ * A resolver root using a plaintext protocol is subject to + * {@value #INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY}: {@code warn} (the default) logs a + * warning and adds the resolver, {@code fail} rejects it, and {@code ignore} skips the + * check. Roots naming a loopback host are exempt under every policy. * * @param args the resolver descriptor + * @throws RuntimeException under the {@code fail} policy, if the root is a plaintext remote root */ public static void addResolver(Map args) { if (enableGrapes) { + checkResolverRootProtocol(args); GrapeEngine instance = getInstance(); if (instance != null) { instance.addResolver(args); @@ -392,4 +421,132 @@ public static void addResolver(Map args) { } } + /** + * Applies {@value #INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY} to a resolver descriptor. + * + * @param args the resolver descriptor + * @throws RuntimeException under the {@code fail} policy, if the root is a plaintext remote root + */ + // package-private so tests can exercise the policy directly, without mutating the global engine + static void checkResolverRootProtocol(Map args) { + if (args == null) { + return; + } + String policy = insecureProtocolPolicy(); + if (INSECURE_PROTOCOL_POLICY_IGNORE.equals(policy)) { + return; + } + Object value = args.get("root"); + if (value == null) value = args.get("value"); + if (!(value instanceof CharSequence)) { + return; + } + String root = value.toString(); + if (!isInsecureResolverRoot(root)) { + return; + } + Object name = args.get("name"); + Object label = name != null ? name : root; + if (INSECURE_PROTOCOL_POLICY_FAIL.equals(policy)) { + throw new RuntimeException("Grape resolver '" + label + "' uses the plaintext root '" + root + + "' and was rejected because -D" + INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY + "=" + + INSECURE_PROTOCOL_POLICY_FAIL + " is set. Use an https root, or relax the policy to '" + + INSECURE_PROTOCOL_POLICY_WARN + "' or '" + INSECURE_PROTOCOL_POLICY_IGNORE + "'."); + } + // Warn once per distinct root; a script may add the same resolver repeatedly. The key is + // trimmed to match the classifier, so surrounding whitespace does not defeat the de-dup. + if (WARNED_INSECURE_ROOTS.add(root.trim())) { + LOGGER.log(WARNING, + "Grape resolver ''{0}'' uses the plaintext root ''{1}''; artifacts fetched from it can be" + + " read or modified in transit. Prefer https, or set -D{2}={3} to silence this warning.", + label, root, INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY, INSECURE_PROTOCOL_POLICY_IGNORE); + } + } + + /** + * Returns the configured insecure-protocol policy, defaulting to {@code warn}. An + * unrecognised value falls back to {@code warn} rather than to the laxer {@code ignore}, + * so that a typo cannot silently disable the check; the fallback is reported once per + * offending value. + * + * @return one of {@code fail}, {@code warn} or {@code ignore} + */ + static String insecureProtocolPolicy() { + String policy = System.getProperty(INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY, INSECURE_PROTOCOL_POLICY_WARN) + .trim().toLowerCase(Locale.ROOT); + if (INSECURE_PROTOCOL_POLICY_FAIL.equals(policy) + || INSECURE_PROTOCOL_POLICY_WARN.equals(policy) + || INSECURE_PROTOCOL_POLICY_IGNORE.equals(policy)) { + return policy; + } + if (WARNED_POLICY_VALUES.add(policy)) { + LOGGER.log(WARNING, "Unrecognised -D{0} value ''{1}''; using ''{2}''. Expected one of {3}, {4}, {5}.", + INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY, policy, INSECURE_PROTOCOL_POLICY_WARN, + INSECURE_PROTOCOL_POLICY_FAIL, INSECURE_PROTOCOL_POLICY_WARN, INSECURE_PROTOCOL_POLICY_IGNORE); + } + return INSECURE_PROTOCOL_POLICY_WARN; + } + + /** + * Returns whether the given resolver root fetches over a plaintext protocol from a host + * other than loopback. + *

+ * Only schemes known to be plaintext are classified as insecure, currently {@code http} + * and {@code ftp}. This is deliberately an allow-list of bad schemes rather than a + * deny-list of good ones: transports such as {@code s3} and {@code gs} are encrypted in + * practice and would otherwise be reported falsely. The consequence is that an exotic + * plaintext scheme is not reported, so {@code fail} means "reject known-plaintext roots", + * not "reject anything not proven safe". + *

+ * {@code file:} roots are never insecure. They cross no network, and a {@code file:} root + * on a network mount cannot be distinguished from a local one by inspecting the URI. + * Integrity for such repositories is the job of checksum verification, which applies to + * every transport rather than only to remote ones. Roots which are not valid URIs, or + * which name no scheme at all, are likewise left to the engine. + * + * @param root the resolver root + * @return true if the root is a plaintext remote root + */ + static boolean isInsecureResolverRoot(String root) { + if (root == null) { + return false; + } + String scheme; + String host; + try { + URI uri = new URI(root.trim()); + scheme = uri.getScheme(); + host = uri.getHost(); + } catch (URISyntaxException e) { + return false; // not a URI we can reason about; leave it to the engine + } + if (scheme == null) { + return false; + } + scheme = scheme.toLowerCase(Locale.ROOT); + if (!"http".equals(scheme) && !"ftp".equals(scheme)) { + return false; + } + return !isLoopbackHost(host); + } + + private static boolean isLoopbackHost(String host) { + if (host == null) { + return false; + } + String name = host.toLowerCase(Locale.ROOT); + if (name.startsWith("[") && name.endsWith("]")) { // IPv6 literal + name = name.substring(1, name.length() - 1); + } + if ("localhost".equals(name) || "::1".equals(name)) { + return true; + } + // 127.0.0.0/8, matched as a dotted quad so that a host merely beginning with "127." + // (such as 127.example.com) is not mistaken for a loopback address. An out-of-range quad + // such as 127.999.999.999 cannot reach here: URI.getHost() returns null for it, so + // isInsecureResolverRoot already treats it as insecure without consulting this method. + Matcher ipv4 = IPV4_LITERAL.matcher(name); + return ipv4.matches() && "127".equals(ipv4.group(1)); + } + } diff --git a/src/test/groovy/groovy/grape/GrapeInsecureResolverRootTest.groovy b/src/test/groovy/groovy/grape/GrapeInsecureResolverRootTest.groovy new file mode 100644 index 00000000000..bd71c12c55b --- /dev/null +++ b/src/test/groovy/groovy/grape/GrapeInsecureResolverRootTest.groovy @@ -0,0 +1,193 @@ +/* + * 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 groovy.grape + +import groovy.transform.CompileStatic +import org.junit.jupiter.api.Test + +import static groovy.test.GroovyAssert.shouldFail + +/** + * Tests the classification behind the warning Grape logs for plaintext resolver roots. + * The classification is shared by both engines because every documented route to adding a + * resolver -- {@code @GrabResolver}, the {@code grape} command line tool and + * {@link Grape#addResolver(java.util.Map)} -- passes through the same facade method. + */ +@CompileStatic +final class GrapeInsecureResolverRootTest { + + @Test + void testPlaintextRemoteRootsAreInsecure() { + assert Grape.isInsecureResolverRoot('http://repo.corp.example/maven2') + assert Grape.isInsecureResolverRoot('ftp://repo.corp.example/maven2') + } + + @Test + void testSchemeComparisonIgnoresCase() { + assert Grape.isInsecureResolverRoot('HTTP://repo.corp.example/maven2') + assert !Grape.isInsecureResolverRoot('HTTPS://repo.corp.example/maven2') + } + + @Test + void testSurroundingWhitespaceIsIgnored() { + assert Grape.isInsecureResolverRoot(' http://repo.corp.example/maven2 ') + } + + @Test + void testEncryptedRootsAreNotInsecure() { + assert !Grape.isInsecureResolverRoot('https://repo.maven.apache.org/maven2/') + } + + @Test + void testLocalRootsAreNotInsecure() { + // file: roots never cross a network, so the warning would be noise. + assert !Grape.isInsecureResolverRoot('file:/home/dev/repo') + assert !Grape.isInsecureResolverRoot(new File('build').toURI().toString()) + } + + @Test + void testLoopbackRootsAreExempt() { + // A local mirror or proxy over plaintext is not exposed in transit. + assert !Grape.isInsecureResolverRoot('http://localhost:8081/repository/maven-public') + assert !Grape.isInsecureResolverRoot('http://LocalHost:8081/repository/maven-public') + assert !Grape.isInsecureResolverRoot('http://127.0.0.1:8081/repo') + assert !Grape.isInsecureResolverRoot('http://127.1.2.3/repo') + assert !Grape.isInsecureResolverRoot('http://[::1]:8081/repo') + } + + @Test + void testNonLoopbackLookalikesAreStillInsecure() { + // Guard the prefix test against hosts that merely start with the same text. + assert Grape.isInsecureResolverRoot('http://127.evil.example/repo') + assert Grape.isInsecureResolverRoot('http://localhost.evil.example/repo') + // A dotted quad with an out-of-range octet is not a valid host: URI.getHost() returns + // null for it, so it is treated as insecure rather than exempted as loopback. + assert Grape.isInsecureResolverRoot('http://127.999.999.999/repo') + assert Grape.isInsecureResolverRoot('http://127.0.0.256/repo') + } + + @Test + void testUnusableRootsAreLeftToTheEngine() { + assert !Grape.isInsecureResolverRoot(null) + assert !Grape.isInsecureResolverRoot('') + assert !Grape.isInsecureResolverRoot('not a uri at all') + assert !Grape.isInsecureResolverRoot('repo.corp.example/maven2') // no scheme + } + + @Test + void testUnknownSchemesAreNotReported() { + // Deliberate: an allow-list of known-plaintext schemes, so encrypted transports such + // as s3 and gs are not reported falsely. See the isInsecureResolverRoot javadoc. + assert !Grape.isInsecureResolverRoot('s3://corp-artifacts/maven2') + assert !Grape.isInsecureResolverRoot('gs://corp-artifacts/maven2') + } + + // --- policy selection --- + + @Test + void testPolicyDefaultsToWarn() { + withPolicy(null) { + assert Grape.insecureProtocolPolicy() == 'warn' + } + } + + @Test + void testPolicyValuesAreRecognised() { + withPolicy('fail') { assert Grape.insecureProtocolPolicy() == 'fail' } + withPolicy('warn') { assert Grape.insecureProtocolPolicy() == 'warn' } + withPolicy('ignore') { assert Grape.insecureProtocolPolicy() == 'ignore' } + } + + @Test + void testPolicyIsCaseInsensitiveAndTrimmed() { + withPolicy(' FAIL ') { assert Grape.insecureProtocolPolicy() == 'fail' } + } + + @Test + void testUnrecognisedPolicyFallsBackToWarnNotIgnore() { + // A typo must not silently disable the check, so the fallback is the stricter of the + // two non-failing policies. + withPolicy('flase') { assert Grape.insecureProtocolPolicy() == 'warn' } + withPolicy('true') { assert Grape.insecureProtocolPolicy() == 'warn' } + } + + // --- policy application --- + + // These drive checkResolverRootProtocol directly rather than through addResolver, which would + // mutate the JVM-global resolver list (there is no removeResolver) and leak into other tests. + + @Test + void testFailPolicyRejectsPlaintextRoot() { + withPolicy('fail') { + def ex = shouldFail(RuntimeException) { + Grape.checkResolverRootProtocol([name: 'corp', root: 'http://repo.corp.example/maven2'] as Map) + } + assert ex.message.contains('plaintext root') + assert ex.message.contains(Grape.INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY) + } + } + + @Test + void testFailPolicyAllowsSecureLoopbackAndFileRoots() { + withPolicy('fail') { + // none of these is a plaintext remote root, so the check passes (no exception) + Grape.checkResolverRootProtocol([name: 'secure', root: 'https://repo.corp.example/maven2'] as Map) + Grape.checkResolverRootProtocol([name: 'local', root: 'http://localhost:8081/repo'] as Map) + Grape.checkResolverRootProtocol([name: 'onDisk', root: 'file:/home/dev/repo'] as Map) + } + } + + @Test + void testIgnorePolicyAcceptsPlaintextRoot() { + withPolicy('ignore') { + Grape.checkResolverRootProtocol([name: 'corp', root: 'http://repo.corp.example/maven2'] as Map) + } + } + + // addResolver must apply the policy before adding the resolver, so a rejected root never + // reaches the engine. This is the one path that goes through the public facade, and because + // it throws before the resolver is added it does not mutate the global engine. + @Test + void testAddResolverAppliesPolicyBeforeAdding() { + withPolicy('fail') { + shouldFail(RuntimeException) { + Grape.addResolver([name: 'corp', root: 'http://repo.corp.example/maven2'] as Map) + } + } + } + + private static void withPolicy(String value, Closure body) { + String property = Grape.INSECURE_PROTOCOL_POLICY_SYSTEM_PROPERTY + String previous = System.getProperty(property) + if (value == null) { + System.clearProperty(property) + } else { + System.setProperty(property, value) + } + try { + body() + } finally { + if (previous == null) { + System.clearProperty(property) + } else { + System.setProperty(property, previous) + } + } + } +}