From 5527ebf0851fe08cd5bbc89acecfed92f8e31b3a Mon Sep 17 00:00:00 2001 From: David Sparer Date: Tue, 14 Feb 2017 13:21:54 -0700 Subject: [PATCH] added an Auth attribute to the xml cred file to get around an issue when the file contains no passwords --- .../Security/RandomGeneratorTests.cs | 23 +++++++++ mRemoteNGTests/mRemoteNGTests.csproj | 1 + .../Serializers/XmlCredentialDeserializer.cs | 50 ++++++++++--------- .../XmlCredentialRecordSerializer.cs | 1 + mRemoteV1/Schemas/mremoteng_creds_v1_0.xsd | 1 + mRemoteV1/Security/RandomGenerator.cs | 25 ++++++++++ mRemoteV1/mRemoteV1.csproj | 1 + 7 files changed, 79 insertions(+), 23 deletions(-) create mode 100644 mRemoteNGTests/Security/RandomGeneratorTests.cs create mode 100644 mRemoteV1/Security/RandomGenerator.cs diff --git a/mRemoteNGTests/Security/RandomGeneratorTests.cs b/mRemoteNGTests/Security/RandomGeneratorTests.cs new file mode 100644 index 000000000..6969fe120 --- /dev/null +++ b/mRemoteNGTests/Security/RandomGeneratorTests.cs @@ -0,0 +1,23 @@ +using mRemoteNG.Security; +using NUnit.Framework; + +namespace mRemoteNGTests.Security +{ + public class RandomGeneratorTests + { + [Test] + public void ProducesStringOfCorrectLength() + { + var text = RandomGenerator.RandomString(8); + Assert.That(text.Length, Is.EqualTo(8)); + } + + [Test] + public void StringsDiffer() + { + var text1 = RandomGenerator.RandomString(8); + var text2 = RandomGenerator.RandomString(8); + Assert.That(text1, Is.Not.EqualTo(text2)); + } + } +} \ No newline at end of file diff --git a/mRemoteNGTests/mRemoteNGTests.csproj b/mRemoteNGTests/mRemoteNGTests.csproj index bbc85e5e1..e0470afa0 100644 --- a/mRemoteNGTests/mRemoteNGTests.csproj +++ b/mRemoteNGTests/mRemoteNGTests.csproj @@ -145,6 +145,7 @@ + diff --git a/mRemoteV1/Config/Serializers/XmlCredentialDeserializer.cs b/mRemoteV1/Config/Serializers/XmlCredentialDeserializer.cs index acb49a951..588bcfdde 100644 --- a/mRemoteV1/Config/Serializers/XmlCredentialDeserializer.cs +++ b/mRemoteV1/Config/Serializers/XmlCredentialDeserializer.cs @@ -5,6 +5,7 @@ using System.Security; using System.Xml.Linq; using mRemoteNG.Credential; using mRemoteNG.Security; +using mRemoteNG.Security.Authentication; namespace mRemoteNG.Config.Serializers @@ -12,39 +13,42 @@ namespace mRemoteNG.Config.Serializers public class XmlCredentialDeserializer { public string SchemaVersion { get; } = "1.0"; + public IAuthenticator Authenticator { get; set; } public IEnumerable Deserialize(string xml, SecureString decryptionKey) { - try - { - var xdoc = XDocument.Parse(xml); - ValidateSchemaVersion(xdoc); - var cryptographyProvider = BuildCryptoProvider(xdoc.Root); - var credentials = from element in xdoc.Descendants("Credential") - select new CredentialRecord(Guid.Parse(element.Attribute("Id")?.Value)) - { - Title = element.Attribute("Title")?.Value ?? "", - Username = element.Attribute("Username")?.Value ?? "", - Password = - cryptographyProvider.Decrypt(element.Attribute("Password")?.Value, decryptionKey) - .ConvertToSecureString(), - Domain = element.Attribute("Domain")?.Value ?? "" - }; - return credentials; - } - catch - { - return new ICredentialRecord[0]; - } + var xdoc = XDocument.Parse(xml); + var rootElement = xdoc.Root; + ValidateSchemaVersion(rootElement); + var cryptographyProvider = BuildCryptoProvider(rootElement); + Authenticate(rootElement, cryptographyProvider, decryptionKey); + + var credentials = from element in xdoc.Descendants("Credential") + select new CredentialRecord(Guid.Parse(element.Attribute("Id")?.Value)) + { + Title = element.Attribute("Title")?.Value ?? "", + Username = element.Attribute("Username")?.Value ?? "", + Password = + cryptographyProvider.Decrypt(element.Attribute("Password")?.Value, decryptionKey) + .ConvertToSecureString(), + Domain = element.Attribute("Domain")?.Value ?? "" + }; + return credentials.ToArray(); } - private void ValidateSchemaVersion(XDocument xdoc) + private void ValidateSchemaVersion(XElement rootElement) { - var docSchemaVersion = xdoc.Root?.Attribute("SchemaVersion")?.Value; + var docSchemaVersion = rootElement?.Attribute("SchemaVersion")?.Value; if (docSchemaVersion != SchemaVersion) throw new Exception($"The schema version of this document is not supported by this class. Document Version: {docSchemaVersion} Supported Version: {SchemaVersion}"); } + private void Authenticate(XElement rootElement, ICryptographyProvider cryptographyProvider, SecureString key) + { + var authString = rootElement.Attribute("Auth")?.Value; + cryptographyProvider.Decrypt(authString, key); + } + private ICryptographyProvider BuildCryptoProvider(XElement rootElement) { if (rootElement == null) diff --git a/mRemoteV1/Config/Serializers/XmlCredentialRecordSerializer.cs b/mRemoteV1/Config/Serializers/XmlCredentialRecordSerializer.cs index c2eeefa6b..bfebc6b50 100644 --- a/mRemoteV1/Config/Serializers/XmlCredentialRecordSerializer.cs +++ b/mRemoteV1/Config/Serializers/XmlCredentialRecordSerializer.cs @@ -29,6 +29,7 @@ namespace mRemoteNG.Config.Serializers new XAttribute("EncryptionEngine", _cryptographyProvider.CipherEngine), new XAttribute("BlockCipherMode", _cryptographyProvider.CipherMode), new XAttribute("KdfIterations", _cryptographyProvider.KeyDerivationIterations), + new XAttribute("Auth", _cryptographyProvider.Encrypt(RandomGenerator.RandomString(30), encryptionKey)), new XAttribute("SchemaVersion", SchemaVersion), from r in credentialRecords select new XElement("Credential", diff --git a/mRemoteV1/Schemas/mremoteng_creds_v1_0.xsd b/mRemoteV1/Schemas/mremoteng_creds_v1_0.xsd index 462ccde14..8ad81340f 100644 --- a/mRemoteV1/Schemas/mremoteng_creds_v1_0.xsd +++ b/mRemoteV1/Schemas/mremoteng_creds_v1_0.xsd @@ -17,6 +17,7 @@ + diff --git a/mRemoteV1/Security/RandomGenerator.cs b/mRemoteV1/Security/RandomGenerator.cs new file mode 100644 index 000000000..286caddfa --- /dev/null +++ b/mRemoteV1/Security/RandomGenerator.cs @@ -0,0 +1,25 @@ +using System; +using System.Text; +using Org.BouncyCastle.Security; + +namespace mRemoteNG.Security +{ + public class RandomGenerator + { + public static string RandomString(int length) + { + if (length < 0) + throw new ArgumentException($"{nameof(length)} must be a positive integer"); + + var randomGen = new SecureRandom(); + var stringBuilder = new StringBuilder(); + const string availableChars = @"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789`~!@#$%^&*()-_=+|[]{};:',./<>?"; + for (var x = 0; x < length; x++) + { + var randomIndex = randomGen.Next(availableChars.Length - 1); + stringBuilder.Append(availableChars[randomIndex]); + } + return stringBuilder.ToString(); + } + } +} \ No newline at end of file diff --git a/mRemoteV1/mRemoteV1.csproj b/mRemoteV1/mRemoteV1.csproj index ef705e8a7..ec6270c23 100644 --- a/mRemoteV1/mRemoteV1.csproj +++ b/mRemoteV1/mRemoteV1.csproj @@ -213,6 +213,7 @@ +