WIP Lib version 2.39.1

ModernKeePassLib/Serialization/FileLock.cs

@@ -129,9 +129,12 @@ namespace ModernKeePassLib.Serialization
 				{
 					s = IOConnection.OpenRead(iocLockFile);
 					if(s == null) return null;
-					StreamReader sr = new StreamReader(s, StrUtil.Utf8);
-					string str = sr.ReadToEnd();
-					sr.Dispose();
+
+					string str = null;
+					using(StreamReader sr = new StreamReader(s, StrUtil.Utf8))
+					{
+						str = sr.ReadToEnd();
+					}
 					if(str == null) { Debug.Assert(false); return null; }
 
 					str = StrUtil.NormalizeNewLines(str, false);

ModernKeePassLib/Serialization/FileTransactionEx.cs

@@ -19,6 +19,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.ComponentModel;
 using System.Diagnostics;
 using System.IO;
 using System.Text;
@@ -32,19 +33,25 @@ using ModernKeePassLib.Utility;
 using System.Threading.Tasks;
 using Windows.Storage;
 using Windows.Storage.Streams;
+using ModernKeePassLib.Cryptography;
 using ModernKeePassLib.Resources;
 
 namespace ModernKeePassLib.Serialization
 {
-	public sealed class FileTransactionEx
+	public sealed class FileTransactionEx : IDisposable
 	{
 		private bool m_bTransacted;
-		private IOConnectionInfo m_iocBase;
+		private IOConnectionInfo m_iocBase; // Null means disposed
 		private IOConnectionInfo m_iocTemp;
+		private IOConnectionInfo m_iocTxfMidFallback = null; // Null <=> TxF not used
 
 		private bool m_bMadeUnhidden = false;
 
+		private List<IOConnectionInfo> m_lToDelete = new List<IOConnectionInfo>();
+
 		private const string StrTempSuffix = ".tmp";
+		private const string StrTxfTempPrefix = PwDefs.ShortProductName + "_TxF_";
+		private const string StrTxfTempSuffix = ".tmp";
 
 		private static Dictionary<string, bool> g_dEnabled =
 			new Dictionary<string, bool>(StrUtil.CaseIgnoreComparer);
@@ -56,26 +63,23 @@ namespace ModernKeePassLib.Serialization
 			set { g_bExtraSafe = value; }
 		}
 
-		public FileTransactionEx(IOConnectionInfo iocBaseFile)
+		public FileTransactionEx(IOConnectionInfo iocBaseFile) :
+			this(iocBaseFile, true)
 		{
-			Initialize(iocBaseFile, true);
 		}
 
 		public FileTransactionEx(IOConnectionInfo iocBaseFile, bool bTransacted)
-		{
-			Initialize(iocBaseFile, bTransacted);
-		}
-
-		private void Initialize(IOConnectionInfo iocBaseFile, bool bTransacted)
 		{
 			if(iocBaseFile == null) throw new ArgumentNullException("iocBaseFile");
 
 			m_bTransacted = bTransacted;
+
 			m_iocBase = iocBaseFile.CloneDeep();
+			if(m_iocBase.IsLocalFile())
+				m_iocBase.Path = UrlUtil.GetShortestAbsolutePath(m_iocBase.Path);
 
 			string strPath = m_iocBase.Path;
 
-#if !ModernKeePassLib
 			if(m_iocBase.IsLocalFile())
 			{
 				try
@@ -93,10 +97,23 @@ namespace ModernKeePassLib.Serialization
 						if((long)(fa & FileAttributes.ReparsePoint) != 0)
 							m_bTransacted = false;
 					}
+					else
+					{
+						// If the base and the temporary file are in different
+						// folders and the base file doesn't exist (i.e. we can't
+						// backup the ACL), a transaction would cause the new file
+						// to have the default ACL of the temporary folder instead
+						// of the one of the base folder; therefore, we don't use
+						// a transaction when the base file doesn't exist (this
+						// also results in other applications monitoring the folder
+						// to see one file creation only)
+						m_bTransacted = false;
+					}
 				}
 				catch(Exception) { Debug.Assert(false); }
 			}
 
+#if !ModernKeePassLib
 			// Prevent transactions for FTP URLs under .NET 4.0 in order to
 			// avoid/workaround .NET bug 621450:
 			// https://connect.microsoft.com/VisualStudio/feedback/details/621450/problem-renaming-file-on-ftp-server-using-ftpwebrequest-in-net-framework-4-0-vs2010-only
@@ -118,90 +135,152 @@ namespace ModernKeePassLib.Serialization
 			{
 				m_iocTemp = m_iocBase.CloneDeep();
 				m_iocTemp.Path += StrTempSuffix;
+
+				TxfPrepare(); // Adjusts m_iocTemp
 			}
 			else m_iocTemp = m_iocBase;
 		}
 
+		~FileTransactionEx()
+		{
+			Dispose(false);
+		}
+
+		public void Dispose()
+		{
+			Dispose(true);
+			GC.SuppressFinalize(this);
+		}
+
+		private void Dispose(bool bDisposing)
+		{
+			m_iocBase = null;
+			if(!bDisposing) return;
+
+			try
+			{
+				foreach(IOConnectionInfo ioc in m_lToDelete)
+				{
+					if(IOConnection.FileExists(ioc, false))
+						IOConnection.DeleteFile(ioc);
+				}
+
+				m_lToDelete.Clear();
+			}
+			catch(Exception) { Debug.Assert(false); }
+		}
+
 		public Stream OpenWrite()
 		{
-			if(!m_bTransacted) m_bMadeUnhidden = UrlUtil.UnhideFile(m_iocTemp.Path);
-			else // m_bTransacted
-			{
-				try { IOConnection.DeleteFile(m_iocTemp); }
-				catch(Exception) { }
-			}
+			if(m_iocBase == null) { Debug.Assert(false); throw new ObjectDisposedException(null); }
+
+			if(!m_bTransacted) m_bMadeUnhidden |= UrlUtil.UnhideFile(m_iocTemp.Path);
 
 			return IOConnection.OpenWrite(m_iocTemp);
 		}
 
 		public void CommitWrite()
 		{
-			if(m_bTransacted) CommitWriteTransaction();
-			else // !m_bTransacted
+			if(m_iocBase == null) { Debug.Assert(false); throw new ObjectDisposedException(null); }
+
+			if(!m_bTransacted)
 			{
-				if(m_bMadeUnhidden) UrlUtil.HideFile(m_iocTemp.Path, true); // Hide again
+				if(m_bMadeUnhidden) UrlUtil.HideFile(m_iocTemp.Path, true);
 			}
+			else CommitWriteTransaction();
+
+			m_iocBase = null; // Dispose
 		}
 
 		private void CommitWriteTransaction()
 		{
-			bool bMadeUnhidden = UrlUtil.UnhideFile(m_iocBase.Path);
-
-#if (!ModernKeePassLib && !KeePassLibSD)
-			FileSecurity bkSecurity = null;
-			bool bEfsEncrypted = false;
-#endif
-
 			if(g_bExtraSafe)
 			{
 				if(!IOConnection.FileExists(m_iocTemp))
 					throw new FileNotFoundException(m_iocTemp.Path +
-						Environment.NewLine + KLRes.FileSaveFailed);
+						MessageService.NewLine + KLRes.FileSaveFailed);
 			}
 
-			if(IOConnection.FileExists(m_iocBase))
-			{
-#if (!ModernKeePassLib && !KeePassLibSD)
-				if(m_iocBase.IsLocalFile())
-				{
-					try
-					{
-						FileAttributes faBase = File.GetAttributes(m_iocBase.Path);
-						bEfsEncrypted = ((long)(faBase & FileAttributes.Encrypted) != 0);
+			bool bMadeUnhidden = UrlUtil.UnhideFile(m_iocBase.Path);
 
-						DateTime tCreation = File.GetCreationTimeUtc(m_iocBase.Path);
-						bkSecurity = File.GetAccessControl(m_iocBase.Path);
+#if !ModernKeePassLib
+			// 'All' includes 'Audit' (SACL), which requires SeSecurityPrivilege,
+			// which we usually don't have and therefore get an exception;
+			// trying to set 'Owner' or 'Group' can result in an
+			// UnauthorizedAccessException; thus we restore 'Access' (DACL) only
+			const AccessControlSections acs = AccessControlSections.Access;
 
-						File.SetCreationTimeUtc(m_iocTemp.Path, tCreation);
-					}
-					catch(Exception) { Debug.Assert(false); }
-				}
+			bool bEfsEncrypted = false;
+			byte[] pbSec = null;
 #endif
+			DateTime? otCreation = null;
 
-				IOConnection.DeleteFile(m_iocBase);
-			}
-
-			IOConnection.RenameFile(m_iocTemp, m_iocBase);
-
-#if (!ModernKeePassLib && !KeePassLibSD)
-			if(m_iocBase.IsLocalFile())
+			bool bBaseExists = IOConnection.FileExists(m_iocBase);
+			if(bBaseExists && m_iocBase.IsLocalFile())
 			{
+				// FileAttributes faBase = FileAttributes.Normal;
 				try
 				{
-					if(bEfsEncrypted)
-					{
-						try { File.Encrypt(m_iocBase.Path); }
-						catch(Exception) { Debug.Assert(false); }
-					}
-
-					if(bkSecurity != null)
-						File.SetAccessControl(m_iocBase.Path, bkSecurity);
-				}
-				catch(Exception) { Debug.Assert(false); }
-			}
+#if !ModernKeePassLib
+					FileAttributes faBase = File.GetAttributes(m_iocBase.Path);
+					bEfsEncrypted = ((long)(faBase & FileAttributes.Encrypted) != 0);
+					try { if(bEfsEncrypted) File.Decrypt(m_iocBase.Path); } // For TxF
+					catch(Exception) { Debug.Assert(false); }
 #endif
+					otCreation = File.GetCreationTimeUtc(m_iocBase.Path);
+#if !ModernKeePassLib
+					// May throw with Mono
+					FileSecurity sec = File.GetAccessControl(m_iocBase.Path, acs);
+					if(sec != null) pbSec = sec.GetSecurityDescriptorBinaryForm();
+#endif
+				}
+				catch(Exception) { Debug.Assert(NativeLib.IsUnix()); }
 
-			if(bMadeUnhidden) UrlUtil.HideFile(m_iocBase.Path, true); // Hide again
+				// if((long)(faBase & FileAttributes.ReadOnly) != 0)
+				//	throw new UnauthorizedAccessException();
+			}
+
+			if(!TxfMove())
+			{
+				if(bBaseExists) IOConnection.DeleteFile(m_iocBase);
+				IOConnection.RenameFile(m_iocTemp, m_iocBase);
+			}
+			else { Debug.Assert(pbSec != null); } // TxF success => NTFS => has ACL
+
+			try
+			{
+				// If File.GetCreationTimeUtc fails, it may return a
+				// date with year 1601, and Unix times start in 1970,
+				// so testing for 1971 should ensure validity;
+				// https://msdn.microsoft.com/en-us/library/system.io.file.getcreationtimeutc.aspx
+				if(otCreation.HasValue && (otCreation.Value.Year >= 1971))
+					File.SetCreationTimeUtc(m_iocBase.Path, otCreation.Value);
+
+#if !ModernKeePassLib
+				if(bEfsEncrypted)
+				{
+					try { File.Encrypt(m_iocBase.Path); }
+					catch(Exception) { Debug.Assert(false); }
+				}
+
+				// File.SetAccessControl(m_iocBase.Path, secPrev);
+				// Directly calling File.SetAccessControl with the previous
+				// FileSecurity object does not work; the binary form
+				// indirection is required;
+				// https://sourceforge.net/p/keepass/bugs/1738/
+				// https://msdn.microsoft.com/en-us/library/system.io.file.setaccesscontrol.aspx
+				if((pbSec != null) && (pbSec.Length != 0))
+				{
+					FileSecurity sec = new FileSecurity();
+					sec.SetSecurityDescriptorBinaryForm(pbSec, acs);
+
+					File.SetAccessControl(m_iocBase.Path, sec);
+				}
+#endif
+			}
+			catch(Exception) { Debug.Assert(false); }
+
+			if(bMadeUnhidden) UrlUtil.HideFile(m_iocBase.Path, true);
 		}
 
 		// For plugins
@@ -213,5 +292,155 @@ namespace ModernKeePassLib.Serialization
 				g_dEnabled[strPrefix] = obTransacted.Value;
 			else g_dEnabled.Remove(strPrefix);
 		}
+
+		private static bool TxfIsSupported(char chDriveLetter)
+		{
+			if(chDriveLetter == '\0') return false;
+
+#if !ModernKeePassLib
+			try
+			{
+				string strRoot = (new string(chDriveLetter, 1)) + ":\\";
+
+				const int cch = NativeMethods.MAX_PATH + 1;
+				StringBuilder sbName = new StringBuilder(cch + 1);
+				uint uSerial = 0, cchMaxComp = 0, uFlags = 0;
+				StringBuilder sbFileSystem = new StringBuilder(cch + 1);
+
+				if(!NativeMethods.GetVolumeInformation(strRoot, sbName, (uint)cch,
+					ref uSerial, ref cchMaxComp, ref uFlags, sbFileSystem, (uint)cch))
+				{
+					Debug.Assert(false, (new Win32Exception()).Message);
+					return false;
+				}
+
+				return ((uFlags & NativeMethods.FILE_SUPPORTS_TRANSACTIONS) != 0);
+			}
+			catch(Exception) { Debug.Assert(false); }
+#endif
+			return false;
+		}
+
+		private void TxfPrepare()
+		{
+			try
+			{
+				if(NativeLib.IsUnix()) return;
+				if(!m_iocBase.IsLocalFile()) return;
+
+				string strID = StrUtil.AlphaNumericOnly(Convert.ToBase64String(
+					CryptoRandom.Instance.GetRandomBytes(16)));
+				string strTempDir = UrlUtil.GetTempPath();
+				// See also ClearOld method
+				string strTemp = UrlUtil.EnsureTerminatingSeparator(strTempDir,
+					false) + StrTxfTempPrefix + strID + StrTxfTempSuffix;
+
+				char chB = UrlUtil.GetDriveLetter(m_iocBase.Path);
+				char chT = UrlUtil.GetDriveLetter(strTemp);
+				if(!TxfIsSupported(chB)) return;
+				if((chT != chB) && !TxfIsSupported(chT)) return;
+
+				m_iocTxfMidFallback = m_iocTemp;
+				m_iocTemp = IOConnectionInfo.FromPath(strTemp);
+
+				m_lToDelete.Add(m_iocTemp);
+			}
+			catch(Exception) { Debug.Assert(false); m_iocTxfMidFallback = null; }
+		}
+
+#if !ModernKeePassLib
+        private bool TxfMove()
+		{
+			if(m_iocTxfMidFallback == null) return false;
+
+			if(TxfMoveWithTx()) return true;
+
+			// Move the temporary file onto the base file's drive first,
+			// such that it cannot happen that both the base file and
+			// the temporary file are deleted/corrupted
+			const uint f = (NativeMethods.MOVEFILE_COPY_ALLOWED |
+				NativeMethods.MOVEFILE_REPLACE_EXISTING);
+			bool b = NativeMethods.MoveFileEx(m_iocTemp.Path, m_iocTxfMidFallback.Path, f);
+			if(b) b = NativeMethods.MoveFileEx(m_iocTxfMidFallback.Path, m_iocBase.Path, f);
+			if(!b) throw new Win32Exception();
+
+			Debug.Assert(!File.Exists(m_iocTemp.Path));
+			Debug.Assert(!File.Exists(m_iocTxfMidFallback.Path));
+			return true;
+		}
+
+		private bool TxfMoveWithTx()
+		{
+			IntPtr hTx = new IntPtr((int)NativeMethods.INVALID_HANDLE_VALUE);
+			Debug.Assert(hTx.ToInt64() == NativeMethods.INVALID_HANDLE_VALUE);
+			try
+			{
+				string strTx = PwDefs.ShortProductName + " TxF - " +
+					StrUtil.AlphaNumericOnly(Convert.ToBase64String(
+					CryptoRandom.Instance.GetRandomBytes(16)));
+				const int mchTx = NativeMethods.MAX_TRANSACTION_DESCRIPTION_LENGTH;
+				if(strTx.Length >= mchTx) strTx = strTx.Substring(0, mchTx - 1);
+
+				hTx = NativeMethods.CreateTransaction(IntPtr.Zero,
+					IntPtr.Zero, 0, 0, 0, 0, strTx);
+				if(hTx.ToInt64() == NativeMethods.INVALID_HANDLE_VALUE)
+				{
+					Debug.Assert(false, (new Win32Exception()).Message);
+					return false;
+				}
+
+				if(!NativeMethods.MoveFileTransacted(m_iocTemp.Path, m_iocBase.Path,
+					IntPtr.Zero, IntPtr.Zero, (NativeMethods.MOVEFILE_COPY_ALLOWED |
+					NativeMethods.MOVEFILE_REPLACE_EXISTING), hTx))
+				{
+					Debug.Assert(false, (new Win32Exception()).Message);
+					return false;
+				}
+
+				if(!NativeMethods.CommitTransaction(hTx))
+				{
+					Debug.Assert(false, (new Win32Exception()).Message);
+					return false;
+				}
+
+				Debug.Assert(!File.Exists(m_iocTemp.Path));
+				return true;
+			}
+			catch(Exception) { Debug.Assert(false); }
+			finally
+			{
+				if(hTx.ToInt64() != NativeMethods.INVALID_HANDLE_VALUE)
+				{
+					try { if(!NativeMethods.CloseHandle(hTx)) { Debug.Assert(false); } }
+					catch(Exception) { Debug.Assert(false); }
+				}
+			}
+
+			return false;
+		}
+
+		internal static void ClearOld()
+		{
+			try
+			{
+				// See also TxfPrepare method
+				DirectoryInfo di = new DirectoryInfo(UrlUtil.GetTempPath());
+				List<FileInfo> l = UrlUtil.GetFileInfos(di, StrTxfTempPrefix +
+					"*" + StrTxfTempSuffix, SearchOption.TopDirectoryOnly);
+
+				foreach(FileInfo fi in l)
+				{
+					if(fi == null) { Debug.Assert(false); continue; }
+					if(!fi.Name.StartsWith(StrTxfTempPrefix, StrUtil.CaseIgnoreCmp) ||
+						!fi.Name.EndsWith(StrTxfTempSuffix, StrUtil.CaseIgnoreCmp))
+						continue;
+
+					if((DateTime.UtcNow - fi.LastWriteTimeUtc).TotalDays > 1.0)
+						fi.Delete();
+				}
+			}
+			catch(Exception) { Debug.Assert(false); }
+		}
+#endif
 	}
 }

ModernKeePassLib/Serialization/IOConnection.cs

@@ -431,7 +431,7 @@ namespace ModernKeePassLib.Serialization
 					string strInfo = m_strProxyAddr;
 					if(m_strProxyPort.Length > 0)
 						strInfo += ":" + m_strProxyPort;
-					MessageService.ShowWarning(strInfo, ex.Message);
+					MessageService.ShowWarning(strInfo, ex);
 				}
 #endif
 
@@ -642,7 +642,7 @@ namespace ModernKeePassLib.Serialization
 #endif
 		}
 
-        public static bool FileExists(IOConnectionInfo ioc)
+		public static bool FileExists(IOConnectionInfo ioc)
 		{
 			return FileExists(ioc, false);
 		}

ModernKeePassLib/Serialization/KdbxFile.Read.cs

@@ -530,7 +530,7 @@ namespace ModernKeePassLib.Serialization
 			/* KdbxFile f = new KdbxFile(pwDatabase);
 			f.m_format = KdbxFormat.PlainXml;
 
-			XmlDocument doc = new XmlDocument();
+			XmlDocument doc = XmlUtilEx.CreateXmlDocument();
 			doc.Load(msData);
 
 			XmlElement el = doc.DocumentElement;

ModernKeePassLib/Serialization/KdbxFile.Write.cs

@@ -87,6 +87,7 @@ namespace ModernKeePassLib.Serialization
 
 			m_format = fmt;
 			m_slLogger = slLogger;
+			m_xmlWriter = null;
 
 			PwGroup pgRoot = (pgDataSource ?? m_pwDatabase.RootGroup);
 			UTF8Encoding encNoBom = StrUtil.Utf8;
@@ -203,44 +204,25 @@ namespace ModernKeePassLib.Serialization
 					throw new ArgumentOutOfRangeException("fmt");
 				}
 
-#if ModernKeePassLib || KeePassUAP
-				XmlWriterSettings xws = new XmlWriterSettings();
-				xws.Encoding = encNoBom;
-				xws.Indent = true;
-				xws.IndentChars = "\t";
-				xws.NewLineOnAttributes = false;
-#if ModernKeePassLib
-                // This is needed for Argon2Kdf write
-                xws.Async = true;
-			    if (m_uFileVersion >= FileVersion32_4) xws.CloseOutput = true;
-#endif
-
-				XmlWriter xw = XmlWriter.Create(sXml, xws);
-#else
-				XmlTextWriter xw = new XmlTextWriter(sXml, encNoBom);
-
-				xw.Formatting = Formatting.Indented;
-				xw.IndentChar = '\t';
-				xw.Indentation = 1;
-#endif
-				m_xmlWriter = xw;
+				m_xmlWriter = XmlUtilEx.CreateXmlWriter(sXml, m_uFileVersion >= FileVersion32_4);
 
 				WriteDocument(pgRoot);
 
 				m_xmlWriter.Flush();
-				m_xmlWriter.Dispose();
 			}
 			finally
 			{
+				CommonCleanUpWrite(lStreams, sHashing);
+
 				if(pbCipherKey != null) MemUtil.ZeroByteArray(pbCipherKey);
 				if(pbHmacKey64 != null) MemUtil.ZeroByteArray(pbHmacKey64);
-
-				CommonCleanUpWrite(lStreams, sHashing);
 			}
 		}
 
 		private void CommonCleanUpWrite(List<Stream> lStreams, HashingStreamEx sHashing)
 		{
+			if(m_xmlWriter != null) { m_xmlWriter.Dispose(); m_xmlWriter = null; }
+
 			CloseStreams(lStreams);
 
 			Debug.Assert(lStreams.Contains(sHashing)); // sHashing must be closed
@@ -249,7 +231,6 @@ namespace ModernKeePassLib.Serialization
 
 			CleanUpInnerRandomStream();
 
-			m_xmlWriter = null;
 			m_pbHashOfHeader = null;
 		}
 

ModernKeePassLib/Serialization/KdbxFile.cs

@@ -403,8 +403,8 @@ namespace ModernKeePassLib.Serialization
 			ICipherEngine iCipher = CipherPool.GlobalPool.GetCipher(pu);
 			if(iCipher == null) // CryptographicExceptions are translated to "file corrupted"
 				throw new Exception(KLRes.FileUnknownCipher +
-					Environment.NewLine + KLRes.FileNewVerOrPlgReq +
-					Environment.NewLine + "UUID: " + pu.ToHexString() + ".");
+					MessageService.NewParagraph + KLRes.FileNewVerOrPlgReq +
+					MessageService.NewParagraph + "UUID: " + pu.ToHexString() + ".");
 
 			ICipherEngine2 iCipher2 = (iCipher as ICipherEngine2);
 			if(iCipher2 != null)
@@ -469,14 +469,14 @@ namespace ModernKeePassLib.Serialization
 				Debug.Assert((lStreams.IndexOf(lStreams[i]) == i) &&
 					(lStreams.LastIndexOf(lStreams[i]) == i));
 
-			    try { lStreams[i].Dispose(); }
-                // Unnecessary exception from CryptoStream with
-                // RijndaelManagedTransform when a stream hasn't been
-                // read completely (e.g. incorrect master key)
+				try { lStreams[i].Dispose(); }
+				// Unnecessary exception from CryptoStream with
+				// RijndaelManagedTransform when a stream hasn't been
+				// read completely (e.g. incorrect master key)
 #if !ModernKeePassLib
-                catch(CryptographicException) { }
+				catch(CryptographicException) { }
 #endif
-			    catch (Exception) { Debug.Assert(false); }
+				catch(Exception) { Debug.Assert(false); }
 			}
 
 			// Do not clear the list
@@ -539,8 +539,8 @@ namespace ModernKeePassLib.Serialization
 			FileStream fs = new FileStream(strPath, FileMode.Create,
 				FileAccess.Write, FileShare.None);
 			byte[] pbData = pb.ReadData();
-			fs.Write(pbData, 0, pbData.Length);
-			fs.Close();
+			try { File.WriteAllBytes(strPath, pbData); }
+			finally { if(pb.IsProtected) MemUtil.ZeroByteArray(pbData); }
 #endif
 		}
 	}