From 5091c8ad8ab473d7a7b55d1b24d06a55530b65c5 Mon Sep 17 00:00:00 2001 From: Michael McCann Date: Wed, 11 Oct 2017 13:38:17 -0500 Subject: [PATCH] This feature addresses the narrow scope security case where misconfigured IIS allows for the downloading of settings.txt or settings.json. In the current state these files store database passwords in as unencrypted clear text. This feature uses dotNet's ProtectedData API (scoped to the account the application is running under) to encrypt the values found in the dataSettings.json file so that in the event the file is exposed the sensitive information isn't available in clear text. --- src/Libraries/Nop.Core/Data/DataSettings.cs | 100 ++++++++++++- .../Nop.Core/Data/DataSettingsManager.cs | 134 ++++++++++++------ .../Nop.Core/Data/ProtectedDataService.cs | 75 ++++++++++ 3 files changed, 261 insertions(+), 48 deletions(-) create mode 100644 src/Libraries/Nop.Core/Data/ProtectedDataService.cs diff --git a/src/Libraries/Nop.Core/Data/DataSettings.cs b/src/Libraries/Nop.Core/Data/DataSettings.cs index f63789ef0ba..76232464167 100644 --- a/src/Libraries/Nop.Core/Data/DataSettings.cs +++ b/src/Libraries/Nop.Core/Data/DataSettings.cs @@ -1,5 +1,7 @@ -using System; +using Newtonsoft.Json; +using System; using System.Collections.Generic; +using System.Reflection; namespace Nop.Core.Data { @@ -8,12 +10,14 @@ namespace Nop.Core.Data /// public partial class DataSettings { + protected ProtectedDataService ProtectedDataService = new ProtectedDataService(); + /// /// Ctor /// public DataSettings() { - RawDataSettings=new Dictionary(); + RawDataSettings = new Dictionary(); } /// @@ -26,6 +30,11 @@ public DataSettings() /// public string DataConnectionString { get; set; } + /// + /// Should DataConnectionString be encrypted + /// + public bool EncryptStringSettings { get; set; } + /// /// Raw settings file /// @@ -37,7 +46,90 @@ public DataSettings() /// public bool IsValid() { - return !string.IsNullOrEmpty(this.DataProvider) && !string.IsNullOrEmpty(this.DataConnectionString); + return !String.IsNullOrEmpty(this.DataProvider) && !String.IsNullOrEmpty(this.DataConnectionString); + } + + /// + /// Returns true if any STRING properties are in a decrypted state as determined by ProtectionDataService + /// + /// + public bool HasAnyDecryptedDataSettings() + { + bool retVal = false; + PropertyInfo[] properties = this.GetType().GetProperties(); + + foreach (var property in properties) + { + if (property.PropertyType == typeof(string) && property.GetValue(this) != null && !ProtectedDataService.IsCipherText(property.GetValue(this).ToString())) + { + retVal = true; + } + } + return retVal; + } + + /// + /// Returns true if any STRING properties are in an encrypted state as determined by ProtectionDataService + /// + /// + public bool HasAnyEncryptedDataSettings() + { + bool retVal = false; + PropertyInfo[] properties = this.GetType().GetProperties(); + + foreach (var property in properties) + { + if (property.PropertyType == typeof(string) && property.GetValue(this) != null && ProtectedDataService.IsCipherText(property.GetValue(this).ToString())) + { + retVal = true; + } + } + return retVal; + } + + /// + /// Decrypts all STRING PROPERTIES. If already decrypted then they are unchanged. + /// + /// + public void Decrypt() + { + PropertyInfo[] properties = this.GetType().GetProperties(); + + foreach (var property in properties) + { + if (property.PropertyType == typeof(string) && property.GetValue(this) != null && ProtectedDataService.IsCipherText(property.GetValue(this).ToString())) + { + string clearText = ProtectedDataService.GetClearText(property.GetValue(this).ToString()); + property.SetValue(this, clearText); + } + } + } + + /// + /// Encrypts all STRING PROPERTIES. If already encrypted then they are unchanged. + /// + /// + public void Encrypt() + { + PropertyInfo[] properties = this.GetType().GetProperties(); + + foreach (var property in properties) + { + if (property.PropertyType == typeof(string) && property.GetValue(this) != null && !ProtectedDataService.IsCipherText(property.GetValue(this).ToString())) + { + string cipherText = ProtectedDataService.GetCipherText(this.ProtectedDataService.GetCipherText(property.GetValue(this).ToString())); + property.SetValue(this, cipherText); + } + } + } + + /// + /// + /// + /// + public DataSettings GetClone() + { + return JsonConvert.DeserializeObject(JsonConvert.SerializeObject(this, Formatting.None)); } } -} +} \ No newline at end of file diff --git a/src/Libraries/Nop.Core/Data/DataSettingsManager.cs b/src/Libraries/Nop.Core/Data/DataSettingsManager.cs index c170fab0354..e233278a379 100644 --- a/src/Libraries/Nop.Core/Data/DataSettingsManager.cs +++ b/src/Libraries/Nop.Core/Data/DataSettingsManager.cs @@ -1,7 +1,7 @@ -using System; -using System.IO; -using Newtonsoft.Json; +using Newtonsoft.Json; using Nop.Core.Infrastructure; +using System; +using System.IO; namespace Nop.Core.Data { @@ -11,23 +11,18 @@ namespace Nop.Core.Data public partial class DataSettingsManager { #region Const - private const string ObsoleteDataSettingsFilePath = "~/App_Data/Settings.txt"; private const string DataSettingsFilePath_ = "~/App_Data/dataSettings.json"; - - #endregion + #endregion Const #region Properties - /// /// Gets the path to file that contains data settings /// public static string DataSettingsFilePath => DataSettingsFilePath_; - - #endregion + #endregion Properties #region Methods - /// /// Load settings /// @@ -50,33 +45,7 @@ public virtual DataSettings LoadSettings(string filePath = null, bool reloadSett return new DataSettings(); //get data settings from the old txt file - var dataSettings = new DataSettings(); - using (var reader = new StringReader(File.ReadAllText(filePath))) - { - var settingsLine = string.Empty; - while ((settingsLine = reader.ReadLine()) != null) - { - var separatorIndex = settingsLine.IndexOf(':'); - if (separatorIndex == -1) - continue; - - var key = settingsLine.Substring(0, separatorIndex).Trim(); - var value = settingsLine.Substring(separatorIndex + 1).Trim(); - - switch (key) - { - case "DataProvider": - dataSettings.DataProvider = value; - continue; - case "DataConnectionString": - dataSettings.DataConnectionString = value; - continue; - default: - dataSettings.RawDataSettings.Add(key, value); - continue; - } - } - } + var dataSettings = GetSettingsFromLegacyTextFile(filePath); //save data settings to the new file SaveSettings(dataSettings); @@ -84,7 +53,9 @@ public virtual DataSettings LoadSettings(string filePath = null, bool reloadSett //and delete the old one File.Delete(filePath); - Singleton.Instance = dataSettings; + var usableDataSettings = dataSettings.GetClone(); + usableDataSettings.Decrypt(); + Singleton.Instance = usableDataSettings; return Singleton.Instance; } @@ -93,7 +64,13 @@ public virtual DataSettings LoadSettings(string filePath = null, bool reloadSett return new DataSettings(); //get data settings from the JSON file - Singleton.Instance = JsonConvert.DeserializeObject(text); + var candidateDataSettings = JsonConvert.DeserializeObject(text); + candidateDataSettings.Decrypt(); + Singleton.Instance = candidateDataSettings; + + //Handles resaving the file if the encryption setting has been changed by the user via editing the file + ResaveSettingsWhenEncryptionStateMismatch(); + return Singleton.Instance; } @@ -104,7 +81,65 @@ public virtual DataSettings LoadSettings(string filePath = null, bool reloadSett public virtual void SaveSettings(DataSettings settings) { Singleton.Instance = settings ?? throw new ArgumentNullException(nameof(settings)); + var clonedSettings = settings.GetClone(); + if (settings.EncryptStringSettings) + { + clonedSettings.Encrypt(); + } + else + { + clonedSettings.Decrypt(); + } + string text = JsonConvert.SerializeObject(clonedSettings, Formatting.Indented); + SaveFile(text); //encrypt settings file after saving. + } + /// + /// Attempts to load settings from old-style settings.txt file + /// + /// Path to settings.txt + /// Returns a data settings object + protected DataSettings GetSettingsFromLegacyTextFile(string filePath) + { + var dataSettings = new DataSettings(); + using (var reader = new StringReader(File.ReadAllText(filePath))) + { + var settingsLine = string.Empty; + while ((settingsLine = reader.ReadLine()) != null) + { + var separatorIndex = settingsLine.IndexOf(':'); + if (separatorIndex == -1) + continue; + + var key = settingsLine.Substring(0, separatorIndex).Trim(); + var value = settingsLine.Substring(separatorIndex + 1).Trim(); + + switch (key) + { + case "DataProvider": + dataSettings.DataProvider = value; + continue; + case "DataConnectionString": + dataSettings.DataConnectionString = value; + continue; + case "EncryptStringSettings": + dataSettings.EncryptStringSettings = Convert.ToBoolean(value); + continue; + default: + dataSettings.RawDataSettings.Add(key, value); + continue; + } + } + } + return dataSettings; + } + + /// + /// Saves the value of the text param to the settings file. + /// + /// value of all text to be saved to the settings file. + protected void SaveFile(string text) + { var filePath = CommonHelper.MapPath(DataSettingsFilePath); //create file if not exists @@ -113,12 +148,23 @@ public virtual void SaveSettings(DataSettings settings) //we use 'using' to close the file after it's created using (File.Create(filePath)) { } } - - //save data settings to the file - var text = JsonConvert.SerializeObject(Singleton.Instance, Formatting.Indented); File.WriteAllText(filePath, text); } - #endregion + /// + /// Will call SaveSettings() as needed when the EncryptStringSettings method does not match the state of the current datasettings object. Used when the user manually edits the value of EncryptStringSettings in the settings file. + /// + private void ResaveSettingsWhenEncryptionStateMismatch() + { + if (Singleton.Instance.EncryptStringSettings && Singleton.Instance.HasAnyDecryptedDataSettings()) //If it's not encrypted and should be, then encrypt it. + { + SaveSettings(Singleton.Instance); + } + else if (!Singleton.Instance.EncryptStringSettings && !Singleton.Instance.HasAnyEncryptedDataSettings()) //If it's encrypted and it should not be, then decrypt it. + { + SaveSettings(Singleton.Instance); + } + } + #endregion Methods } -} +} \ No newline at end of file diff --git a/src/Libraries/Nop.Core/Data/ProtectedDataService.cs b/src/Libraries/Nop.Core/Data/ProtectedDataService.cs new file mode 100644 index 00000000000..a380b134d13 --- /dev/null +++ b/src/Libraries/Nop.Core/Data/ProtectedDataService.cs @@ -0,0 +1,75 @@ +using System; +using System.Security.Cryptography; +using System.Security.Principal; +using System.Text; + +namespace Nop.Core.Data +{ + /// + /// Provides utility methods to allow easy encryption and description of small pieces of data with keys based on the user that the program is being run under. + /// + public class ProtectedDataService + { + protected const DataProtectionScope PROTECTION_SCOPE = DataProtectionScope.CurrentUser; //This value determines if ProtectedData will happen at the user account level or machine level + private const string ENCRYPTION_PREFIX = "ENCRYPT::"; //This will be prepended to the beginning of every encrypted string so we can easily determine if it's encrypted or not. + + /// + /// Returns the clear text version of ciphertext generated from this class. When it receives a string without the correct prefix it will function as a null object and return the input parameter. + /// + /// Text to be deciphered. Should begin with ENCRYPTION_PREFIX value and be base64 encoded. + /// + public string GetClearText(string cipherText) + { + if (!IsCipherText(cipherText)) + return cipherText; + var decodedCipherText = System.Convert.FromBase64String(cipherText.Substring(ENCRYPTION_PREFIX.Length)); + return Encoding.UTF8.GetString(ProtectedData.Unprotect(decodedCipherText, GetEntropy(), PROTECTION_SCOPE)); + } + + /// + /// Returns ciphertext that is base64 encoded and prefixed with the ENCRYPTION_PREFIX value. + /// + /// Plain text string to be encrypted. + /// + public string GetCipherText(string clearText) + { + if (IsCipherText(clearText)) + return clearText; + byte[] clearTextBytes = Encoding.UTF8.GetBytes(clearText); + var unencodedCipherText = ProtectedData.Protect(clearTextBytes, GetEntropy(), PROTECTION_SCOPE); + var encodedCipherText = System.Convert.ToBase64String(unencodedCipherText); + return PrefixSettingWithEncryptionFlag(encodedCipherText); + } + + /// + /// Helper method that checks to see if a string starts with ENCRYPTION_PREFIX value + /// + /// text to check. + /// + public bool IsCipherText(string value) + { + return value.StartsWith(ENCRYPTION_PREFIX); + } + + /// + /// Returns entropy that is required for ProtectedData to work. The entropy is non-changing based on the user the code is running under. + /// + /// + protected byte[] GetEntropy() + { //TODO: This may not be the best way to create repeatable, secure entropy. + byte[] entropy = Encoding.ASCII.GetBytes(WindowsIdentity.GetCurrent().Name); + Array.Resize(ref entropy, 16); + return entropy; + } + + /// + /// Helper method that returns a string prefixed with the value of ENCRYPTION_PREFIX + /// + /// ciper text that needs to have prefix attached to denote that it is cipher text. + /// + protected string PrefixSettingWithEncryptionFlag(string cipherText) + { + return $"{ENCRYPTION_PREFIX}{cipherText}"; + } + } +} \ No newline at end of file