diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java b/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java index 53162bb73a5..9a698a41d48 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java @@ -86,7 +86,6 @@ public class BaseBuilder // Handle local directories fileInitializers.add(new LocalFileInitializer(baseHome)); - // Downloads are allowed to be performed // Setup Maven Local Repo Path localRepoDir = args.findMavenLocalRepoDir(); if (localRepoDir != null) @@ -106,7 +105,7 @@ public class BaseBuilder fileInitializers.add(new BaseHomeFileInitializer(baseHome)); // Normal URL downloads - fileInitializers.add(new UriFileInitializer(baseHome)); + fileInitializers.add(new UriFileInitializer(startArgs, baseHome)); } } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java b/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java index 25e2b689b74..e8a8eea62ec 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java @@ -16,15 +16,20 @@ package org.eclipse.jetty.start; import java.io.Closeable; import java.io.File; import java.io.IOException; -import java.io.InputStream; +import java.net.URI; +import java.nio.file.FileSystem; +import java.nio.file.FileSystemAlreadyExistsException; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; -import java.util.Enumeration; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; import java.util.Locale; -import java.util.zip.ZipEntry; -import java.util.zip.ZipFile; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class FS { @@ -184,40 +189,60 @@ public class FS public static void extractZip(Path archive, Path destination) throws IOException { - try (ZipFile zip = new ZipFile(archive.toFile())) + StartLog.info("extract %s to %s", archive, destination); + URI jaruri = URI.create("jar:" + archive.toUri()); + Map fsEnv = Map.of(); + try (FileSystem zipFs = FileSystems.newFileSystem(jaruri, fsEnv)) { - StartLog.info("extract %s to %s", archive, destination); - Enumeration entries = zip.entries(); - while (entries.hasMoreElements()) + copyZipContents(zipFs.getPath("/"), destination); + } + catch (FileSystemAlreadyExistsException e) + { + FileSystem zipFs = FileSystems.getFileSystem(jaruri); + copyZipContents(zipFs.getPath("/"), destination); + } + } + + public static void copyZipContents(Path root, Path destination) throws IOException + { + if (!Files.exists(destination)) + { + Files.createDirectories(destination); + } + URI outputDirURI = destination.toUri(); + URI archiveURI = root.toUri(); + int archiveURISubIndex = archiveURI.toASCIIString().indexOf("!/") + 2; + + try (Stream entriesStream = Files.walk(root)) + { + // ensure proper unpack order (eg: directories before files) + Stream sorted = entriesStream + .filter((path) -> path.getNameCount() > 0) + .filter((path) -> !path.getName(0).toString().equalsIgnoreCase("META-INF")) + .sorted(); + + Iterator pathIterator = sorted.iterator(); + while (pathIterator.hasNext()) { - ZipEntry entry = entries.nextElement(); - - if (entry.isDirectory() || entry.getName().startsWith("/META-INF")) + Path path = pathIterator.next(); + URI entryURI = path.toUri(); + String subURI = entryURI.toASCIIString().substring(archiveURISubIndex); + URI outputPathURI = outputDirURI.resolve(subURI); + Path outputPath = Path.of(outputPathURI); + StartLog.info("zipFs: %s > %s", path, outputPath); + if (Files.isDirectory(path)) { - // skip - continue; + if (!Files.exists(outputPath)) + Files.createDirectory(outputPath); } - - String entryName = entry.getName(); - Path destFile = destination.resolve(entryName).normalize().toAbsolutePath(); - // make sure extracted path does not escape the destination directory - if (!destFile.startsWith(destination)) + else if (Files.exists(outputPath)) { - throw new IOException(String.format("Malicious Archive %s found with bad entry \"%s\"", - archive, entryName)); - } - if (!Files.exists(destFile)) - { - FS.ensureDirectoryExists(destFile.getParent()); - try (InputStream input = zip.getInputStream(entry)) - { - StartLog.debug("extracting %s", destFile); - Files.copy(input, destFile); - } + StartLog.debug("skipping extraction (file exists) %s", outputPath); } else { - StartLog.debug("skipping extract (file exists) %s", destFile); + StartLog.info("extracting %s to %s", path, outputPath); + Files.copy(path, outputPath); } } } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java b/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java index e6d95aebc78..0d26cdcf894 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java @@ -109,7 +109,11 @@ public abstract class FileInitializer Path destination = _basehome.getBasePath(location); - // now on copy/download paths (be safe above all else) + // We restrict our behavior to only modifying what exists in ${jetty.base}. + // If the user decides they want to use advanced setups, such as symlinks to point + // to content outside of ${jetty.base}, that is their decision ad we will not + // attempt to save them from themselves. + // Note: All copy and extract steps, will not replace files that already exist. if (destination != null && !destination.startsWith(_basehome.getBasePath())) throw new IOException("For security reasons, Jetty start is unable to process file resource not in ${jetty.base} - " + location); @@ -120,35 +124,6 @@ public abstract class FileInitializer return destination; } - protected void download(URI uri, Path destination) throws IOException - { - if (FS.ensureDirectoryExists(destination.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); - - StartLog.info("download %s to %s", uri, _basehome.toShortForm(destination)); - - URLConnection connection = uri.toURL().openConnection(); - - if (connection instanceof HttpURLConnection) - { - HttpURLConnection http = (HttpURLConnection)uri.toURL().openConnection(); - http.setInstanceFollowRedirects(true); - http.setAllowUserInteraction(false); - - int status = http.getResponseCode(); - - if (status != HttpURLConnection.HTTP_OK) - { - throw new IOException("URL GET Failure [" + status + "/" + http.getResponseMessage() + "] on " + uri); - } - } - - try (InputStream in = connection.getInputStream()) - { - Files.copy(in, destination, StandardCopyOption.REPLACE_EXISTING); - } - } - /** * Test if any of the Paths exist (as files) * @@ -199,7 +174,11 @@ public abstract class FileInitializer if (copyDirectory(from, to)) modified = true; } - else if (!Files.exists(to)) + else if (Files.exists(to)) + { + StartLog.debug("skipping copy (file exists in destination) %s", to); + } + else { StartLog.info("copy %s to %s", _basehome.toShortForm(from), _basehome.toShortForm(to)); Files.copy(from, to); diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index 17d4a0ec4d5..13b265ce5ec 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java @@ -54,6 +54,7 @@ public class StartArgs public static final String VERSION; public static final Set ALL_PARTS = Set.of("java", "opts", "path", "main", "args"); public static final Set ARG_PARTS = Set.of("args"); + public static final String ARG_ALLOW_INSECURE_HTTP_DOWNLOADS = "--allow-insecure-http-downloads"; private static final String JETTY_VERSION_KEY = "jetty.version"; private static final String JETTY_TAG_NAME_KEY = "jetty.tag.version"; @@ -216,6 +217,7 @@ public class StartArgs private boolean exec = false; private String execProperties; + private boolean allowInsecureHttpDownloads = false; private boolean approveAllLicenses = false; public StartArgs(BaseHome baseHome) @@ -960,6 +962,11 @@ public class StartArgs return false; } + public boolean isAllowInsecureHttpDownloads() + { + return allowInsecureHttpDownloads; + } + public boolean isApproveAllLicenses() { return approveAllLicenses; @@ -1244,6 +1251,13 @@ public class StartArgs return; } + // Allow insecure-http downloads + if (ARG_ALLOW_INSECURE_HTTP_DOWNLOADS.equals(arg)) + { + allowInsecureHttpDownloads = true; + return; + } + // Enable forked execution of Jetty server if ("--approve-all-licenses".equals(arg)) { diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java new file mode 100644 index 00000000000..c6aca076d95 --- /dev/null +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java @@ -0,0 +1,122 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.start.fileinits; + +import java.io.IOException; +import java.io.InputStream; +import java.net.ProxySelector; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Optional; + +import org.eclipse.jetty.start.BaseHome; +import org.eclipse.jetty.start.FS; +import org.eclipse.jetty.start.FileInitializer; +import org.eclipse.jetty.start.StartArgs; +import org.eclipse.jetty.start.StartLog; + +public abstract class DownloadFileInitializer extends FileInitializer +{ + private HttpClient httpClient; + + protected DownloadFileInitializer(BaseHome basehome, String... scheme) + { + super(basehome, scheme); + } + + protected abstract boolean allowInsecureHttpDownloads(); + + protected void download(URI uri, Path destination) throws IOException + { + if ("http".equalsIgnoreCase(uri.getScheme()) && !allowInsecureHttpDownloads()) + throw new IOException("Insecure HTTP download not allowed (use " + StartArgs.ARG_ALLOW_INSECURE_HTTP_DOWNLOADS + " to bypass): " + uri); + + if (Files.exists(destination)) + { + if (Files.isRegularFile(destination)) + { + StartLog.warn("skipping download of %s : file exists in destination %s", uri, destination); + } + else + { + StartLog.warn("skipping download of %s : path conflict at destination %s", uri, destination); + } + return; + } + + if (FS.ensureDirectoryExists(destination.getParent())) + StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + + StartLog.info("download %s to %s", uri, _basehome.toShortForm(destination)); + + HttpClient httpClient = getHttpClient(); + + try + { + HttpRequest request = HttpRequest.newBuilder() + .uri(uri) + .GET() + .build(); + + HttpResponse response = + httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); + + int status = response.statusCode(); + + if (!allowInsecureHttpDownloads() && (status >= 300) && (status <= 399)) + { + // redirection status, provide more details in error + Optional location = response.headers().firstValue("Location"); + if (location.isPresent()) + { + throw new IOException("URL GET Failure [status " + status + "] on " + uri + + " wanting to redirect to insecure HTTP location (use " + + StartArgs.ARG_ALLOW_INSECURE_HTTP_DOWNLOADS + " to bypass): " + + location.get()); + } + } + + if (status != 200) + { + throw new IOException("URL GET Failure [status " + status + "] on " + uri); + } + + try (InputStream in = response.body()) + { + Files.copy(in, destination, StandardCopyOption.REPLACE_EXISTING); + } + } + catch (InterruptedException e) + { + throw new IOException("Failed to GET: " + uri, e); + } + } + + private HttpClient getHttpClient() + { + if (httpClient == null) + { + httpClient = HttpClient.newBuilder() + .followRedirects(allowInsecureHttpDownloads() ? HttpClient.Redirect.ALWAYS : HttpClient.Redirect.NORMAL) + .proxy(ProxySelector.getDefault()) + .build(); + } + return httpClient; + } +} diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java index 5f9674881da..63ee792e40c 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java @@ -22,7 +22,6 @@ import javax.xml.parsers.ParserConfigurationException; import org.eclipse.jetty.start.BaseHome; import org.eclipse.jetty.start.FS; -import org.eclipse.jetty.start.FileInitializer; import org.eclipse.jetty.start.StartLog; import org.eclipse.jetty.start.Utils; import org.xml.sax.SAXException; @@ -42,7 +41,7 @@ import org.xml.sax.SAXException; *
optional type and classifier requirement
* */ -public class MavenLocalRepoFileInitializer extends FileInitializer +public class MavenLocalRepoFileInitializer extends DownloadFileInitializer { public static class Coordinates { @@ -126,6 +125,19 @@ public class MavenLocalRepoFileInitializer extends FileInitializer this.mavenRepoUri = mavenRepoUri; } + @Override + protected boolean allowInsecureHttpDownloads() + { + // Always allow insecure http downloads in this file initializer. + + // The user is either using the DEFAULT_REMOTE_REPO, or has redeclared it to a new URI. + // If the `maven.repo.uri` property has been changed from default, this indicates a change + // to a different maven uri, overwhelmingly pointing to a maven repository manager + // like artifactory or nexus. This is viewed as an intentional decision by the + // user and as such we should not put additional hurdles in their way. + return true; + } + private static Path newTempRepo() { Path javaTempDir = Paths.get(System.getProperty("java.io.tmpdir")); diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenMetadata.java b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenMetadata.java index 87c683c5601..9e7134ac80f 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenMetadata.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenMetadata.java @@ -26,6 +26,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -56,6 +57,7 @@ public class MavenMetadata throws IOException, ParserConfigurationException, SAXException { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); DocumentBuilder builder = factory.newDocumentBuilder(); try (InputStream input = Files.newInputStream(metadataXml)) { diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/UriFileInitializer.java b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/UriFileInitializer.java index efdd404bdba..737454b9edd 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/UriFileInitializer.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/UriFileInitializer.java @@ -19,13 +19,22 @@ import java.nio.file.Files; import java.nio.file.Path; import org.eclipse.jetty.start.BaseHome; -import org.eclipse.jetty.start.FileInitializer; +import org.eclipse.jetty.start.StartArgs; -public class UriFileInitializer extends FileInitializer +public class UriFileInitializer extends DownloadFileInitializer { - public UriFileInitializer(BaseHome baseHome) + protected final boolean _allowInsecureHttpDownloads; + + public UriFileInitializer(StartArgs startArgs, BaseHome baseHome) { super(baseHome, "http", "https"); + _allowInsecureHttpDownloads = startArgs.isAllowInsecureHttpDownloads(); + } + + @Override + protected boolean allowInsecureHttpDownloads() + { + return _allowInsecureHttpDownloads; } @Override diff --git a/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt b/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt index c392c6b8ee5..19a865b7ccd 100644 --- a/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt +++ b/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt @@ -191,6 +191,9 @@ Options: issues when the Jetty logging module has not yet started due to configuration errors. + --allow-insecure-http-downloads + Allow the use of insecure `http://` scheme for content download. + --approve-all-licenses Approves all license questions from modules that have particular license requirements.