/hg/icedtea-web: support creating cache files with restricted ac...

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

/hg/icedtea-web: support creating cache files with restricted ac...

akashche
changeset c234bad6f599 in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=c234bad6f599
author: Alex Kashchenko <[hidden email]>
date: Wed Nov 08 11:37:01 2017 +0000

        support creating cache files with restricted access on windows


diffstat:

 netx/net/sourceforge/jnlp/util/FileUtils.java                |  80 +++++++----
 tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java |  26 +++
 2 files changed, 74 insertions(+), 32 deletions(-)

diffs (166 lines):

diff -r a7dc403313cb -r c234bad6f599 netx/net/sourceforge/jnlp/util/FileUtils.java
--- a/netx/net/sourceforge/jnlp/util/FileUtils.java Fri Nov 03 12:39:31 2017 +0100
+++ b/netx/net/sourceforge/jnlp/util/FileUtils.java Wed Nov 08 11:37:01 2017 +0000
@@ -33,12 +33,12 @@
 import java.io.Writer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
+import java.nio.file.Files;
+import java.nio.file.attribute.*;
 import java.security.DigestInputStream;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+import java.util.*;
 
 import javax.swing.JFrame;
 import javax.swing.JOptionPane;
@@ -60,6 +60,8 @@
 
     private static final String WIN_DRIVE_LETTER_COLON_WILDCHAR = "WINDOWS_VERY_SPECIFIC_DOUBLEDOT";
 
+    private static final List<String> WIN_ROOT_PRINCIPALS = Arrays.asList("NT AUTHORITY\\SYSTEM", "BUILTIN\\Administrators");
+
     /**
      * Indicates whether a file was successfully opened. If not, provides specific reasons
      * along with a general failure case
@@ -242,37 +244,52 @@
         }
 
         if (JNLPRuntime.isWindows()) {
-            // remove all permissions
-            if (!tempFile.setExecutable(false, false)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RRemoveXPermFailed", tempFile));
-            }
-            if (!tempFile.setReadable(false, false)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RRemoveRPermFailed", tempFile));
-            }
-            if (!tempFile.setWritable(false, false)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RRemoveWPermFailed", tempFile));
-            }
-
-            // allow owner to read
-            if (!tempFile.setReadable(true, true)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RGetRPermFailed", tempFile));
+            // prepare ACL flags
+            Set<AclEntryFlag> flags = new LinkedHashSet<>();
+            if (tempFile.isDirectory()) {
+                flags.add(AclEntryFlag.DIRECTORY_INHERIT);
+                flags.add(AclEntryFlag.FILE_INHERIT);
             }
 
-            // allow owner to write
-            if (writableByOwner && !tempFile.setWritable(true, true)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RGetWPermFailed", tempFile));
+            // prepare ACL permissions
+            Set<AclEntryPermission> permissions = new LinkedHashSet<>();
+            permissions.addAll(Arrays.asList(
+                    AclEntryPermission.READ_DATA,
+                    AclEntryPermission.READ_NAMED_ATTRS,
+                    AclEntryPermission.EXECUTE,
+                    AclEntryPermission.READ_ATTRIBUTES,
+                    AclEntryPermission.READ_ACL,
+                    AclEntryPermission.SYNCHRONIZE));
+            if (writableByOwner) {
+                permissions.addAll(Arrays.asList(
+                        AclEntryPermission.WRITE_DATA,
+                        AclEntryPermission.APPEND_DATA,
+                        AclEntryPermission.WRITE_NAMED_ATTRS,
+                        AclEntryPermission.DELETE_CHILD,
+                        AclEntryPermission.WRITE_ATTRIBUTES,
+                        AclEntryPermission.DELETE,
+                        AclEntryPermission.WRITE_ACL,
+                        AclEntryPermission.WRITE_OWNER));
             }
 
-            // allow owner to enter directories
-            if (isDir && !tempFile.setExecutable(true, true)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RGetXPermFailed", tempFile));
+            // filter ACL's leaving only root and owner
+            AclFileAttributeView view = Files.getFileAttributeView(tempFile.toPath(), AclFileAttributeView.class);
+            List<AclEntry> 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)) {
+                    list.add(AclEntry.newBuilder()
+                            .setType(AclEntryType.ALLOW)
+                            .setPrincipal(ae.principal())
+                            .setPermissions(permissions)
+                            .setFlags(flags)
+                            .build());
+                }
             }
-            // rename this file. Unless the file is moved/renamed, any program that
-            // opened the file right after it was created might still be able to
-            // read the data.
-            if (!tempFile.renameTo(file)) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RCantRename", tempFile, file));
-            }
+
+            // apply ACL
+            view.setAcl(list);
         } else {
         // remove all permissions
         if (!tempFile.setExecutable(false, false)) {
@@ -299,15 +316,14 @@
         if (isDir && !tempFile.setExecutable(true, true)) {
             throw new IOException(R("RGetXPermFailed", tempFile));
         }
-        
+        }
+
         // rename this file. Unless the file is moved/renamed, any program that
         // opened the file right after it was created might still be able to
         // read the data.
         if (!tempFile.renameTo(file)) {
             throw new IOException(R("RCantRename", tempFile, file));
         }
-        }
-
     }
 
     /**
diff -r a7dc403313cb -r c234bad6f599 tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java
--- a/tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java Fri Nov 03 12:39:31 2017 +0100
+++ b/tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java Wed Nov 08 11:37:01 2017 +0000
@@ -37,6 +37,9 @@
 package net.sourceforge.jnlp.util;
 
 import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.attribute.AclEntry;
+import java.nio.file.attribute.AclFileAttributeView;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -130,4 +133,27 @@
         assertFalse(testChild.exists());
     }
 
+    @Test
+    public void testCreateRestrictedFile() throws Exception {
+        if (!JNLPRuntime.isWindows()) {
+            return;
+        }
+        final File tmpdir = new File(System.getProperty("java.io.tmpdir")), testfile = new File(tmpdir, "itw_test_create_restricted_file");
+        if (testfile.exists()) {
+            assertTrue(testfile.delete());
+        }
+        testfile.deleteOnExit();
+        FileUtils.createRestrictedFile(testfile, true);
+        boolean hasOwner = false;
+        AclFileAttributeView view = Files.getFileAttributeView(testfile.toPath(), AclFileAttributeView.class);
+        for (AclEntry ae : view.getAcl()) {
+            if (view.getOwner().getName().equals(ae.principal().getName())) {
+                assertFalse("Duplicate owner entry", hasOwner);
+                hasOwner = true;
+                assertEquals("Owner must have all perimissions",14, ae.permissions().size());
+            }
+        }
+        assertTrue("No owner entry", hasOwner);
+    }
+
 }
Reply | Threaded
Open this post in threaded view
|

Re: /hg/icedtea-web: support creating cache files with restricted ac...

Jiri Vanek
On 11/09/2017 05:18 PM, Jiri Vanek wrote:
  Please add missing changelog entry.

  TY for patch!
     J.

>
> On 11/08/2017 12:37 PM, [hidden email] wrote:
>> changeset c234bad6f599 in /hg/icedtea-web
>> details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=c234bad6f599
>> author: Alex Kashchenko <[hidden email]>
>> date: Wed Nov 08 11:37:01 2017 +0000
>>
>>     support creating cache files with restricted access on windows
>>
>>
>> diffstat:
>>
>>   netx/net/sourceforge/jnlp/util/FileUtils.java                |  80 +++++++----
>>   tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java |  26 +++
>>   2 files changed, 74 insertions(+), 32 deletions(-)
>>
>> diffs (166 lines):
>>
>> diff -r a7dc403313cb -r c234bad6f599 netx/net/sourceforge/jnlp/util/FileUtils.java
>> --- a/netx/net/sourceforge/jnlp/util/FileUtils.java    Fri Nov 03 12:39:31 2017 +0100
>> +++ b/netx/net/sourceforge/jnlp/util/FileUtils.java    Wed Nov 08 11:37:01 2017 +0000
>> @@ -33,12 +33,12 @@
>>   import java.io.Writer;
>>   import java.nio.channels.FileChannel;
>>   import java.nio.channels.FileLock;
>> +import java.nio.file.Files;
>> +import java.nio.file.attribute.*;
>>   import java.security.DigestInputStream;
>>   import java.security.MessageDigest;
>>   import java.security.NoSuchAlgorithmException;
>> -import java.util.ArrayList;
>> -import java.util.Arrays;
>> -import java.util.List;
>> +import java.util.*;
>>   import javax.swing.JFrame;
>>   import javax.swing.JOptionPane;
>> @@ -60,6 +60,8 @@
>>       private static final String WIN_DRIVE_LETTER_COLON_WILDCHAR =
>> "WINDOWS_VERY_SPECIFIC_DOUBLEDOT";
>> +    private static final List<String> WIN_ROOT_PRINCIPALS = Arrays.asList("NT AUTHORITY\\SYSTEM",
>> "BUILTIN\\Administrators");
>> +
>>       /**
>>        * Indicates whether a file was successfully opened. If not, provides specific reasons
>>        * along with a general failure case
>> @@ -242,37 +244,52 @@
>>           }
>>           if (JNLPRuntime.isWindows()) {
>> -            // remove all permissions
>> -            if (!tempFile.setExecutable(false, false)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RRemoveXPermFailed", tempFile));
>> -            }
>> -            if (!tempFile.setReadable(false, false)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RRemoveRPermFailed", tempFile));
>> -            }
>> -            if (!tempFile.setWritable(false, false)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RRemoveWPermFailed", tempFile));
>> -            }
>> -
>> -            // allow owner to read
>> -            if (!tempFile.setReadable(true, true)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RGetRPermFailed", tempFile));
>> +            // prepare ACL flags
>> +            Set<AclEntryFlag> flags = new LinkedHashSet<>();
>> +            if (tempFile.isDirectory()) {
>> +                flags.add(AclEntryFlag.DIRECTORY_INHERIT);
>> +                flags.add(AclEntryFlag.FILE_INHERIT);
>>               }
>> -            // allow owner to write
>> -            if (writableByOwner && !tempFile.setWritable(true, true)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RGetWPermFailed", tempFile));
>> +            // prepare ACL permissions
>> +            Set<AclEntryPermission> permissions = new LinkedHashSet<>();
>> +            permissions.addAll(Arrays.asList(
>> +                    AclEntryPermission.READ_DATA,
>> +                    AclEntryPermission.READ_NAMED_ATTRS,
>> +                    AclEntryPermission.EXECUTE,
>> +                    AclEntryPermission.READ_ATTRIBUTES,
>> +                    AclEntryPermission.READ_ACL,
>> +                    AclEntryPermission.SYNCHRONIZE));
>> +            if (writableByOwner) {
>> +                permissions.addAll(Arrays.asList(
>> +                        AclEntryPermission.WRITE_DATA,
>> +                        AclEntryPermission.APPEND_DATA,
>> +                        AclEntryPermission.WRITE_NAMED_ATTRS,
>> +                        AclEntryPermission.DELETE_CHILD,
>> +                        AclEntryPermission.WRITE_ATTRIBUTES,
>> +                        AclEntryPermission.DELETE,
>> +                        AclEntryPermission.WRITE_ACL,
>> +                        AclEntryPermission.WRITE_OWNER));
>>               }
>> -            // allow owner to enter directories
>> -            if (isDir && !tempFile.setExecutable(true, true)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RGetXPermFailed", tempFile));
>> +            // filter ACL's leaving only root and owner
>> +            AclFileAttributeView view = Files.getFileAttributeView(tempFile.toPath(),
>> AclFileAttributeView.class);
>> +            List<AclEntry> 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)) {
>> +                    list.add(AclEntry.newBuilder()
>> +                            .setType(AclEntryType.ALLOW)
>> +                            .setPrincipal(ae.principal())
>> +                            .setPermissions(permissions)
>> +                            .setFlags(flags)
>> +                            .build());
>> +                }
>>               }
>> -            // rename this file. Unless the file is moved/renamed, any program that
>> -            // opened the file right after it was created might still be able to
>> -            // read the data.
>> -            if (!tempFile.renameTo(file)) {
>> -                OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
>> R("RCantRename", tempFile, file));
>> -            }
>> +
>> +            // apply ACL
>> +            view.setAcl(list);
>>           } else {
>>           // remove all permissions
>>           if (!tempFile.setExecutable(false, false)) {
>> @@ -299,15 +316,14 @@
>>           if (isDir && !tempFile.setExecutable(true, true)) {
>>               throw new IOException(R("RGetXPermFailed", tempFile));
>>           }
>> -
>> +        }
>> +
>>           // rename this file. Unless the file is moved/renamed, any program that
>>           // opened the file right after it was created might still be able to
>>           // read the data.
>>           if (!tempFile.renameTo(file)) {
>>               throw new IOException(R("RCantRename", tempFile, file));
>>           }
>> -        }
>> -
>>       }
>>       /**
>> diff -r a7dc403313cb -r c234bad6f599 tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java
>> --- a/tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java    Fri Nov 03 12:39:31 2017 +0100
>> +++ b/tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java    Wed Nov 08 11:37:01 2017 +0000
>> @@ -37,6 +37,9 @@
>>   package net.sourceforge.jnlp.util;
>>   import java.io.File;
>> +import java.nio.file.Files;
>> +import java.nio.file.attribute.AclEntry;
>> +import java.nio.file.attribute.AclFileAttributeView;
>>   import java.util.ArrayList;
>>   import java.util.Arrays;
>>   import java.util.List;
>> @@ -130,4 +133,27 @@
>>           assertFalse(testChild.exists());
>>       }
>> +    @Test
>> +    public void testCreateRestrictedFile() throws Exception {
>> +        if (!JNLPRuntime.isWindows()) {
>> +            return;
>> +        }
>> +        final File tmpdir = new File(System.getProperty("java.io.tmpdir")), testfile = new
>> File(tmpdir, "itw_test_create_restricted_file");
>> +        if (testfile.exists()) {
>> +            assertTrue(testfile.delete());
>> +        }
>> +        testfile.deleteOnExit();
>> +        FileUtils.createRestrictedFile(testfile, true);
>> +        boolean hasOwner = false;
>> +        AclFileAttributeView view = Files.getFileAttributeView(testfile.toPath(),
>> AclFileAttributeView.class);
>> +        for (AclEntry ae : view.getAcl()) {
>> +            if (view.getOwner().getName().equals(ae.principal().getName())) {
>> +                assertFalse("Duplicate owner entry", hasOwner);
>> +                hasOwner = true;
>> +                assertEquals("Owner must have all perimissions",14, ae.permissions().size());
>> +            }
>> +        }
>> +        assertTrue("No owner entry", hasOwner);
>> +    }
>> +
>>   }
>>
>
>


--
Jiri Vanek
Senior QE engineer, OpenJDK QE lead, Mgr.
Red Hat Czech
[hidden email]    M: +420775390109