# HG changeset patch # User Jiri Vanek # Date 1538414331 -7200 # Node ID 2cce11acb592f42bb3edbcdd50d3a780ecf3f591 # Parent 2cf2e3479d57d0d12c4d03e203e21205adf966de Support creating cache files with restricted access on windows * netx/net/sourceforge/jnlp/util/FileUtils.java: set proper ACLs for cache files and directories * tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java: added testCreateRestrictedFile test that checks ACLs for cache file diff -r 2cf2e3479d57 -r 2cce11acb592 .hgignore --- a/.hgignore Mon Oct 01 19:16:32 2018 +0200 +++ b/.hgignore Mon Oct 01 19:18:51 2018 +0200 @@ -14,4 +14,4 @@ netx-dist-tests-whitelist rust-launcher/target rust-launcher/Cargo.lock - +rust-launcher/.idea diff -r 2cf2e3479d57 -r 2cce11acb592 ChangeLog --- a/ChangeLog Mon Oct 01 19:16:32 2018 +0200 +++ b/ChangeLog Mon Oct 01 19:18:51 2018 +0200 @@ -157,6 +157,12 @@ * NEWS: set date and added content for 1.7.1 * configure.ac: (AC_INIT) set to use 1.7.1 +2017-11-08 Alex Kashchenko + + Support creating cache files with restricted access on windows + * netx/net/sourceforge/jnlp/util/FileUtils.java: set proper ACLs for cache files and directories + * tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java: added testCreateRestrictedFile test that checks ACLs for cache file + 2017-15-12 Jiri Vanek Tomáš Votava diff -r 2cf2e3479d57 -r 2cce11acb592 netx/net/sourceforge/jnlp/util/FileUtils.java --- a/netx/net/sourceforge/jnlp/util/FileUtils.java Mon Oct 01 19:16:32 2018 +0200 +++ b/netx/net/sourceforge/jnlp/util/FileUtils.java Mon Oct 01 19:18:51 2018 +0200 @@ -64,6 +64,8 @@ private static final String WIN_DRIVE_LETTER_COLON_WILDCHAR = "WINDOWS_VERY_SPECIFIC_DOUBLEDOT"; + private static final List 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 @@ -246,37 +248,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 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 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 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)) { @@ -303,14 +320,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 2cf2e3479d57 -r 2cce11acb592 tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java --- a/tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java Mon Oct 01 19:16:32 2018 +0200 +++ b/tests/netx/unit/net/sourceforge/jnlp/util/FileUtilsTest.java Mon Oct 01 19:18:51 2018 +0200 @@ -133,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); + } + }