jetty-start cleanup (#9555)

* Extract jars/zips using zipfs
* Restrict MavenMetadata external DTD/XSD access
* Introduce --allow-insecure-http-downloads
* Produce debug log if file exists in destination during copy.
* Simpler MavenMetadata
* If `maven.repo.uri` has been redeclared by user, automatically set allow insecure http downloads.

---------

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Joakim Erdfelt 2023-04-10 08:53:58 -05:00 committed by GitHub
parent 2c61011de1
commit 81efae2f98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 233 additions and 68 deletions

View File

@ -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));
}
}

View File

@ -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<String, ?> fsEnv = Map.of();
try (FileSystem zipFs = FileSystems.newFileSystem(jaruri, fsEnv))
{
StartLog.info("extract %s to %s", archive, destination);
Enumeration<? extends ZipEntry> 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<Path> entriesStream = Files.walk(root))
{
// ensure proper unpack order (eg: directories before files)
Stream<Path> sorted = entriesStream
.filter((path) -> path.getNameCount() > 0)
.filter((path) -> !path.getName(0).toString().equalsIgnoreCase("META-INF"))
.sorted();
Iterator<Path> 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);
}
}
}

View File

@ -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);

View File

@ -54,6 +54,7 @@ public class StartArgs
public static final String VERSION;
public static final Set<String> ALL_PARTS = Set.of("java", "opts", "path", "main", "args");
public static final Set<String> 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))
{

View File

@ -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<InputStream> 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<String> 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;
}
}

View File

@ -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;
* <dd>optional type and classifier requirement</dd>
* </dl>
*/
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"));

View File

@ -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))
{

View File

@ -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

View File

@ -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.