QPID-6247: Updates to configuration files should maintain existing file permissions

Merged from trunk with commands:

svn merge -c 1661162 https://svn.apache.org/repos/asf/qpid/trunk
svn merge -c 1661530  https://svn.apache.org/repos/asf/qpid/trunk




git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/0.32@1661931 13f79535-47bb-0310-9956-ffa450edef68

12 files changed, 454 insertions, 208 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/configuration/BrokerProperties.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/configuration/BrokerProperties.java
index 765e1e4fa5..d6dbe37a6b 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/configuration/BrokerProperties.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/configuration/BrokerProperties.java
@@ -48,6 +48,7 @@ public class BrokerProperties
     public static final String PROPERTY_QPID_HOME = "QPID_HOME";
     public static final String PROPERTY_QPID_WORK = "QPID_WORK";
     public static final String PROPERTY_LOG_RECORDS_BUFFER_SIZE = "qpid.broker_log_records_buffer_size";
+    public static final String POSIX_FILE_PERMISSIONS = "qpid.default_posix_file_permissions";
 
     private BrokerProperties()
     {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java
index a69808180d..c1a05a9e35 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/SystemConfig.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.model;
 
+import org.apache.qpid.server.configuration.BrokerProperties;
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogRecorder;
 import org.apache.qpid.server.store.DurableConfigurationStore;
@@ -37,6 +38,9 @@ public interface SystemConfig<X extends SystemConfig<X>> extends ConfiguredObjec
     String INITIAL_CONFIGURATION_LOCATION = "initialConfigurationLocation";
     String STARTUP_LOGGED_TO_SYSTEM_OUT = "startupLoggedToSystemOut";
 
+    @ManagedContextDefault(name = BrokerProperties.POSIX_FILE_PERMISSIONS)
+    String DEFAULT_POSIX_FILE_PERMISSIONS = "rw-r-----";
+
     @ManagedAttribute(defaultValue = "false")
     boolean isManagementMode();
 
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java
index 19aec414de..327b7ddfe9 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java
@@ -34,6 +34,7 @@ import java.util.UUID;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.configuration.BrokerProperties;
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.AbstractConfiguredObject;
 import org.apache.qpid.server.model.Broker;
@@ -50,6 +51,7 @@ import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.auth.UsernamePrincipal;
 import org.apache.qpid.server.security.group.FileGroupDatabase;
 import org.apache.qpid.server.security.group.GroupPrincipal;
+import org.apache.qpid.server.util.FileHelper;
 
 public class FileBasedGroupProviderImpl
         extends AbstractConfiguredObject<FileBasedGroupProviderImpl> implements FileBasedGroupProvider<FileBasedGroupProviderImpl>
@@ -162,9 +164,11 @@ public class FileBasedGroupProviderImpl
             {
                 throw new IllegalConfigurationException(String.format("Cannot create groups file at '%s'",_path));
             }
+
             try
             {
-                file.createNewFile();
+                String posixFileAttributes = getContextValue(String.class, BrokerProperties.POSIX_FILE_PERMISSIONS);
+                new FileHelper().createNewFile(file, posixFileAttributes);
             }
             catch (IOException e)
             {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java
index e3ded3006d..7046f2973e 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderImpl.java
@@ -21,14 +21,14 @@
 
 package org.apache.qpid.server.model.adapter;
 
-import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.RandomAccessFile;
-import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.nio.channels.OverlappingFileLockException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -38,6 +38,9 @@ import java.util.Set;
 import java.util.TreeMap;
 
 import org.apache.log4j.Logger;
+import org.apache.qpid.server.configuration.BrokerProperties;
+import org.apache.qpid.server.util.BaseAction;
+import org.apache.qpid.server.util.FileHelper;
 import org.codehaus.jackson.JsonParser;
 import org.codehaus.jackson.JsonProcessingException;
 import org.codehaus.jackson.map.ObjectMapper;
@@ -118,7 +121,7 @@ public class FileSystemPreferencesProviderImpl
         FileSystemPreferencesStore store = new FileSystemPreferencesStore(new File(_path));
 
         // we need to check and create file if it does not exist every time on open
-        store.createIfNotExist();
+        store.createIfNotExist(getContextValue(String.class, BrokerProperties.POSIX_FILE_PERMISSIONS));
         store.open();
         _store = store;
         _open = true;
@@ -184,6 +187,7 @@ public class FileSystemPreferencesProviderImpl
 
         if(_store != null)
         {
+            _store.close();
             _store.delete();
             deleted();
             _authenticationProvider.setPreferencesProvider(null);
@@ -280,7 +284,7 @@ public class FileSystemPreferencesProviderImpl
             else
             {
                 FileSystemPreferencesStore store = new FileSystemPreferencesStore(new File(_path));
-                store.createIfNotExist();
+                store.createIfNotExist(getContextValue(String.class, BrokerProperties.POSIX_FILE_PERMISSIONS));
                 store.open();
                 _store = store;
             }
@@ -334,9 +338,9 @@ public class FileSystemPreferencesProviderImpl
     {
         private final ObjectMapper _objectMapper;
         private final Map<String, Map<String, Object>> _preferences;
+        private final FileHelper _fileHelper;
         private File _storeFile;
         private FileLock _storeLock;
-        private RandomAccessFile _storeRAF;
 
         public FileSystemPreferencesStore(File preferencesFile)
         {
@@ -345,9 +349,10 @@ public class FileSystemPreferencesProviderImpl
             _objectMapper.configure(SerializationConfig.Feature.INDENT_OUTPUT, true);
             _objectMapper.configure(JsonParser.Feature.ALLOW_COMMENTS, true);
             _preferences = new TreeMap<String, Map<String, Object>>();
+            _fileHelper = new FileHelper();
         }
 
-        public void createIfNotExist()
+        public void createIfNotExist(String filePermissions)
         {
             if (!_storeFile.exists())
             {
@@ -358,7 +363,8 @@ public class FileSystemPreferencesProviderImpl
                 }
                 try
                 {
-                    if (_storeFile.createNewFile() && !_storeFile.exists())
+                    Path path = _fileHelper.createNewFile(_storeFile, filePermissions);
+                    if (!Files.exists(path))
                     {
                         throw new IllegalConfigurationException(String.format("Cannot create preferences store file at '%s'", _storeFile.getAbsolutePath()));
                     }
@@ -391,43 +397,20 @@ public class FileSystemPreferencesProviderImpl
             }
             try
             {
-                _storeRAF = new RandomAccessFile(_storeFile, "rw");
-                FileChannel fileChannel = _storeRAF.getChannel();
-                try
-                {
-                    _storeLock = fileChannel.tryLock();
-                }
-                catch (OverlappingFileLockException e)
-                {
-                    _storeLock = null;
-                }
-                if (_storeLock == null)
+                getFileLock(_storeFile.getPath() + ".lck");
+                if (_storeFile.length() > 0)
                 {
-                    throw new IllegalConfigurationException("Cannot get lock on store file " + _storeFile.getName()
-                            + " is another instance running?");
-                }
-                long fileSize = fileChannel.size();
-                if (fileSize > 0)
-                {
-                    ByteBuffer buffer = ByteBuffer.allocate((int) fileSize);
-                    fileChannel.read(buffer);
-                    buffer.rewind();
-                    buffer.flip();
-                    byte[] data = buffer.array();
-                    try
-                    {
-                        Map<String, Map<String, Object>> preferencesMap = _objectMapper.readValue(data,
-                                new TypeReference<Map<String, Map<String, Object>>>()
-                                {
-                                });
-                        _preferences.putAll(preferencesMap);
-                    }
-                    catch (JsonProcessingException e)
-                    {
-                        throw new IllegalConfigurationException("Cannot parse preferences json in " + _storeFile.getName(), e);
-                    }
+                    Map<String, Map<String, Object>> preferencesMap = _objectMapper.readValue(_storeFile,
+                            new TypeReference<Map<String, Map<String, Object>>>()
+                            {
+                            });
+                    _preferences.putAll(preferencesMap);
                 }
             }
+            catch (JsonProcessingException e)
+            {
+                throw new IllegalConfigurationException("Cannot parse preferences json in " + _storeFile.getName(), e);
+            }
             catch (IOException e)
             {
                 throw new IllegalConfigurationException("Cannot load preferences from " + _storeFile.getName(), e);
@@ -443,6 +426,7 @@ public class FileSystemPreferencesProviderImpl
                     if (_storeLock != null)
                     {
                         _storeLock.release();
+                        _storeLock.channel().close();
                     }
                 }
                 catch (IOException e)
@@ -452,22 +436,7 @@ public class FileSystemPreferencesProviderImpl
                 finally
                 {
                     _storeLock = null;
-                    try
-                    {
-                        if (_storeRAF != null)
-                        {
-                            _storeRAF.close();
-                        }
-                    }
-                    catch (IOException e)
-                    {
-                        LOGGER.error("Cannot close preferences file", e);
-                    }
-                    finally
-                    {
-                        _storeRAF = null;
-                        _preferences.clear();
-                    }
+                    _preferences.clear();
                 }
             }
         }
@@ -544,16 +513,14 @@ public class FileSystemPreferencesProviderImpl
             checkStoreOpened();
             try
             {
-                ByteArrayOutputStream baos = new ByteArrayOutputStream();
-                _objectMapper.writeValue(baos, _preferences);
-                FileChannel channel = _storeRAF.getChannel();
-                long currentSize = channel.size();
-                channel.position(0);
-                channel.write(ByteBuffer.wrap(baos.toByteArray()));
-                if (currentSize > baos.size())
+                _fileHelper.writeFileSafely(_storeFile.toPath(), new BaseAction<File, IOException>()
                 {
-                    channel.truncate(baos.size());
-                }
+                    @Override
+                    public void performAction(File file) throws IOException
+                    {
+                        _objectMapper.writeValue(file, _preferences);
+                    }
+                });
             }
             catch (IOException e)
             {
@@ -569,5 +536,32 @@ public class FileSystemPreferencesProviderImpl
             }
         }
 
+        private void getFileLock(String lockFilePath)
+        {
+            File lockFile = new File(lockFilePath);
+            try
+            {
+                lockFile.createNewFile();
+                lockFile.deleteOnExit();
+
+                @SuppressWarnings("resource")
+                FileOutputStream out = new FileOutputStream(lockFile);
+                FileChannel channel = out.getChannel();
+                _storeLock = channel.tryLock();
+            }
+            catch (IOException ioe)
+            {
+                throw new IllegalStateException("Cannot create the lock file " + lockFile.getName(), ioe);
+            }
+            catch(OverlappingFileLockException e)
+            {
+                _storeLock = null;
+            }
+
+            if(_storeLock == null)
+            {
+                throw new IllegalStateException("Cannot get lock on file " + lockFile.getAbsolutePath() + ". Is another instance running?");
+            }
+        }
     }
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java
index cb5bc54cd2..2c692ddf4d 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java
@@ -22,6 +22,8 @@ package org.apache.qpid.server.security.auth.database;
 
 import org.apache.log4j.Logger;
 import org.apache.qpid.server.security.auth.UsernamePrincipal;
+import org.apache.qpid.server.util.BaseAction;
+import org.apache.qpid.server.util.FileHelper;
 
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.login.AccountNotFoundException;
@@ -36,7 +38,6 @@ import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Random;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.regex.Pattern;
 
@@ -45,9 +46,9 @@ public abstract class AbstractPasswordFilePrincipalDatabase<U extends PasswordPr
     protected static final String DEFAULT_ENCODING = "utf-8";
 
     private final Pattern _regexp = Pattern.compile(":");
-    private final Map<String, U> _userMap = new HashMap<String, U>();
+    private final Map<String, U> _userMap = new HashMap<>();
     private final ReentrantLock _userUpdate = new ReentrantLock();
-    private final Random _random = new Random();
+    private final FileHelper _fileHelper = new FileHelper();
     private File _passwordFile;
 
     public final void open(File passwordFile) throws IOException
@@ -181,7 +182,7 @@ public abstract class AbstractPasswordFilePrincipalDatabase<U extends PasswordPr
         try
         {
             _userUpdate.lock();
-            final Map<String, U> newUserMap = new HashMap<String, U>();
+            final Map<String, U> newUserMap = new HashMap<>();
 
             BufferedReader reader = null;
             try
@@ -224,71 +225,33 @@ public abstract class AbstractPasswordFilePrincipalDatabase<U extends PasswordPr
 
     protected abstract Logger getLogger();
 
-    protected File createTempFileOnSameFilesystem()
-    {
-        File liveFile = _passwordFile;
-        File tmp;
-
-        do
-        {
-            tmp = new File(liveFile.getPath() + _random.nextInt() + ".tmp");
-        }
-        while(tmp.exists());
 
-        tmp.deleteOnExit();
-        return tmp;
-    }
-
-    protected void swapTempFileToLive(final File temp) throws IOException
+    protected void savePasswordFile() throws IOException
     {
-        File live = _passwordFile;
-        // Remove any existing ".old" file
-        final File old = new File(live.getAbsoluteFile() + ".old");
-        if (old.exists())
+        try
         {
-            old.delete();
-        }
+            _userUpdate.lock();
 
-        // Create an new ".old" file
-        if(!live.renameTo(old))
-        {
-            //unable to rename the existing file to the backup name
-            getLogger().error("Could not backup the existing password file");
-            throw new IOException("Could not backup the existing password file");
+            _fileHelper.writeFileSafely(_passwordFile.toPath(), new BaseAction<File,IOException>()
+            {
+                @Override
+                public void performAction(File file) throws IOException
+                {
+                    writeToFile(file);
+                }
+            });
         }
-
-        // Move temp file to be the new "live" file
-        if(!temp.renameTo(live))
+        finally
         {
-            //failed to rename the new file to the required filename
-            if(!old.renameTo(live))
-            {
-                //unable to return the backup to required filename
-                getLogger().error(
-                        "Could not rename the new password file into place, and unable to restore original file");
-                throw new IOException("Could not rename the new password file into place, and unable to restore original file");
-            }
-
-            getLogger().error("Could not rename the new password file into place");
-            throw new IOException("Could not rename the new password file into place");
+            _userUpdate.unlock();
         }
     }
 
-    protected void savePasswordFile() throws IOException
+    private void writeToFile(File tmp) throws IOException
     {
-        try
-        {
-            _userUpdate.lock();
-
-            BufferedReader reader = null;
-            PrintStream writer = null;
-
-            File tmp = createTempFileOnSameFilesystem();
-
-            try
+            try(PrintStream writer = new PrintStream(tmp);
+                BufferedReader reader = new BufferedReader(new FileReader(_passwordFile)))
             {
-                writer = new PrintStream(tmp);
-                reader = new BufferedReader(new FileReader(_passwordFile));
                 String line;
 
                 while ((line = reader.readLine()) != null)
@@ -346,32 +309,6 @@ public abstract class AbstractPasswordFilePrincipalDatabase<U extends PasswordPr
                 getLogger().error("Unable to create the new password file: " + e);
                 throw new IOException("Unable to create the new password file",e);
             }
-            finally
-            {
-
-                try
-                {
-                    if (reader != null)
-                    {
-                        reader.close();
-                    }
-                }
-                finally
-                {
-                    if (writer != null)
-                    {
-                        writer.close();
-                    }
-                }
-
-            }
-
-            swapTempFileToLive(tmp);
-        }
-        finally
-        {
-            _userUpdate.unlock();
-        }
     }
 
     protected abstract U createUserFromPassword(Principal principal, char[] passwd);
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
index d3c9635502..cf165ff4af 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
@@ -23,6 +23,8 @@ package org.apache.qpid.server.security.auth.manager;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.security.AccessControlException;
 import java.security.Principal;
 import java.util.Collection;
@@ -40,6 +42,7 @@ import javax.security.sasl.SaslServer;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.configuration.BrokerProperties;
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.AbstractConfiguredObject;
 import org.apache.qpid.server.model.Broker;
@@ -56,6 +59,7 @@ import org.apache.qpid.server.security.auth.AuthenticationResult;
 import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus;
 import org.apache.qpid.server.security.auth.UsernamePrincipal;
 import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
+import org.apache.qpid.server.util.FileHelper;
 
 public abstract class PrincipalDatabaseAuthenticationManager<T extends PrincipalDatabaseAuthenticationManager<T>>
         extends AbstractAuthenticationManager<T>
@@ -96,7 +100,11 @@ public abstract class PrincipalDatabaseAuthenticationManager<T extends Principal
         {
             try
             {
-                passwordFile.createNewFile();
+                Path path = new FileHelper().createNewFile(passwordFile, getContextValue(String.class, BrokerProperties.POSIX_FILE_PERMISSIONS));
+                if (!Files.exists(path))
+                {
+                    throw new IllegalConfigurationException(String.format("Cannot create password file at '%s'", _path));
+                }
             }
             catch (IOException e)
             {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/FileGroupDatabase.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/FileGroupDatabase.java
index 0a02ce38fc..1e9e646e35 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/FileGroupDatabase.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/FileGroupDatabase.java
@@ -34,6 +34,8 @@ import java.util.concurrent.ConcurrentSkipListSet;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
+import org.apache.qpid.server.util.BaseAction;
+import org.apache.qpid.server.util.FileHelper;
 import org.apache.qpid.server.util.ServerScopedRuntimeException;
 
 /**
@@ -232,9 +234,9 @@ public class FileGroupDatabase implements GroupDatabase
         }
     }
 
-    private synchronized void writeGroupFile(String groupFile) throws IOException
+    private synchronized void writeGroupFile(final String groupFile) throws IOException
     {
-        Properties propertiesFile = new Properties();
+        final Properties propertiesFile = new Properties();
 
         for (String group : _groupToUserMap.keySet())
         {
@@ -244,19 +246,19 @@ public class FileGroupDatabase implements GroupDatabase
             propertiesFile.setProperty(group + ".users", userList);
         }
 
-        String comment = "Written " + new Date();
-        FileOutputStream fileOutputStream = new FileOutputStream(groupFile);
-        try
-        {
-            propertiesFile.store(fileOutputStream, comment);
-        }
-        finally
+
+        new FileHelper().writeFileSafely(new File(groupFile).toPath(), new BaseAction<File, IOException>()
         {
-            if(fileOutputStream != null)
+            @Override
+            public void performAction(File file) throws IOException
             {
-                fileOutputStream.close();
+                String comment = "Written " + new Date();
+                try(FileOutputStream fileOutputStream = new FileOutputStream(file))
+                {
+                    propertiesFile.store(fileOutputStream, comment);
+                }
             }
-        }
+        });
     }
 
     private void validatePropertyNameIsGroupName(String propertyName)
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java
index 0de6543713..ad671d7e99 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java
@@ -27,6 +27,8 @@ import java.io.IOException;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.nio.channels.OverlappingFileLockException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -40,6 +42,9 @@ import java.util.Map;
 import java.util.UUID;
 
 import org.apache.log4j.Logger;
+import org.apache.qpid.server.configuration.BrokerProperties;
+import org.apache.qpid.server.util.BaseAction;
+import org.apache.qpid.server.util.FileHelper;
 import org.codehaus.jackson.JsonGenerator;
 import org.codehaus.jackson.JsonProcessingException;
 import org.codehaus.jackson.Version;
@@ -85,6 +90,7 @@ public class JsonFileConfigStore implements DurableConfigurationStore
     private final Map<String, List<UUID>> _idsByType = new HashMap<String, List<UUID>>();
     private final ObjectMapper _objectMapper = new ObjectMapper();
     private final Class<? extends ConfiguredObject> _rootClass;
+    private final FileHelper _fileHelper;
 
     private Map<String,Class<? extends ConfiguredObject>> _classNameMapping;
     private String _directoryName;
@@ -123,6 +129,7 @@ public class JsonFileConfigStore implements DurableConfigurationStore
         _objectMapper.registerModule(_module);
         _objectMapper.enable(SerializationConfig.Feature.INDENT_OUTPUT);
         _rootClass = rootClass;
+        _fileHelper = new FileHelper();
     }
 
     @Override
@@ -173,7 +180,7 @@ public class JsonFileConfigStore implements DurableConfigurationStore
             _directoryName = fileFromSettings.getParent();
             _configFileName = fileFromSettings.getName();
             _backupFileName = fileFromSettings.getName() + ".bak";
-            _tempFileName = fileFromSettings.getName() + ".tmp";;
+            _tempFileName = fileFromSettings.getName() + ".tmp";
 
             _lockFileName = fileFromSettings.getName() + ".lck";
         }
@@ -191,56 +198,45 @@ public class JsonFileConfigStore implements DurableConfigurationStore
         checkDirectoryIsWritable(_directoryName);
         getFileLock();
 
-        if(!fileExists(_configFileName))
+        Path storeFile = new File(_directoryName, _configFileName).toPath();
+        Path backupFile = new File(_directoryName, _backupFileName).toPath();
+        if(!Files.exists(storeFile))
         {
-            if(!fileExists(_backupFileName))
+            if(!Files.exists(backupFile))
             {
-                File newFile = new File(_directoryName, _configFileName);
                 try
                 {
-                    _objectMapper.writeValue(newFile, Collections.emptyMap());
+                    String posixFileAttributes = _parent.getContextValue(String.class, BrokerProperties.POSIX_FILE_PERMISSIONS);
+                    storeFile = _fileHelper.createNewFile(storeFile, posixFileAttributes);
+                    _objectMapper.writeValue(storeFile.toFile(), Collections.emptyMap());
                 }
                 catch (IOException e)
                 {
-                    throw new StoreException("Could not write configuration file " + newFile, e);
+                    throw new StoreException("Could not write configuration file " + storeFile, e);
                 }
             }
             else
             {
-                renameFile(_backupFileName, _configFileName);
+                try
+                {
+                    _fileHelper.atomicFileMoveOrReplace(backupFile, storeFile);
+                }
+                catch (IOException e)
+                {
+                    throw new StoreException("Could not move backup to configuration file " + storeFile, e);
+                }
             }
         }
-        deleteFileIfExists(_backupFileName);
-    }
-
-    private void renameFile(String fromFileName, String toFileName)
-    {
-        File toFile = deleteFileIfExists(toFileName);
-        File fromFile = new File(_directoryName, fromFileName);
 
-        if(!fromFile.renameTo(toFile))
+        try
         {
-            throw new StoreException("Cannot rename file " + fromFile.getAbsolutePath() + " to " + toFile.getAbsolutePath());
+            Files.deleteIfExists(backupFile);
         }
-    }
-
-    private File deleteFileIfExists(final String toFileName)
-    {
-        File toFile = new File(_directoryName, toFileName);
-        if(toFile.exists())
+        catch (IOException e)
         {
-            if(!toFile.delete())
-            {
-                throw new StoreException("Cannot delete file " + toFile.getAbsolutePath());
-            }
+            throw new StoreException("Could not delete backup file " + backupFile, e);
         }
-        return toFile;
-    }
 
-    private boolean fileExists(String fileName)
-    {
-        File file = new File(_directoryName, fileName);
-        return file.exists();
     }
 
     private void getFileLock()
@@ -396,7 +392,7 @@ public class JsonFileConfigStore implements DurableConfigurationStore
     private void save()
     {
         UUID rootId = getRootId();
-        Map<String, Object> data = null;
+        final Map<String, Object> data;
 
         if (rootId == null)
         {
@@ -409,15 +405,18 @@ public class JsonFileConfigStore implements DurableConfigurationStore
 
         try
         {
-            deleteFileIfExists(_tempFileName);
-            deleteFileIfExists(_backupFileName);
-
-            File tmpFile = new File(_directoryName, _tempFileName);
-            _objectMapper.writeValue(tmpFile, data);
-            renameFile(_configFileName, _backupFileName);
-            renameFile(tmpFile.getName(),_configFileName);
-            tmpFile.delete();
-            deleteFileIfExists(_backupFileName);
+            Path tmpFile = new File(_directoryName, _tempFileName).toPath();
+            _fileHelper.writeFileSafely( new File(_directoryName, _configFileName).toPath(),
+                    new File(_directoryName, _backupFileName).toPath(),
+                    tmpFile,
+                    new BaseAction<File, IOException>()
+                    {
+                        @Override
+                        public void performAction(File file) throws IOException
+                        {
+                            _objectMapper.writeValue(file, data);
+                        }
+                    });
         }
         catch (IOException e)
         {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/Action.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/Action.java
index 0d53b4d03b..715709a1b2 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/Action.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/Action.java
@@ -20,7 +20,7 @@
  */
 package org.apache.qpid.server.util;
 
-public interface Action<T>
+public interface Action<T> extends BaseAction<T, RuntimeException>
 {
     void performAction(T object);
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/BaseAction.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/BaseAction.java
new file mode 100644
index 0000000000..f7dbcb4d3c
--- /dev/null
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/BaseAction.java
@@ -0,0 +1,26 @@
+/*
+ *
+ * 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.server.util;
+
+public interface BaseAction<T, E extends Exception>
+{
+    void performAction(T object) throws E;
+}
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java
new file mode 100644
index 0000000000..0e1a28f220
--- /dev/null
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java
@@ -0,0 +1,133 @@
+/*
+ *
+ * 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.server.util;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Set;
+
+public class FileHelper
+{
+
+    public void writeFileSafely(Path targetFile, BaseAction<File, IOException> operation) throws IOException
+    {
+        String name = targetFile.toFile().getName();
+        writeFileSafely(targetFile,
+                targetFile.resolveSibling(name + ".bak"),
+                targetFile.resolveSibling(name + ".tmp"),
+                operation);
+    }
+
+    public void writeFileSafely(Path targetFile, Path backupFile, Path tmpFile, BaseAction<File, IOException> write) throws IOException
+    {
+        Files.deleteIfExists(tmpFile);
+        Files.deleteIfExists(backupFile);
+
+        Set<PosixFilePermission> permissions = null;
+        if (Files.exists(targetFile) && isPosixFileSystem(targetFile))
+        {
+            permissions =  Files.getPosixFilePermissions(targetFile);
+        }
+
+        tmpFile = createNewFile(tmpFile, permissions);
+
+        write.performAction(tmpFile.toFile());
+
+        atomicFileMoveOrReplace(targetFile, backupFile);
+
+        if (permissions != null)
+        {
+            Files.setPosixFilePermissions(backupFile, permissions);
+        }
+
+        atomicFileMoveOrReplace(tmpFile, targetFile);
+
+        Files.deleteIfExists(tmpFile);
+        Files.deleteIfExists(backupFile);
+    }
+
+    public Path createNewFile(File newFile, String posixFileAttributes) throws IOException
+    {
+        return createNewFile(newFile.toPath(), posixFileAttributes);
+    }
+
+    public Path createNewFile(Path newFile, String posixFileAttributes) throws IOException
+    {
+        Set<PosixFilePermission> permissions = posixFileAttributes == null ? null : PosixFilePermissions.fromString(posixFileAttributes);
+        return createNewFile(newFile, permissions );
+    }
+
+    public Path createNewFile(Path newFile, Set<PosixFilePermission> permissions) throws IOException
+    {
+        if (!Files.exists(newFile))
+        {
+            newFile = Files.createFile(newFile);
+        }
+
+        if (permissions != null && isPosixFileSystem(newFile))
+        {
+            Files.setPosixFilePermissions(newFile, permissions);
+        }
+
+        return newFile;
+    }
+
+    public boolean isPosixFileSystem(Path path) throws IOException
+    {
+        while (!Files.exists(path))
+        {
+            path = path.getParent();
+
+            if (path == null)
+            {
+                return false;
+            }
+        }
+        return Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
+    }
+
+    public Path atomicFileMoveOrReplace(Path sourceFile, Path targetFile) throws IOException
+    {
+        try
+        {
+            return Files.move(sourceFile, targetFile, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
+        }
+        catch(AtomicMoveNotSupportedException e)
+        {
+            if (sourceFile.toFile().renameTo(targetFile.toFile()))
+            {
+                return targetFile;
+            }
+            else
+            {
+                throw new RuntimeException("Atomic move is unsupported and rename from : '"
+                + sourceFile + "' to: '" + targetFile + "' failed.");
+            }
+        }
+    }
+}
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java
new file mode 100644
index 0000000000..9d47ed496a
--- /dev/null
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java
@@ -0,0 +1,138 @@
+/*
+ *
+ * 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.server.util;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Set;
+
+import org.apache.qpid.test.utils.QpidTestCase;
+
+public class FileHelperTest extends QpidTestCase
+{
+    private static final String TEST_FILE_PERMISSIONS = "rwxr-x---";
+    private File _testFile;
+    private FileHelper _fileHelper;
+
+    @Override
+    public void setUp() throws Exception
+    {
+        super.setUp();
+        _testFile = new File(TMP_FOLDER, "test-" + System.currentTimeMillis());
+        _fileHelper = new FileHelper();
+    }
+
+    @Override
+    public void tearDown() throws Exception
+    {
+        try
+        {
+            super.tearDown();
+        }
+        finally
+        {
+            Files.deleteIfExists(_testFile.toPath());
+        }
+    }
+
+    public void testCreateNewFile() throws Exception
+    {
+        assertFalse("File should not exist", _testFile.exists());
+        Path path = _fileHelper.createNewFile(_testFile, TEST_FILE_PERMISSIONS);
+        assertTrue("File was not created", path.toFile().exists());
+        if (Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class))
+        {
+            assertPermissions(path);
+        }
+    }
+
+    public void testCreateNewFileUsingRelativePath() throws Exception
+    {
+        _testFile = new File("./tmp-" + System.currentTimeMillis());
+        assertFalse("File should not exist", _testFile.exists());
+        Path path = _fileHelper.createNewFile(_testFile, TEST_FILE_PERMISSIONS);
+        assertTrue("File was not created", path.toFile().exists());
+        if (Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class))
+        {
+            assertPermissions(path);
+        }
+    }
+
+    public void testWriteFileSafely() throws Exception
+    {
+        Path path = _fileHelper.createNewFile(_testFile, TEST_FILE_PERMISSIONS);
+        _fileHelper.writeFileSafely(path, new BaseAction<File, IOException>()
+        {
+            @Override
+            public void performAction(File file) throws IOException
+            {
+                Files.write(file.toPath(), "test".getBytes("UTF8"));
+                assertEquals("Unexpected name", _testFile.getAbsolutePath() + ".tmp", file.getPath());
+            }
+        });
+
+        assertTrue("File was not created", path.toFile().exists());
+
+        if (Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class))
+        {
+            assertPermissions(path);
+        }
+
+        String content =  new String(Files.readAllBytes(path), "UTF-8");
+        assertEquals("Unexpected file content", "test", content);
+    }
+
+    public void testAtomicFileMoveOrReplace() throws Exception
+    {
+        Path path = _fileHelper.createNewFile(_testFile, TEST_FILE_PERMISSIONS);
+        Files.write(path, "test".getBytes("UTF8"));
+        _testFile = _fileHelper.atomicFileMoveOrReplace(path, path.resolveSibling(_testFile.getName() + ".target")).toFile();
+
+        assertFalse("File was not moved", path.toFile().exists());
+        assertTrue("Target file does not exist", _testFile.exists());
+
+        if (Files.getFileStore(_testFile.toPath()).supportsFileAttributeView(PosixFileAttributeView.class))
+        {
+            assertPermissions(_testFile.toPath());
+        }
+    }
+
+
+    private void assertPermissions(Path path) throws IOException
+    {
+        Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(path);
+        assertTrue("Unexpected owner read permission", permissions.contains(PosixFilePermission.OWNER_READ));
+        assertTrue("Unexpected owner write permission", permissions.contains(PosixFilePermission.OWNER_WRITE));
+        assertTrue("Unexpected owner exec permission", permissions.contains(PosixFilePermission.OWNER_EXECUTE));
+        assertTrue("Unexpected group read permission", permissions.contains(PosixFilePermission.GROUP_READ));
+        assertFalse("Unexpected group write permission", permissions.contains(PosixFilePermission.GROUP_WRITE));
+        assertTrue("Unexpected group exec permission", permissions.contains(PosixFilePermission.GROUP_EXECUTE));
+        assertFalse("Unexpected others read permission", permissions.contains(PosixFilePermission.OTHERS_READ));
+        assertFalse("Unexpected others write permission", permissions.contains(PosixFilePermission.OTHERS_WRITE));
+        assertFalse("Unexpected others exec permission", permissions.contains(PosixFilePermission.OTHERS_EXECUTE));
+    }
+}