Create/Verify signed license files

Question

The purpose of the class is to provide an easy way for create and verify a signed license file (or key). I am interested in any improvments / thoughts about the class.

Usage:

var signer = new LengthSigner();
// creation
var properties = new Dictionary<string, string>();
properties.Add("Key1", "Value1");
properties.Add("Key2", "Value2");
var licenseFile = new LicenseFile("Me", "My Company", properties);
licenseFile.Sign(new LengthSigner());
var content = licenseFile.Serialize();
Console.WriteLine(content);
// actual content:
// ExcsNAEsVjMTFCI0BjhUMxgSMgk8TwEOKG97JHVtCXNDTHsJPEkBehMYVGVadwEyLyhJYU8wXlVDTndhBA9uNREvCA==
// unconfused content:
// 15.07.2016 00:00:00
// Me
// My Company
// Key1:Value1
// Key2:Value2
// 63
// verification
var deserializedLicenseFile = LicenseFile.Deserialize(content);
deserializedLicenseFile.Verify(new LengthSigner()); // true

LicenseFile.cs

public class LicenseFile
{
 public LicenseFile(string licensee, string company, IDictionary<string, string> properties)
 : this(licensee, company, DateTime.UtcNow.Date, properties, null)
 {
 }
 private LicenseFile(string licensee, string company, DateTime issueDate, IDictionary<string, string> properties, string signature)
 {
 if (string.IsNullOrWhiteSpace(licensee))
 throw new ArgumentException("licensee");
 IssueDate = issueDate;
 Licensee = licensee;
 Company = string.IsNullOrWhiteSpace(company) ? "Private License" : company;
 Signature = signature;
 var dict = properties ?? new Dictionary<string, string>();
 if (dict.Keys.Any(key => key.Contains(":")))
 throw new FormatException("Character ':' is not allowed as property key.");
 Properties = new ReadOnlyDictionary<string, string>(dict);
 }
 private static byte[] ConfusingBytes = new byte[] { 34, 34, 2, 4, 54, 2, 100, 3 };
 public DateTime IssueDate { get; private set; }
 public string Licensee { get; private set; }
 public string Company { get; private set; }
 private string Signature { get; set; }
 public IDictionary<string, string> Properties { get; private set; }
 public static LicenseFile Deserialize(string content)
 {
 var unconfused = Unconfuse(content);
 var lines = (unconfused ?? "").Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
 if (lines.Length < 4)
 ThrowInvalidFormatException();
 return ReadLicenseFile(lines);
 }
 public string Serialize()
 {
 var sb = new StringBuilder();
 WriteLicenseProperties(sb);
 WriteSignature(sb);
 return Confuse(sb.ToString());
 }
 private static LicenseFile ReadLicenseFile(string[] lines)
 {
 try
 {
 var issueDate = DateTime.Parse(lines[0]);
 var licensee = lines[1];
 var company = lines[2];
 var signature = lines.Last();
 var properties = new Dictionary<string, string>();
 foreach (var line in lines.Skip(3).Take(lines.Length - 4))
 {
 var pair = GetKeyValuePair(line);
 properties.Add(pair.Key, pair.Value);
 }
 return new LicenseFile(licensee, company, issueDate, properties, signature);
 }
 catch (Exception ex)
 {
 //Log.LogError("Error while deserializing LicenseFile.", ex);
 ThrowInvalidFormatException();
 return null;
 }
 }
 public void Verify(ISigner signer)
 {
 var sb = new StringBuilder();
 WriteLicenseProperties(sb);
 if (!signer.Verify(sb.ToString(), Signature))
 ThrowInvalidSignatureException();
 }
 public void Sign(ISigner signer)
 {
 var sb = new StringBuilder();
 WriteLicenseProperties(sb);
 Signature = signer.Sign(sb.ToString());
 }
 private void WriteSignature(StringBuilder sb)
 {
 if (string.IsNullOrEmpty(Signature))
 ThrowNotSignedException();
 sb.AppendLine(Signature);
 }
 private void WriteLicenseProperties(StringBuilder sb)
 {
 sb.AppendLine(IssueDate.ToString());
 sb.AppendLine(Licensee);
 sb.AppendLine(Company);
 foreach (var property in Properties)
 sb.AppendLine(property.Key + ":" + property.Value);
 }
 private static KeyValuePair<string, string> GetKeyValuePair(string line)
 {
 var index = line.IndexOf(':');
 if (index < 0)
 ThrowInvalidFormatException();
 var key = line.Substring(0, index);
 var value = line.Substring(index + 1);
 return new KeyValuePair<string, string>(key, value);
 }
 private static string Confuse(string input)
 {
 var bytes = Encoding.UTF8.GetBytes(input);
 for (int i = 0; i < bytes.Length; i++)
 bytes[i] ^= ConfusingBytes[i % ConfusingBytes.Length];
 return Convert.ToBase64String(bytes);
 }
 private static string Unconfuse(string input)
 {
 var bytes = Convert.FromBase64String(input);
 for (int i = 0; i < bytes.Length; i++)
 bytes[i] ^= ConfusingBytes[i % ConfusingBytes.Length];
 return Encoding.UTF8.GetString(bytes);
 }
 // - Throw helper
 private static void ThrowInvalidFormatException()
 {
 var msg = "License file has not a valid format.";
 //Log.LogError(msg);
 throw new LicenseFileException(msg);
 }
 private static void ThrowNotSignedException()
 {
 var msg = "License file is not signed.";
 //Log.LogError(msg);
 throw new LicenseFileException(msg);
 }
 private static void ThrowInvalidSignatureException()
 {
 var msg = "Signature of license file is not valid.";
 //Log.LogError(msg);
 throw new LicenseFileException(msg);
 }
}

ISigner.cs

public interface ISigner
{
 string Sign(string content);
 bool Verify(string content, string signature);
}

LicenseFileException.cs

internal class LicenseFileException : Exception
{
 public LicenseFileException(string message)
 : base(message)
 { }
}

LengthSigner.cs

Just for testing purposes

public class LengthSigner : ISigner
{
 public string Sign(string content)
 {
 return content.Length.ToString();
 }
 public bool Verify(string content, string signature)
 {
 return content.Length == int.Parse(signature);
 }
}

• CodesInChaos: What's the point of your "confusion"? The content of a license file usually isn't secret. And why are you using such a comically weak signer, instead of (EC)DSA or RSA?
• JanDotNet: The "confusion" function is just to make the license text not readable (event not by pasting it to a base64 encoder). It is not a real encryption but it should demotivate people from analyzing the license mechanism. The LengthSigner ist just an example, actually an RsaSigner is used. If you are intrested I can post it as another question.

Answer 1

I think the LicenseFile class has too many resposibilities. It stores the license data, it encrypts/decrypts the license, it serializes/deserializes it, it writes/reads it into/from a file.

My suggest to refactor it like this...

---

The LicenseExcryption class would take care of the confusing part:

public class LicenseEncryption
{
 private static byte[] ConfusingBytes = new byte[] { 34, 34, 2, 4, 54, 2, 100, 3 };
 public static string EncryptLicense(string license)
 {
 var bytes = Encoding.UTF8.GetBytes(license);
 for (int i = 0; i < bytes.Length; i++)
 {
 bytes[i] ^= ConfusingBytes[i % ConfusingBytes.Length];
 }
 return Convert.ToBase64String(bytes);
 }
 private static string DencryptLicense(string input)
 {
 var bytes = Convert.FromBase64String(input);
 for (int i = 0; i < bytes.Length; i++)
 bytes[i] ^= ConfusingBytes[i % ConfusingBytes.Length];
 return Encoding.UTF8.GetString(bytes);
 }
}

The License class would only store the data and know how to turn it into a string and maybe deserialize it:

public class License
{
 public License(string license)
 {
 // decrypt, parse/deserialize etc.
 }
 public License(string licensee, string company, DateTime issueDate, IDictionary<string, string> properties, string signature)
 {
 // ...
 }
 public DateTime IssueDate { get; private set; }
 public string Licensee { get; private set; }
 public string Company { get; private set; }
 private string Signature { get; set; }
 public IDictionary<string, string> Properties { get; private set; }
 public void Verify(ISigner signer)
 {
 // verify the signer
 }
 public void Sign(ISigner signer)
 {
 // sign
 }
 private static KeyValuePair<string, string> GetKeyValuePair(string line)
 {
 var index = line.IndexOf(':');
 if (index < 0) ThrowInvalidFormatException();
 var key = line.Substring(0, index);
 var value = line.Substring(index + 1);
 return new KeyValuePair<string, string>(key, value);
 }
 public override string ToString()
 {
 var licenseBuilder = new StringBuilder();
 AddProperties(licenseBuilder, this);
 AddSignature(licenseBuilder, this);
 return LicenseEncryption.EncryptLicense(licenseBuilder.ToString());
 }
 private static void AddSignature(StringBuilder licenseBuilder, License license)
 {
 if (string.IsNullOrEmpty(license.Signature)) ThrowNotSignedException();
 licenseBuilder.AppendLine(license.Signature);
 }
 private static void AddProperties(StringBuilder licenseBuilder, License license)
 {
 licenseBuilder.AppendLine(license.IssueDate.ToString());
 licenseBuilder.AppendLine(license.Licensee);
 licenseBuilder.AppendLine(license.Company);
 foreach (var property in license.Properties)
 {
 licenseBuilder.AppendLine(property.Key + ":" + property.Value);
 }
 }
 private static void ThrowInvalidFormatException()
 {
 var msg = "License file has not a valid format.";
 //Log.LogError(msg);
 throw new LicenseFileException(msg);
 }
 private static void ThrowNotSignedException()
 {
 var msg = "License file is not signed.";
 //Log.LogError(msg);
 throw new LicenseFileException(msg);
 }
 private static void ThrowInvalidSignatureException()
 {
 var msg = "Signature of license file is not valid.";
 //Log.LogError(msg);
 throw new LicenseFileException(msg);
 }
}

Lastly the LicenseFile would only know how to read and write license data from a file or write it into a one:

public class LicenseFile
{
 public static void Save(License license, string fileName)
 {
 // write the license into a file
 }
 public static License From(string fileName)
 {
 // read the file contents, create license...
 } 
}

You could also add the Save/From methods to the License class itself but still keep the writing/reading logic in a specialized unit.

• JanDotNet: Thanks for the review! Extracting the "encryption" logic is a good idea, I'll do that. The LicenseFile is called LicenseFile because there is already a type License in .net Framework, but it hasn't any methods for reading/writing a file. However, the "File" postfix is confusing... Maybe I should call it just Lic or SignedLicense!?
• t3chb0t: That's what namespaces are for ;-) I wouldn't avoid any name only because .net already uses it. ok maybe if it was a String or something essential but (Signed)License sounds like a perfect name for a (signed)license.
• JanDotNet: Yes.. Normaly that is right, but the SignedLicense (I decide to call it that way ;)) may be used with the .Net-License type togeter AND I have another type called License that is used to store license information in backend and is used togerther with the SignedLicense.

Answer 2

Just scratching on the surface - SOLID Principle

• Interface Segregation : Why is your ISigner responsible for Signing and Verifying. From the name I would imagine it's responsibility is to Sign and not Verify.

• Dependency Injection : In your second constructor, you create this

  Properties = new ReadOnlyDictionary<string, string>(dict); why not Properties=properties

• Arrays are always declared with the first letter as Lowercase

  private static byte[] ConfusingBytes = new byte[] { 34, 34, 2, 4, 54, 2, 100, 3 }; should be

  private static byte[] confusingBytes = new byte[] { 34, 34, 2, 4, 54, 2, 100, 3 };

Note you might as well use DateTime.TryParse as opposed to Try..Catch

• JanDotNet: Thanks for the Review! a) Yes, when following the interface segregation principle strictly the interface should be splitted... on the other hand: an interface with 2 methods (that have a similar context) isn't too big. b) Because I want the class to be immutable. Otherwise the signature becomes invalid. c) sure... little slip-up from me.