From 6f5d3fab4cc0b59e594ed0ac685561a1a535abd0 Mon Sep 17 00:00:00 2001 From: Emmanuel Hugonnet Date: Wed, 18 Dec 2024 09:09:55 +0100 Subject: [PATCH] [WFCORE-7102]: AccessDeniedException on Windows when using a read-only configuration dir. * Replacing java.io.File.canWrite() by java.nio.file.Files.isWritable(Path) in: - ConfigurationFile - FilePersistenceUtils - ConfigurationFilePersistenceResource * Enabling the test for Windows and chaning the temp folder creation Jira: https://issues.redhat.com/browse/WFCORE-7102 Signed-off-by: Emmanuel Hugonnet --- .../persistence/ConfigurationFile.java | 2 +- .../ConfigurationFilePersistenceResource.java | 3 +- .../persistence/FilePersistenceUtils.java | 2 +- .../ServerProcessReloadHandler.java | 1 + .../management/ReadOnlyModeTestCase.java | 76 ++++++++++++++----- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFile.java b/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFile.java index 95ac5d8d7bc..18ba4f41ad7 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFile.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFile.java @@ -185,7 +185,7 @@ public ConfigurationFile(final File configurationDir, final String rawName, fina this.configurationExtension = configurationExtension; this.interactionPolicy = interactionPolicy == null ? InteractionPolicy.STANDARD : interactionPolicy; // If we are in a read only policy and the configurationDir cannot be written, then we use the tmpDir for temporal files and history - this.historyRoot = new File(tmpDir != null && this.interactionPolicy.isReadOnly() && !configurationDir.canWrite() ? tmpDir : configurationDir, + this.historyRoot = new File(tmpDir != null && this.interactionPolicy.isReadOnly() && !Files.isWritable(configurationDir.toPath())? tmpDir : configurationDir, rawName.replace('.', '_') + "_history"); this.currentHistory = new File(historyRoot, "current"); this.snapshotsDirectory = new File(historyRoot, "snapshot"); diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFilePersistenceResource.java b/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFilePersistenceResource.java index e4750fef697..833969ce666 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFilePersistenceResource.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationFilePersistenceResource.java @@ -9,6 +9,7 @@ import java.io.File; import java.io.InputStream; +import java.nio.file.Files; import org.jboss.dmr.ModelNode; @@ -37,7 +38,7 @@ protected void doCommit(InputStream in) { if ( FilePersistenceUtils.isParentFolderWritable(fileName) ){ tempFileName = FilePersistenceUtils.createTempFile(fileName); - } else if (configurationFile.getConfigurationDir().canWrite()) { + } else if (Files.isWritable(configurationFile.getConfigurationDir().toPath())) { tempFileName = FilePersistenceUtils.createTempFile(configurationFile.getConfigurationDir(), fileName.getName()); } else { tempFileName = FilePersistenceUtils.createTempFile(configurationFile.getConfigurationTmpDir(), fileName.getName()); diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/FilePersistenceUtils.java b/controller/src/main/java/org/jboss/as/controller/persistence/FilePersistenceUtils.java index 3c2607c252a..61dbaa96e31 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/FilePersistenceUtils.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/FilePersistenceUtils.java @@ -178,6 +178,6 @@ static boolean isParentFolderWritable(File file){ if ( !file.exists() || file.getParentFile() == null ){ return false; } - return file.getParentFile().canWrite(); + return Files.isWritable(file.getParentFile().toPath()); } } diff --git a/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java b/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java index fa4a64d4c81..b52dbf94e80 100644 --- a/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java +++ b/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java @@ -159,6 +159,7 @@ protected ProcessReloadHandler.ReloadContext initializeReloa if (serverConfig != null && !environment.getServerConfigurationFile().checkCanFindNewBootFile(serverConfig)) { throw ServerLogger.ROOT_LOGGER.serverConfigForReloadNotFound(serverConfig); } + ServerLogger.ROOT_LOGGER.warn("Reloading with config file " + serverConfig); return new ReloadContext() { @Override diff --git a/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/ReadOnlyModeTestCase.java b/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/ReadOnlyModeTestCase.java index 1b6f2460f97..ba76d620b71 100644 --- a/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/ReadOnlyModeTestCase.java +++ b/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/ReadOnlyModeTestCase.java @@ -17,6 +17,14 @@ import java.util.Set; import jakarta.inject.Inject; +import java.io.File; +import java.nio.file.attribute.AclEntry; +import java.nio.file.attribute.AclEntryPermission; +import java.nio.file.attribute.AclEntryType; +import java.nio.file.attribute.AclFileAttributeView; +import java.nio.file.attribute.FileOwnerAttributeView; +import java.nio.file.attribute.UserPrincipal; +import java.util.List; import org.jboss.as.controller.PathAddress; import org.jboss.as.controller.PathElement; @@ -28,7 +36,6 @@ import org.jboss.dmr.ModelNode; import org.junit.After; import org.junit.Assert; -import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; import org.wildfly.core.testrunner.ServerControl; @@ -69,29 +76,37 @@ public void testConfigurationNotUpdated() throws Exception { @Test public void testReadOnlyConfigurationDirectory() throws Exception { - // We ignore the test on Windows to prevent in case of errors the pollution of %TMPDIR% with read only directories - // On unix machines, the /tmp dir is always deleted on each server boot by root user. - Assume.assumeFalse(TestSuiteEnvironment.isWindows()); - final Path jbossHome = Paths.get(System.getProperty("jboss.home")); final Path configDir = jbossHome.resolve("standalone").resolve("configuration"); final Path standaloneTmpDir = jbossHome.resolve("standalone").resolve("tmp"); - final Path osTmpDir = Paths.get("/tmp"); + final Path osTmpDir = TestSuiteEnvironment.isWindows() ? new File("target", "tmp").toPath().toAbsolutePath() : Paths.get("/tmp"); + if(Files.notExists(osTmpDir)) { + Files.createDirectories(osTmpDir); + } final Path roConfigDir = Files.createTempDirectory(osTmpDir, "wildfly-test-suite-"); - PathUtil.copyRecursively(configDir, roConfigDir, true); - Set perms = new HashSet<>(); - - perms.add(PosixFilePermission.OWNER_READ); - perms.add(PosixFilePermission.OWNER_EXECUTE); - perms.add(PosixFilePermission.GROUP_READ); - perms.add(PosixFilePermission.GROUP_EXECUTE); - perms.add(PosixFilePermission.OTHERS_READ); - perms.add(PosixFilePermission.OTHERS_EXECUTE); - - Files.getFileAttributeView(roConfigDir, PosixFileAttributeView.class).setPermissions(perms); - + if (TestSuiteEnvironment.isWindows()) { + UserPrincipal owner = Files.getFileAttributeView(configDir, FileOwnerAttributeView.class).getOwner(); + AclEntry entry = AclEntry.newBuilder() + .setPrincipal(owner) + .setType(AclEntryType.DENY) + .setPermissions(AclEntryPermission.WRITE_DATA, AclEntryPermission.APPEND_DATA) + .build(); + AclFileAttributeView view = Files.getFileAttributeView(roConfigDir, AclFileAttributeView.class); + List acl = view.getAcl(); + acl.add(0, entry); + view.setAcl(acl); + } else { + Set perms = new HashSet<>(); + perms.add(PosixFilePermission.OWNER_READ); + perms.add(PosixFilePermission.OWNER_EXECUTE); + perms.add(PosixFilePermission.GROUP_READ); + perms.add(PosixFilePermission.GROUP_EXECUTE); + perms.add(PosixFilePermission.OTHERS_READ); + perms.add(PosixFilePermission.OTHERS_EXECUTE); + Files.getFileAttributeView(roConfigDir, PosixFileAttributeView.class).setPermissions(perms); + } assertFalse(roConfigDir.toString() + " is writeable", Files.isWritable(roConfigDir)); try { @@ -120,13 +135,34 @@ public void testReadOnlyConfigurationDirectory() throws Exception { } } finally { + if (TestSuiteEnvironment.isWindows()) { + UserPrincipal owner = Files.getFileAttributeView(configDir, FileOwnerAttributeView.class).getOwner(); + AclEntry entry = AclEntry.newBuilder() + .setPrincipal(owner) + .setType(AclEntryType.ALLOW) + .setPermissions(AclEntryPermission.WRITE_DATA, AclEntryPermission.APPEND_DATA) + .build(); + AclFileAttributeView view = Files.getFileAttributeView(roConfigDir, AclFileAttributeView.class); + List acl = view.getAcl(); + acl.add(0, entry); + view.setAcl(acl); + } else { + Set perms = new HashSet<>(); perms.add(PosixFilePermission.OWNER_WRITE); perms.add(PosixFilePermission.GROUP_WRITE); perms.add(PosixFilePermission.OTHERS_WRITE); - + perms.add(PosixFilePermission.OWNER_READ); + perms.add(PosixFilePermission.OWNER_EXECUTE); + perms.add(PosixFilePermission.GROUP_READ); + perms.add(PosixFilePermission.GROUP_EXECUTE); + perms.add(PosixFilePermission.OTHERS_READ); + perms.add(PosixFilePermission.OTHERS_EXECUTE); Files.getFileAttributeView(roConfigDir, PosixFileAttributeView.class).setPermissions(perms); - + } PathUtil.deleteRecursively(roConfigDir); + if( TestSuiteEnvironment.isWindows()) { + PathUtil.deleteRecursively(osTmpDir); + } } }