From 6bd9b5f4d4e267b226c40fe3aaea3c79a4480983 Mon Sep 17 00:00:00 2001 From: janakmulani Date: Tue, 4 Feb 2025 11:32:47 +0530 Subject: [PATCH 1/3] OWS-644, ITW 955 : fix restricted file ACL --- .../jnlp/util/RestrictedFileUtils.java | 24 ++++++++++++++++++- .../jnlp/util/RestrictedFileUtilsTest.java | 3 ++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java b/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java index 71c499dba..3f2549779 100644 --- a/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java +++ b/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java @@ -23,16 +23,19 @@ import java.io.File; import java.io.IOException; +import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.attribute.AclEntry; import java.nio.file.attribute.AclEntryFlag; import java.nio.file.attribute.AclEntryPermission; import java.nio.file.attribute.AclEntryType; import java.nio.file.attribute.AclFileAttributeView; +import java.security.Principal; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import static java.lang.Boolean.parseBoolean; @@ -49,6 +52,8 @@ public final class RestrictedFileUtils { private static final Logger LOG = LoggerFactory.getLogger(RestrictedFileUtils.class); private static final List WIN_ROOT_PRINCIPALS = Arrays.asList("NT AUTHORITY\\SYSTEM", "BUILTIN\\Administrators"); + private static final List WIN_PRINCIPAL_SIDS = Arrays.asList("S-1-5-18" /*NT AUTHORITY\SYSTEM*/, "S-1-5-32-544" /*BUILTIN\Administrators*/, "S-1-5-11" /*NT AUTHORITY\Authenticated Users*/); + public static final String NT_AUTHENTICATED_USER_SID = "S-1-5-11"; /** * Creates a new directory with minimum permissions. The directory is not @@ -125,7 +130,8 @@ private static void createRestrictedFile(File file, boolean isDir) throws IOExce String owner = view.getOwner().getName(); for (AclEntry ae : view.getAcl()) { String principalName = ae.principal().getName(); - if (WIN_ROOT_PRINCIPALS.contains(principalName) || owner.equals(principalName)) { + if (WIN_ROOT_PRINCIPALS.contains(principalName) || owner.equals(principalName) || principalInWinSIDS(ae.principal())) { + LOG.debug("Allowing permissions on restricted file {} for principal {} : {} ", tempFile.getAbsolutePath(), principalName, getSIDForPrincipal(ae.principal())); list.add(AclEntry.newBuilder() .setType(AclEntryType.ALLOW) .setPrincipal(ae.principal()) @@ -189,4 +195,20 @@ private static void createFileOrDir(File file, boolean isDir) throws IOException } } } + + public static boolean principalInWinSIDS(Principal principal) { + return WIN_PRINCIPAL_SIDS.contains(getSIDForPrincipal(principal)); + } + + public static String getSIDForPrincipal(Principal principal) { + try { + Class clazz = principal.getClass().getSuperclass(); + Method method = clazz.getDeclaredMethod("sidString"); + method.setAccessible(true); + return (String) method.invoke(principal); + } catch (Exception e) { + LOG.debug("No SID for {}", principal.getName()); + return ""; + } + } } diff --git a/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java b/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java index 50a6e7490..552714ea1 100644 --- a/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java +++ b/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java @@ -9,6 +9,7 @@ import java.nio.file.attribute.AclEntry; import java.nio.file.attribute.AclFileAttributeView; +import static net.sourceforge.jnlp.util.RestrictedFileUtils.getSIDForPrincipal; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -32,7 +33,7 @@ public void testCreateRestrictedFile() throws Exception { boolean hasOwner = false; AclFileAttributeView view = Files.getFileAttributeView(testfile.toPath(), AclFileAttributeView.class); for (AclEntry ae : view.getAcl()) { - if (view.getOwner().getName().equals(ae.principal().getName())) { + if (view.getOwner().getName().equals(ae.principal().getName()) || getSIDForPrincipal(ae.principal()).equals(RestrictedFileUtils.NT_AUTHENTICATED_USER_SID)) { assertFalse("Duplicate owner entry", hasOwner); hasOwner = true; assertEquals("Owner must have all permissions", 14, ae.permissions().size()); From b2d165d22f983e8e4ca2ac251020c1b1c2c0abcd Mon Sep 17 00:00:00 2001 From: janakmulani Date: Tue, 4 Feb 2025 19:16:43 +0530 Subject: [PATCH 2/3] OWS-644, ITW 955 : add owner ACLs to the restricted file --- .../jnlp/util/RestrictedFileUtils.java | 42 +++++++++++++------ .../jnlp/util/RestrictedFileUtilsTest.java | 3 +- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java b/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java index 3f2549779..70ab11de7 100644 --- a/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java +++ b/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java @@ -35,7 +35,6 @@ import java.util.Arrays; import java.util.LinkedHashSet; import java.util.List; -import java.util.Optional; import java.util.Set; import static java.lang.Boolean.parseBoolean; @@ -51,9 +50,9 @@ public final class RestrictedFileUtils { private static final Logger LOG = LoggerFactory.getLogger(RestrictedFileUtils.class); - private static final List WIN_ROOT_PRINCIPALS = Arrays.asList("NT AUTHORITY\\SYSTEM", "BUILTIN\\Administrators"); - private static final List WIN_PRINCIPAL_SIDS = Arrays.asList("S-1-5-18" /*NT AUTHORITY\SYSTEM*/, "S-1-5-32-544" /*BUILTIN\Administrators*/, "S-1-5-11" /*NT AUTHORITY\Authenticated Users*/); - public static final String NT_AUTHENTICATED_USER_SID = "S-1-5-11"; + private static final List WIN_PRINCIPAL_SIDS = Arrays.asList( + "S-1-5-18" /*NT AUTHORITY\SYSTEM*/, + "S-1-5-32-544" /*BUILTIN\Administrators*/); /** * Creates a new directory with minimum permissions. The directory is not @@ -127,10 +126,9 @@ private static void createRestrictedFile(File file, boolean isDir) throws IOExce // filter ACL's leaving only root and owner AclFileAttributeView view = Files.getFileAttributeView(tempFile.toPath(), AclFileAttributeView.class); List list = new ArrayList<>(); - String owner = view.getOwner().getName(); for (AclEntry ae : view.getAcl()) { String principalName = ae.principal().getName(); - if (WIN_ROOT_PRINCIPALS.contains(principalName) || owner.equals(principalName) || principalInWinSIDS(ae.principal())) { + if (principalInWinSIDS(ae.principal())) { LOG.debug("Allowing permissions on restricted file {} for principal {} : {} ", tempFile.getAbsolutePath(), principalName, getSIDForPrincipal(ae.principal())); list.add(AclEntry.newBuilder() .setType(AclEntryType.ALLOW) @@ -140,7 +138,14 @@ private static void createRestrictedFile(File file, boolean isDir) throws IOExce .build()); } } - + // Add permissions for the owner + LOG.debug("Allowing permissions on restricted file {} for principal {} : {} ", tempFile.getAbsolutePath(), view.getOwner().getName(), getSIDForPrincipal(view.getOwner())); + list.add(AclEntry.newBuilder() + .setType(AclEntryType.ALLOW) + .setPrincipal(view.getOwner()) + .setPermissions(permissions) + .setFlags(flags) + .build()); // apply ACL view.setAcl(list); } else { @@ -202,13 +207,26 @@ public static boolean principalInWinSIDS(Principal principal) { public static String getSIDForPrincipal(Principal principal) { try { - Class clazz = principal.getClass().getSuperclass(); - Method method = clazz.getDeclaredMethod("sidString"); - method.setAccessible(true); - return (String) method.invoke(principal); + Method method = findMethod(principal.getClass(), "sidString"); + if (method != null) { + method.setAccessible(true); + return (String) method.invoke(principal); + } } catch (Exception e) { LOG.debug("No SID for {}", principal.getName()); - return ""; } + return ""; + } + + private static Method findMethod(Class clazz, String methodName) { + while (clazz != null) { + try { + Method method = clazz.getDeclaredMethod(methodName); + return method; + } catch (NoSuchMethodException e) { + clazz = clazz.getSuperclass(); + } + } + return null; } } diff --git a/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java b/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java index 552714ea1..50a6e7490 100644 --- a/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java +++ b/core/src/test/java/net/sourceforge/jnlp/util/RestrictedFileUtilsTest.java @@ -9,7 +9,6 @@ import java.nio.file.attribute.AclEntry; import java.nio.file.attribute.AclFileAttributeView; -import static net.sourceforge.jnlp.util.RestrictedFileUtils.getSIDForPrincipal; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -33,7 +32,7 @@ public void testCreateRestrictedFile() throws Exception { boolean hasOwner = false; AclFileAttributeView view = Files.getFileAttributeView(testfile.toPath(), AclFileAttributeView.class); for (AclEntry ae : view.getAcl()) { - if (view.getOwner().getName().equals(ae.principal().getName()) || getSIDForPrincipal(ae.principal()).equals(RestrictedFileUtils.NT_AUTHENTICATED_USER_SID)) { + if (view.getOwner().getName().equals(ae.principal().getName())) { assertFalse("Duplicate owner entry", hasOwner); hasOwner = true; assertEquals("Owner must have all permissions", 14, ae.permissions().size()); From 9b83096d0bc0d60aab85349b6c26798202323d5e Mon Sep 17 00:00:00 2001 From: janakmulani Date: Tue, 4 Feb 2025 19:21:48 +0530 Subject: [PATCH 3/3] OWS-644, ITW 955 : add owner ACLs to the restricted file --- .../java/net/sourceforge/jnlp/util/RestrictedFileUtils.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java b/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java index 70ab11de7..9b9ad1e13 100644 --- a/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java +++ b/core/src/main/java/net/sourceforge/jnlp/util/RestrictedFileUtils.java @@ -127,9 +127,8 @@ private static void createRestrictedFile(File file, boolean isDir) throws IOExce AclFileAttributeView view = Files.getFileAttributeView(tempFile.toPath(), AclFileAttributeView.class); List list = new ArrayList<>(); for (AclEntry ae : view.getAcl()) { - String principalName = ae.principal().getName(); if (principalInWinSIDS(ae.principal())) { - LOG.debug("Allowing permissions on restricted file {} for principal {} : {} ", tempFile.getAbsolutePath(), principalName, getSIDForPrincipal(ae.principal())); + LOG.debug("Allowing permissions on restricted file {} for principal {} : {} ", tempFile.getAbsolutePath(), ae.principal().getName(), getSIDForPrincipal(ae.principal())); list.add(AclEntry.newBuilder() .setType(AclEntryType.ALLOW) .setPrincipal(ae.principal())