3
\$\begingroup\$

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);
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ 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? \$\endgroup\$ Jul 22, 2016 at 9:50
  • \$\begingroup\$ 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. \$\endgroup\$
    – JanDotNet
    Jul 22, 2016 at 10:05

2 Answers 2

1
\$\begingroup\$

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.

\$\endgroup\$
3
  • \$\begingroup\$ 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!? \$\endgroup\$
    – JanDotNet
    Jul 21, 2016 at 12:42
  • \$\begingroup\$ 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. \$\endgroup\$
    – t3chb0t
    Jul 21, 2016 at 13:00
  • \$\begingroup\$ 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. \$\endgroup\$
    – JanDotNet
    Jul 22, 2016 at 10:11
2
\$\begingroup\$

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

\$\endgroup\$
1
  • \$\begingroup\$ 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. \$\endgroup\$
    – JanDotNet
    Jul 16, 2016 at 10:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.