This is an automated email from the ASF dual-hosted git repository.
vavrtom pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git
The following commit(s) were added to refs/heads/main by this push:
new f227730daa QPID-8753: [Broker-J] Clean up protected test key files
(#425)
f227730daa is described below
commit f227730daae61e055259497af91324bf39626260
Author: Daniil Kirilyuk <[email protected]>
AuthorDate: Fri Aug 28 10:41:07 2026 +0200
QPID-8753: [Broker-J] Clean up protected test key files (#425)
---
.../encryption/AESGCMKeyFileEncrypterTest.java | 22 ++--
.../AbstractAESKeyFileEncrypterFactoryTest.java | 7 +-
.../qpid/server/store/BrokerRecovererTest.java | 105 +++++--------------
.../org/apache/qpid/test/utils/TestFileUtils.java | 114 ++++++++++++++++++++-
.../apache/qpid/test/utils/TestFileUtilsTest.java | 113 ++++++++++++++++++++
5 files changed, 268 insertions(+), 93 deletions(-)
diff --git
a/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESGCMKeyFileEncrypterTest.java
b/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESGCMKeyFileEncrypterTest.java
index 9163bdb8e7..a72ea00975 100644
---
a/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESGCMKeyFileEncrypterTest.java
+++
b/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESGCMKeyFileEncrypterTest.java
@@ -63,7 +63,7 @@ import org.apache.qpid.server.model.ConfiguredObject;
import org.apache.qpid.server.model.JsonSystemConfigImpl;
import org.apache.qpid.server.model.SystemConfig;
import org.apache.qpid.server.model.User;
-import org.apache.qpid.server.util.FileUtils;
+import org.apache.qpid.test.utils.TestFileUtils;
import org.apache.qpid.test.utils.UnitTestBase;
public class AESGCMKeyFileEncrypterTest extends UnitTestBase
@@ -91,13 +91,19 @@ public class AESGCMKeyFileEncrypterTest extends UnitTestBase
@AfterEach
public void tearDown() throws Exception
{
- if (_systemLauncher != null)
+ try
{
- _systemLauncher.shutdown();
+ if (_systemLauncher != null)
+ {
+ _systemLauncher.shutdown();
+ }
}
- if (_workDir != null)
+ finally
{
- FileUtils.deleteDirectory(_workDir.toFile().getAbsolutePath());
+ if (_workDir != null)
+ {
+ TestFileUtils.deleteRecursively(_workDir);
+ }
}
}
@@ -205,14 +211,14 @@ public class AESGCMKeyFileEncrypterTest extends
UnitTestBase
@Test
public void testSetKeyLocationAsExpression() throws Exception
{
- final Path workDir = Files.createTempDirectory("qpid_work_dir");
- final File keyFile = new File(workDir.toFile(), "test.key");
+ _workDir = Files.createTempDirectory("qpid_work_dir");
+ final File keyFile = new File(_workDir.toFile(), "test.key");
AbstractAESKeyFileEncrypterFactory.createAndPopulateKeyFile(keyFile);
final Map<String, String> context = Map.of(
AbstractAESKeyFileEncrypterFactory.ENCRYPTER_KEY_FILE,
"${qpid.work_dir}" + File.separator + keyFile.getName());
createBrokerAndAuthenticationProviderWithEncrypterPassword(AESGCMKeyFileEncrypterFactory.TYPE,
- workDir,
+ _workDir,
context);
final String encryptedPassword = getEncryptedPasswordFromConfig();
final SecretKeySpec aesSecretKey = new
SecretKeySpec(Files.readAllBytes(keyFile.toPath()), "AES");
diff --git
a/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AbstractAESKeyFileEncrypterFactoryTest.java
b/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AbstractAESKeyFileEncrypterFactoryTest.java
index b03585ed8a..e21fb75668 100644
---
a/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AbstractAESKeyFileEncrypterFactoryTest.java
+++
b/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AbstractAESKeyFileEncrypterFactoryTest.java
@@ -43,11 +43,9 @@ import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.security.NoSuchAlgorithmException;
-import java.util.Comparator;
import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
-import java.util.stream.Stream;
import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;
@@ -62,6 +60,7 @@ import org.mockito.stubbing.Answer;
import org.apache.qpid.server.configuration.IllegalConfigurationException;
import org.apache.qpid.server.model.Broker;
import org.apache.qpid.server.model.SystemConfig;
+import org.apache.qpid.test.utils.TestFileUtils;
import org.apache.qpid.test.utils.UnitTestBase;
@SuppressWarnings({"rawtypes", "unchecked"})
@@ -240,9 +239,9 @@ public class AbstractAESKeyFileEncrypterFactoryTest extends
UnitTestBase
@AfterEach
public void tearDown() throws Exception
{
- try (final Stream<Path> stream = Files.walk(_tmpDir))
+ if (_tmpDir != null)
{
-
stream.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
+ TestFileUtils.deleteRecursively(_tmpDir);
}
}
diff --git
a/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
b/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
index 622763131f..8009bf54a0 100644
---
a/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
+++
b/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
@@ -25,36 +25,19 @@ import static
org.junit.jupiter.api.Assertions.assertNotNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
-import java.io.File;
-import java.io.IOException;
import java.lang.reflect.Method;
-import java.nio.file.FileSystems;
-import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.SimpleFileVisitor;
-import java.nio.file.attribute.BasicFileAttributes;
-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.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.UserPrincipal;
-import java.util.ArrayList;
import java.util.Arrays;
-import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
import java.util.stream.Collectors;
-import java.util.stream.Stream;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor;
import org.apache.qpid.server.configuration.updater.TaskExecutor;
import org.apache.qpid.server.logging.EventLogger;
@@ -70,6 +53,7 @@ import org.apache.qpid.server.model.SystemConfig;
import
org.apache.qpid.server.security.auth.manager.SimpleLDAPAuthenticationManager;
import
org.apache.qpid.server.security.encryption.AESGCMKeyFileEncrypterFactory;
import org.apache.qpid.server.security.encryption.ConfigurationSecretEncrypter;
+import org.apache.qpid.test.utils.TestFileUtils;
import org.apache.qpid.test.utils.UnitTestBase;
public class BrokerRecovererTest extends UnitTestBase
@@ -80,17 +64,23 @@ public class BrokerRecovererTest extends UnitTestBase
private SystemConfig<?> _systemConfig;
private TaskExecutor _taskExecutor;
+ private Path _workDir;
@BeforeEach
public void setUp() throws Exception
{
+ cleanUp();
+ _workDir = Files.createTempDirectory(getTestName());
_taskExecutor = CurrentThreadTaskExecutor.newStartedInstance();
- _systemConfig = new JsonSystemConfigImpl(_taskExecutor,
mock(EventLogger.class),null, Map.of())
+ _systemConfig = new JsonSystemConfigImpl(_taskExecutor,
mock(EventLogger.class), null, Map.of())
{
{
updateModel(BrokerModel.getInstance());
}
};
+ _systemConfig.setContextVariable(SystemConfig.QPID_WORK_DIR,
_workDir.toString());
+ assertEquals(_workDir.toString(),
_systemConfig.getContextValue(String.class, SystemConfig.QPID_WORK_DIR),
+ "Unexpected test work directory");
when(_brokerEntry.getId()).thenReturn(_brokerId);
when(_brokerEntry.getType()).thenReturn(Broker.class.getSimpleName());
@@ -111,42 +101,7 @@ public class BrokerRecovererTest extends UnitTestBase
@AfterEach
public void tearDown() throws Exception
{
- _taskExecutor.stop();
- final Path path = Path.of(_systemConfig.getContextValue(String.class,
SystemConfig.QPID_WORK_DIR));
- if (path.toFile().exists())
- {
- try
- {
- Files.walkFileTree(path, new SimpleFileVisitor<>()
- {
- @Override
- public FileVisitResult visitFile(final Path file, final
BasicFileAttributes attrs) throws IOException
- {
- makeFileDeletable(file.toFile());
- Files.deleteIfExists(file);
- return FileVisitResult.CONTINUE;
- }
-
- @Override
- public FileVisitResult postVisitDirectory(final Path dir,
final IOException exc) throws IOException
- {
- makeFileDeletable(dir.toFile());
- Files.deleteIfExists(dir);
- return FileVisitResult.CONTINUE;
- }
-
- @Override
- public FileVisitResult visitFileFailed(final Path file,
final IOException exc)
- {
- return FileVisitResult.CONTINUE;
- }
- });
- }
- catch (IOException e)
- {
- // ignore cleanup issues in tests
- }
- }
+ cleanUp();
}
@Test
@@ -387,38 +342,30 @@ public class BrokerRecovererTest extends UnitTestBase
recoverer.recover(Arrays.asList(records), false);
}
- private void makeFileDeletable(File file)
+ private void cleanUp() throws Exception
{
try
{
- if (Files.getFileAttributeView(file.toPath(),
PosixFileAttributeView.class) != null)
+ if (_taskExecutor != null)
{
- Files.setPosixFilePermissions(file.toPath(),
EnumSet.of(PosixFilePermission.OTHERS_WRITE));
- }
- else if (Files.getFileAttributeView(file.toPath(),
AclFileAttributeView.class) != null)
- {
- file.setWritable(true);
- final AclFileAttributeView attributeView =
- Files.getFileAttributeView(file.toPath(),
AclFileAttributeView.class);
- final ArrayList<AclEntry> acls = new
ArrayList<>(attributeView.getAcl());
-
- final AclEntry.Builder builder = AclEntry.newBuilder();
- final UserPrincipal everyone =
FileSystems.getDefault().getUserPrincipalLookupService()
- .lookupPrincipalByName("Everyone");
- builder.setPrincipal(everyone);
- builder.setType(AclEntryType.ALLOW);
-
builder.setPermissions(Stream.of(AclEntryPermission.values()).collect(Collectors.toSet()));
- acls.add(builder.build());
- attributeView.setAcl(acls);
- }
- else
- {
- throw new IllegalConfigurationException("Failed to change file
permissions");
+ _taskExecutor.stop();
}
}
- catch (IOException e)
+ finally
{
- throw new IllegalConfigurationException("Failed to change file
permissions", e);
+ _taskExecutor = null;
+ _systemConfig = null;
+ if (_workDir != null)
+ {
+ try
+ {
+ TestFileUtils.deleteRecursively(_workDir);
+ }
+ finally
+ {
+ _workDir = null;
+ }
+ }
}
}
}
diff --git
a/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestFileUtils.java
b/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestFileUtils.java
index 24685d2361..f33ebf136b 100644
---
a/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestFileUtils.java
+++
b/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestFileUtils.java
@@ -21,11 +21,28 @@
package org.apache.qpid.test.utils;
import java.io.File;
+import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-
-import java.io.FileOutputStream;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+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.DosFileAttributeView;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.UserPrincipal;
+import java.util.ArrayList;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.ListIterator;
+import java.util.Set;
import org.junit.jupiter.api.TestInfo;
@@ -241,6 +258,99 @@ public class TestFileUtils
return file.delete();
}
+ /**
+ * Recursively deletes a test file tree whose owner permissions may have
been restricted.
+ * Symbolic links are deleted without following them.
+ *
+ * @param path root of the test file tree
+ * @throws IOException if owner permissions cannot be restored or a file
cannot be deleted
+ */
+ public static void deleteRecursively(final Path path) throws IOException
+ {
+ if (Files.isSymbolicLink(path))
+ {
+ Files.deleteIfExists(path);
+ return;
+ }
+
+ try
+ {
+ restoreOwnerPermissions(path);
+ }
+ catch (NoSuchFileException e)
+ {
+ return;
+ }
+
+ if (Files.isDirectory(path, LinkOption.NOFOLLOW_LINKS))
+ {
+ try (final DirectoryStream<Path> children =
Files.newDirectoryStream(path))
+ {
+ for (final Path child : children)
+ {
+ deleteRecursively(child);
+ }
+ }
+ }
+ Files.deleteIfExists(path);
+ }
+
+ private static void restoreOwnerPermissions(final Path path) throws
IOException
+ {
+ final PosixFileAttributeView posixView =
Files.getFileAttributeView(path, PosixFileAttributeView.class,
+ LinkOption.NOFOLLOW_LINKS);
+
+ if (posixView != null)
+ {
+ final Set<PosixFilePermission> permissions =
EnumSet.noneOf(PosixFilePermission.class);
+ permissions.addAll(posixView.readAttributes().permissions());
+ permissions.add(PosixFilePermission.OWNER_READ);
+ permissions.add(PosixFilePermission.OWNER_WRITE);
+ permissions.add(PosixFilePermission.OWNER_EXECUTE);
+ posixView.setPermissions(permissions);
+ }
+ else
+ {
+ final AclFileAttributeView aclView =
Files.getFileAttributeView(path, AclFileAttributeView.class,
+ LinkOption.NOFOLLOW_LINKS);
+ if (aclView != null)
+ {
+ final UserPrincipal owner = Files.getOwner(path,
LinkOption.NOFOLLOW_LINKS);
+ final List<AclEntry> acl = new ArrayList<>(aclView.getAcl());
+ final ListIterator<AclEntry> iterator = acl.listIterator();
+ boolean ownerEntryFound = false;
+ while (iterator.hasNext())
+ {
+ final AclEntry entry = iterator.next();
+ if (entry.type() == AclEntryType.ALLOW &&
owner.equals(entry.principal()))
+ {
+ ownerEntryFound = true;
+ iterator.set(AclEntry.newBuilder(entry)
+
.setPermissions(EnumSet.allOf(AclEntryPermission.class))
+ .build());
+ }
+ }
+ if (!ownerEntryFound)
+ {
+ acl.add(AclEntry.newBuilder()
+ .setType(AclEntryType.ALLOW)
+ .setPrincipal(owner)
+
.setPermissions(EnumSet.allOf(AclEntryPermission.class))
+ .build());
+ }
+ aclView.setAcl(acl);
+ }
+ }
+
+ final DosFileAttributeView dosView = Files.getFileAttributeView(path,
DosFileAttributeView.class,
+ LinkOption.NOFOLLOW_LINKS);
+
+ if (dosView != null)
+ {
+ dosView.setReadOnly(false);
+ }
+ }
+
/**
* Copies the specified InputStream to the specified destination file. If
the destination file does not exist,
* it is created.
diff --git
a/qpid-test-utils/src/test/java/org/apache/qpid/test/utils/TestFileUtilsTest.java
b/qpid-test-utils/src/test/java/org/apache/qpid/test/utils/TestFileUtilsTest.java
new file mode 100644
index 0000000000..8836146264
--- /dev/null
+++
b/qpid-test-utils/src/test/java/org/apache/qpid/test/utils/TestFileUtilsTest.java
@@ -0,0 +1,113 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.qpid.test.utils;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+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.DosFileAttributeView;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.UserPrincipal;
+import java.util.EnumSet;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestFileUtilsTest
+{
+ @TempDir
+ private Path _tempDirectory;
+
+ @Test
+ public void testDeleteRecursivelyRestoresRestrictedOwnerPermissions()
throws Exception
+ {
+ final Path root =
Files.createDirectory(_tempDirectory.resolve("restricted"));
+ final Path child = Files.createDirectory(root.resolve("child"));
+ final Path file = Files.writeString(child.resolve("key"), "secret");
+
+ restrictFile(file);
+ restrictDirectory(child);
+ restrictDirectory(root);
+
+ TestFileUtils.deleteRecursively(root);
+
+ assertFalse(Files.exists(root), "Restricted test directory was not
deleted");
+ }
+
+ private void restrictFile(final Path file) throws Exception
+ {
+ final DosFileAttributeView dosView = Files.getFileAttributeView(file,
DosFileAttributeView.class);
+ if (dosView != null)
+ {
+ dosView.setReadOnly(true);
+ }
+
+ final PosixFileAttributeView posixView =
Files.getFileAttributeView(file, PosixFileAttributeView.class);
+ if (posixView != null)
+ {
+
posixView.setPermissions(EnumSet.of(PosixFilePermission.OWNER_READ));
+ }
+ else
+ {
+ final AclFileAttributeView aclView =
Files.getFileAttributeView(file, AclFileAttributeView.class);
+ if (aclView != null)
+ {
+ final UserPrincipal owner = Files.getOwner(file);
+ aclView.setAcl(List.of(AclEntry.newBuilder()
+ .setType(AclEntryType.ALLOW)
+ .setPrincipal(owner)
+ .setPermissions(AclEntryPermission.READ_DATA,
AclEntryPermission.READ_ATTRIBUTES,
+ AclEntryPermission.READ_ACL,
AclEntryPermission.SYNCHRONIZE)
+ .build()));
+ }
+ }
+ }
+
+ private void restrictDirectory(final Path directory) throws Exception
+ {
+ final PosixFileAttributeView posixView =
Files.getFileAttributeView(directory, PosixFileAttributeView.class);
+ if (posixView != null)
+ {
+
posixView.setPermissions(EnumSet.of(PosixFilePermission.OWNER_READ,
PosixFilePermission.OWNER_EXECUTE));
+ }
+ else
+ {
+ final AclFileAttributeView aclView =
Files.getFileAttributeView(directory, AclFileAttributeView.class);
+ if (aclView != null)
+ {
+ final UserPrincipal owner = Files.getOwner(directory);
+ aclView.setAcl(List.of(AclEntry.newBuilder()
+ .setType(AclEntryType.ALLOW)
+ .setPrincipal(owner)
+ .setPermissions(AclEntryPermission.ADD_FILE,
AclEntryPermission.ADD_SUBDIRECTORY,
+ AclEntryPermission.LIST_DIRECTORY)
+ .build()));
+ }
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
