From 998d82ae31fec4f87092343190efd77d0ee177df Mon Sep 17 00:00:00 2001 From: Tako Schotanus Date: Wed, 13 Mar 2024 22:49:21 +0100 Subject: [PATCH 1/3] fix: make sure we don't ask for the same trust twice --- src/main/java/dev/jbang/cli/Trust.java | 2 +- .../java/dev/jbang/net/TrustedSources.java | 35 +++++++++++++++++-- .../resolvers/RemoteResourceResolver.java | 9 ++--- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/main/java/dev/jbang/cli/Trust.java b/src/main/java/dev/jbang/cli/Trust.java index 2b3923d27..5755428d3 100644 --- a/src/main/java/dev/jbang/cli/Trust.java +++ b/src/main/java/dev/jbang/cli/Trust.java @@ -23,7 +23,7 @@ public class Trust { @CommandLine.Command(name = "add", description = "Add trust domains.") public Integer add( @CommandLine.Parameters(index = "0", description = "Rules for trusted sources", arity = "1..*") List rules) { - TrustedSources.instance().add(rules, Settings.getTrustedSourcesFile().toFile()); + TrustedSources.instance().add(rules); return EXIT_OK; } diff --git a/src/main/java/dev/jbang/net/TrustedSources.java b/src/main/java/dev/jbang/net/TrustedSources.java index 8ce4c85d2..9b6cf7077 100644 --- a/src/main/java/dev/jbang/net/TrustedSources.java +++ b/src/main/java/dev/jbang/net/TrustedSources.java @@ -32,11 +32,13 @@ public class TrustedSources { final static Pattern r127 = Pattern.compile("(?i)^127.0.0.1(:\\d+)?$"); private String[] trustedSources; + private String[] temporaryTrustedSources; private static TrustedSources instance; - public TrustedSources(String[] trustedSources) { + TrustedSources(String[] trustedSources) { this.trustedSources = trustedSources; + this.temporaryTrustedSources = new String[0]; } public static TrustedSources load(Path toPath) throws IOException { @@ -60,7 +62,7 @@ public String[] getTrustedSources() { } /** - * Check whether a url like https://www.microsoft.com matches the list of + * Check whether a URL like https://www.microsoft.com matches the list of * trusted sources. * * - Schemes must match - There's no subdomain matching. For example @@ -73,6 +75,15 @@ public boolean isURLTrusted(URI url) throws URISyntaxException { return true; } + boolean trusted = isURLTrusted(url, trustedSources) || + isURLTrusted(url, temporaryTrustedSources) || + // default trusted for usability and trust + url.toString().startsWith("https://github.com/jbangdev/"); + + return trusted; + } + + private boolean isURLTrusted(URI url, String[] trustedSources) throws URISyntaxException { final String domain = String.format("%s://%s", url.getScheme(), url.getAuthority()); for (String trustedSource : trustedSources) { @@ -193,6 +204,24 @@ protected String getJSon(Collection rules) { return trustedsources; } + public void addTemporary(String trust) { + Util.infoMsg("Adding temporary " + trust); + Set newrules = new LinkedHashSet<>(Arrays.asList(temporaryTrustedSources)); + if (newrules.add(trust)) { + temporaryTrustedSources = newrules.toArray(new String[0]); + } else { + Util.warnMsg("Already trusted source(s). No changes made."); + } + } + + public void add(String trust) { + add(trust, Settings.getTrustedSourcesFile().toFile()); + } + + public void add(List trust) { + add(trust, Settings.getTrustedSourcesFile().toFile()); + } + public void add(String trust, File storage) { add(Collections.singletonList(trust), storage); } @@ -204,6 +233,7 @@ public void add(List trust, File storage) { Set newrules = new LinkedHashSet<>(Arrays.asList(trustedSources)); if (newrules.addAll(trust)) { + trustedSources = newrules.toArray(new String[0]); save(newrules, storage); } else { Util.warnMsg("Already trusted source(s). No changes made."); @@ -218,6 +248,7 @@ public void remove(List trust, File storage) { Set newrules = new LinkedHashSet<>(Arrays.asList(trustedSources)); if (newrules.removeAll(trust)) { + trustedSources = newrules.toArray(new String[0]); save(newrules, storage); } else { Util.warnMsg("Not found in trusted source(s). No changes made."); diff --git a/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java b/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java index bcc4cbfb9..60fb5eead 100644 --- a/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java +++ b/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java @@ -9,7 +9,6 @@ import java.util.ArrayList; import java.util.List; -import dev.jbang.Settings; import dev.jbang.cli.BaseCommand; import dev.jbang.cli.ExitException; import dev.jbang.net.TrustedSources; @@ -68,10 +67,12 @@ public static ResourceRef fetchScriptFromUntrustedURL(String scriptURL) { int result = Util.askInput(question, 30, 0, options.toArray(new String[] {})); TrustedSources ts = TrustedSources.instance(); - if (result == 2) { - ts.add(trustUrl, Settings.getTrustedSourcesFile().toFile()); + if (result == 1) { + ts.addTemporary(trustUrl); + } else if (result == 2) { + ts.add(trustUrl); } else if (result == 3) { - ts.add(trustOrgUrl, Settings.getTrustedSourcesFile().toFile()); + ts.add(trustOrgUrl); } else if (result <= 0) { String exmsg = scriptURL + " is not from a trusted source and user did not confirm trust thus aborting.\n" + From bc32cd3429d871301b2cf3b4e4a80ee02e61589b Mon Sep 17 00:00:00 2001 From: Tako Schotanus Date: Wed, 13 Mar 2024 23:21:52 +0100 Subject: [PATCH 2/3] fix: now handling more HTTP redirect types We were handling only a limited set of redirect types, this has now been extended to include all common and not so common 3xx redirect codes. This includes the somewhat strange 303 See Other where the spec isn't completely clear but the general concensus seems to be that it forces the next request to be a GET request. Fixes #1769 --- src/main/java/dev/jbang/util/Util.java | 32 +++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/main/java/dev/jbang/util/Util.java b/src/main/java/dev/jbang/util/Util.java index fe132c321..8be6ab513 100644 --- a/src/main/java/dev/jbang/util/Util.java +++ b/src/main/java/dev/jbang/util/Util.java @@ -1063,16 +1063,16 @@ private static String extractFileName(URLConnection urlConnection) throws IOExce // extracts file name from header field fileName = getDispositionFilename(disposition); } - if (isBlankString(fileName)) { - // extracts file name from URL if nothing found - int p = fileURL.indexOf("?"); - // Strip parameters from the URL (if any) - String simpleUrl = (p > 0) ? fileURL.substring(0, p) : fileURL; - while (simpleUrl.endsWith("/")) { - simpleUrl = simpleUrl.substring(0, simpleUrl.length() - 1); - } - fileName = simpleUrl.substring(simpleUrl.lastIndexOf("/") + 1); + } + if (isBlankString(fileName)) { + // extracts file name from URL if nothing found + int p = fileURL.indexOf("?"); + // Strip parameters from the URL (if any) + String simpleUrl = (p > 0) ? fileURL.substring(0, p) : fileURL; + while (simpleUrl.endsWith("/")) { + simpleUrl = simpleUrl.substring(0, simpleUrl.length() - 1); } + fileName = simpleUrl.substring(simpleUrl.lastIndexOf("/") + 1); } } else { fileName = fileURL.substring(fileURL.lastIndexOf("/") + 1); @@ -1087,17 +1087,27 @@ private static HttpURLConnection handleRedirects(HttpURLConnection httpConn, Con while (true) { httpConn.setInstanceFollowRedirects(false); responseCode = httpConn.getResponseCode(); - if (responseCode == HttpURLConnection.HTTP_MOVED_PERM || + if (responseCode == HttpURLConnection.HTTP_MULT_CHOICE || + responseCode == HttpURLConnection.HTTP_MOVED_PERM || responseCode == HttpURLConnection.HTTP_MOVED_TEMP || - responseCode == 307 /* TEMP REDIRECT */) { + responseCode == HttpURLConnection.HTTP_SEE_OTHER || + responseCode == 307 /* TEMP REDIRECT */ || + responseCode == 308 /* PERM REDIRECT */) { if (redirects++ > 8) { throw new IOException("Too many redirects"); } String location = httpConn.getHeaderField("Location"); + if (location == null) { + throw new IOException("No 'Location' header in redirect"); + } URL url = new URL(httpConn.getURL(), location); url = new URL(swizzleURL(url.toString())); verboseMsg("Redirected to: " + url); // Should be debug info httpConn = (HttpURLConnection) url.openConnection(); + if (responseCode == HttpURLConnection.HTTP_SEE_OTHER) { + // This response code forces the method to GET + httpConn.setRequestMethod("GET"); + } configurator.configure(httpConn); continue; } From ed6b115405e346d83e07ba2e60b15c798b90dd30 Mon Sep 17 00:00:00 2001 From: Tako Schotanus Date: Thu, 21 Mar 2024 16:52:33 +0100 Subject: [PATCH 3/3] chore: improved trust console messages --- src/main/java/dev/jbang/net/TrustedSources.java | 6 +++--- .../dev/jbang/source/resolvers/RemoteResourceResolver.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/jbang/net/TrustedSources.java b/src/main/java/dev/jbang/net/TrustedSources.java index 9b6cf7077..c1c9a5faa 100644 --- a/src/main/java/dev/jbang/net/TrustedSources.java +++ b/src/main/java/dev/jbang/net/TrustedSources.java @@ -205,7 +205,7 @@ protected String getJSon(Collection rules) { } public void addTemporary(String trust) { - Util.infoMsg("Adding temporary " + trust); + Util.infoMsg("Trusting for this run: " + trust); Set newrules = new LinkedHashSet<>(Arrays.asList(temporaryTrustedSources)); if (newrules.add(trust)) { temporaryTrustedSources = newrules.toArray(new String[0]); @@ -228,7 +228,7 @@ public void add(String trust, File storage) { public void add(List trust, File storage) { - Util.infoMsg("Adding " + trust + " to " + storage); + Util.infoMsg("Trusting permanently: " + trust); Set newrules = new LinkedHashSet<>(Arrays.asList(trustedSources)); @@ -243,7 +243,7 @@ public void add(List trust, File storage) { public void remove(List trust, File storage) { - Util.infoMsg("Removing " + trust + " from " + storage); + Util.infoMsg("Removing permanent trust: " + trust); Set newrules = new LinkedHashSet<>(Arrays.asList(trustedSources)); diff --git a/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java b/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java index 60fb5eead..56f01a3fa 100644 --- a/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java +++ b/src/main/java/dev/jbang/source/resolvers/RemoteResourceResolver.java @@ -59,7 +59,7 @@ public static ResourceRef fetchScriptFromUntrustedURL(String scriptURL) { String trustOrgUrl = orgURL(trustUrl); List options = new ArrayList<>(); options.add( - "Trust once: Add no trust, just download this time (can be run multiple times while cached)"); + "Trust once: Add no trust, only allow access to this URL for the duration of this run"); options.add("Trust limited url in future: " + trustUrl); if (trustOrgUrl != null) { options.add("Trust organization url in future: " + trustOrgUrl);