From 61607d6469d42609267902781dd804f6d5c19c8f Mon Sep 17 00:00:00 2001 From: Laurent Goujon Date: Sat, 27 Apr 2024 14:05:23 -0700 Subject: [PATCH 1/4] Fix modular jar final permissions When a new modular jar file is generated with maven-jar-plugin with Java 11, the final permissions of the file are restricted to the current user instead of using the environment umask which usually allows for group and other users to access the file as well. This is caused by the use of Files#createTempFile() which has a restrictive file permission model for security reason but as the temporary file is generated next to the original jar file, and there's no sensitive reason to restrict its access, the restrictive file permission should not be needed. Fix the issue by creating a simple temporary file generator method. --- .../jar/JarToolModularJarArchiver.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java index b936ed318..161d3f3f9 100644 --- a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.PrintStream; import java.lang.reflect.Method; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -31,6 +32,7 @@ import java.util.Enumeration; import java.util.List; import java.util.Locale; +import java.util.Random; import java.util.TimeZone; import java.util.regex.Pattern; import java.util.zip.ZipEntry; @@ -147,7 +149,7 @@ protected void postCreateArchive() throws ArchiverException { private void fixLastModifiedTimeZipEntries() throws IOException { long timeMillis = getLastModifiedTime().toMillis(); Path destFile = getDestFile().toPath(); - Path tmpZip = Files.createTempFile(destFile.getParent(), null, null); + Path tmpZip = createTempFile(destFile.getParent()); try (ZipFile zipFile = new ZipFile(getDestFile()); ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(tmpZip))) { Enumeration entries = zipFile.entries(); @@ -263,4 +265,24 @@ private boolean isJarDateOptionSupported(Method runMethod) { return false; } } + + /** + * Create a temporary file in the provided directory. + * + * It is an unsecure replacement for {@code Files#createTempFile(Path, String, String, java.nio.file.attribute.FileAttribute...)}: + * The new file permissions are controlled by the umask property instead of just being accessible to the current user. + */ + private Path createTempFile(Path dir) throws IOException { + Random random = new Random(); + for (; ; ) { + + String name = Long.toUnsignedString(random.nextLong()) + ".tmp"; + Path path = dir.resolve(name); + try { + return Files.createFile(path); + } catch (FileAlreadyExistsException e) { + // retry; + } + } + } } From 6bd83e1c57a472f9a9227afb5adcb0fdc7b749aa Mon Sep 17 00:00:00 2001 From: Laurent Goujon Date: Wed, 1 May 2024 10:47:49 -0700 Subject: [PATCH 2/4] Copy mjar permissions when fixing modification times Instead of relying on current umask property, read mjar permissions and provide it to Files#createTempFile(...) --- .../jar/JarToolModularJarArchiver.java | 41 ++++++++----------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java index 161d3f3f9..c082171ad 100644 --- a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java @@ -22,17 +22,20 @@ import java.io.IOException; import java.io.PrintStream; import java.lang.reflect.Method; -import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.FileTime; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.Calendar; import java.util.Enumeration; import java.util.List; import java.util.Locale; -import java.util.Random; import java.util.TimeZone; import java.util.regex.Pattern; import java.util.zip.ZipEntry; @@ -68,6 +71,9 @@ public class JarToolModularJarArchiver extends ModularJarArchiver { private static final Pattern MRJAR_VERSION_AREA = Pattern.compile("META-INF/versions/\\d+/"); + private static final boolean IS_POSIX = + FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); + private Object jarTool; private boolean moduleDescriptorFound; @@ -149,7 +155,16 @@ protected void postCreateArchive() throws ArchiverException { private void fixLastModifiedTimeZipEntries() throws IOException { long timeMillis = getLastModifiedTime().toMillis(); Path destFile = getDestFile().toPath(); - Path tmpZip = createTempFile(destFile.getParent()); + FileAttribute[] attributes; + if (IS_POSIX) { + PosixFileAttributes posixFileAttributes = Files.getFileAttributeView(destFile, PosixFileAttributeView.class) + .readAttributes(); + attributes = new FileAttribute[1]; + attributes[0] = PosixFilePermissions.asFileAttribute(posixFileAttributes.permissions()); + } else { + attributes = new FileAttribute[0]; + } + Path tmpZip = Files.createTempFile(destFile.getParent(), null, null, attributes); try (ZipFile zipFile = new ZipFile(getDestFile()); ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(tmpZip))) { Enumeration entries = zipFile.entries(); @@ -265,24 +280,4 @@ private boolean isJarDateOptionSupported(Method runMethod) { return false; } } - - /** - * Create a temporary file in the provided directory. - * - * It is an unsecure replacement for {@code Files#createTempFile(Path, String, String, java.nio.file.attribute.FileAttribute...)}: - * The new file permissions are controlled by the umask property instead of just being accessible to the current user. - */ - private Path createTempFile(Path dir) throws IOException { - Random random = new Random(); - for (; ; ) { - - String name = Long.toUnsignedString(random.nextLong()) + ".tmp"; - Path path = dir.resolve(name); - try { - return Files.createFile(path); - } catch (FileAlreadyExistsException e) { - // retry; - } - } - } } From 772306805aa2f9d7e558d300b079f9d7a2f2014b Mon Sep 17 00:00:00 2001 From: Laurent Goujon Date: Wed, 1 May 2024 10:55:34 -0700 Subject: [PATCH 3/4] Clean temporary file if an error occurs --- .../jar/JarToolModularJarArchiver.java | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java index c082171ad..ce178896a 100644 --- a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java @@ -165,22 +165,32 @@ private void fixLastModifiedTimeZipEntries() throws IOException { attributes = new FileAttribute[0]; } Path tmpZip = Files.createTempFile(destFile.getParent(), null, null, attributes); - try (ZipFile zipFile = new ZipFile(getDestFile()); - ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(tmpZip))) { - Enumeration entries = zipFile.entries(); - while (entries.hasMoreElements()) { - ZipEntry entry = entries.nextElement(); - // Not using setLastModifiedTime(FileTime) as it sets the extended timestamp - // which is not compatible with the jar tool output. - entry.setTime(timeMillis); - out.putNextEntry(entry); - if (!entry.isDirectory()) { - IOUtil.copy(zipFile.getInputStream(entry), out); + try { + try (ZipFile zipFile = new ZipFile(getDestFile()); + ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(tmpZip))) { + Enumeration entries = zipFile.entries(); + while (entries.hasMoreElements()) { + ZipEntry entry = entries.nextElement(); + // Not using setLastModifiedTime(FileTime) as it sets the extended timestamp + // which is not compatible with the jar tool output. + entry.setTime(timeMillis); + out.putNextEntry(entry); + if (!entry.isDirectory()) { + IOUtil.copy(zipFile.getInputStream(entry), out); + } + out.closeEntry(); } - out.closeEntry(); } + Files.move(tmpZip, destFile, StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + // Clean up temporary file if an error occurs + try { + Files.delete(tmpZip); + } catch (IOException ioe) { + e.addSuppressed(ioe); + } + throw e; } - Files.move(tmpZip, destFile, StandardCopyOption.REPLACE_EXISTING); } /** From b6a14503fcafb987fa733f9d97b91d57eaf6638d Mon Sep 17 00:00:00 2001 From: Laurent Goujon Date: Tue, 7 May 2024 14:39:46 -0700 Subject: [PATCH 4/4] Do not follow symlink when reading jar attributes --- .../archiver/jar/JarToolModularJarArchiver.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java index ce178896a..93c95a51c 100644 --- a/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java @@ -22,8 +22,8 @@ import java.io.IOException; import java.io.PrintStream; import java.lang.reflect.Method; -import java.nio.file.FileSystems; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileAttribute; @@ -71,9 +71,6 @@ public class JarToolModularJarArchiver extends ModularJarArchiver { private static final Pattern MRJAR_VERSION_AREA = Pattern.compile("META-INF/versions/\\d+/"); - private static final boolean IS_POSIX = - FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); - private Object jarTool; private boolean moduleDescriptorFound; @@ -155,10 +152,11 @@ protected void postCreateArchive() throws ArchiverException { private void fixLastModifiedTimeZipEntries() throws IOException { long timeMillis = getLastModifiedTime().toMillis(); Path destFile = getDestFile().toPath(); + PosixFileAttributes posixFileAttributes = Files.getFileAttributeView( + destFile, PosixFileAttributeView.class, LinkOption.NOFOLLOW_LINKS) + .readAttributes(); FileAttribute[] attributes; - if (IS_POSIX) { - PosixFileAttributes posixFileAttributes = Files.getFileAttributeView(destFile, PosixFileAttributeView.class) - .readAttributes(); + if (posixFileAttributes != null) { attributes = new FileAttribute[1]; attributes[0] = PosixFilePermissions.asFileAttribute(posixFileAttributes.permissions()); } else {