0
\$\begingroup\$

I wrote this class. I would be very interested in your feedback how much thread safe this class is. I know users of this class must still use some kind of synchronization when using this class, but I am ok with it, they will do such synchronization.

To save your time and energy, I am more interested in thread safety related feedback of this code, but you can also comment on other aspects of code too, if you wish.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace dppClientModuleNET
{
    public enum ApplicationMode
    {
        SOFTWARE_HSM = 0,
        POSTER_VER = 1
    }


    /// <summary>
    /// This static class is used to manage parameters of this DLL.
    /// You usually initialize it only once using the Init method,
    /// and then mainly query the class for different parameter
    /// values. Properties are mainly only readable.
    /// You can also deinitialize this class.
    /// This class has been written with thread safety in mind-
    /// use with care.
    /// </summary>
    static class DppModuleParameters
    {

        private static bool m_isInited = false;                                                 // Is initialized or not?
        static readonly object m_locker = new object();                                         // Locker

        private static ushort m_softwareVersion = 0x0250;                                       // Software version
        private static ApplicationMode m_applicationMode = ApplicationMode.SOFTWARE_HSM;        // Build type
        private static string m_logDirPath = "";                                                // Log directory
        private static uint m_connectTimeoutMS = 0;                                             // Connect timeout                  
        private static uint m_responseTimeoutMS = 0;                                            // Response timeout
        private static uint m_indexHost = 0;                                                    // Host index
        private static int m_gComPortNumber = 0;                                                // Com port number - this was used as global variable in C++
        private static List<SocketStructure> m_HostAddresses = new List<SocketStructure>();     // List of host addresses
        private static string m_KeysFileName = "";                                              // Path to the keys file
        private static List<Key_t> m_DecryptedKeys = new List<Key_t>();                         // Array of decrypted keys               


        // Getter: Is module initialized?
        public static bool isInited()
        {
            lock (m_locker)
            {
                return m_isInited;
            }
        }

        // Get software version
        public static int GetSoftwareVersion()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_softwareVersion;
            }
        }

        // Get log path
        public static string GetLogPath()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_logDirPath;
            }
        }

        // Get connect timeout
        public static uint GetConnectTimeout()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_connectTimeoutMS;
            }
        }

        // Get build type 
        public static ApplicationMode GetBuildMode()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_applicationMode;
            }
        }

        // Get response timeout
        public static uint GetResponseTimeout()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_responseTimeoutMS;
            }
        }

        // Get index host
        public static uint GetIndexHost()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_indexHost;
            }
        }

        // Set index host
        public static void SetIndexHost(uint host)
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                m_indexHost = host;
            }
        }

        // Get COM port number
        public static int GetComPortNumber()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_gComPortNumber;
            }
        }

        // Get list of host addresses
        // NOTE: Makes a deep copy of the host address array and returns that
        public static List<SocketStructure> GetHostAddressesArray()
        {
            lock (m_locker)
            {
                // Make a deep copy of the list of the host addresses
                List<SocketStructure> tmp = new List<SocketStructure>();
                for (int i = 0; i < m_HostAddresses.Count(); i++)
                {
                    SocketStructure s = new SocketStructure();
                    s.IP = m_HostAddresses[i].IP;
                    s.port = m_HostAddresses[i].port;
                    tmp.Add(s);
                }
                return tmp;
            }
        }

        // Getter for keys file name
        public static string GetKeysFileName()
        {
            lock (m_locker)
            {
                if (m_isInited == false)
                {
                    throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
                }

                return m_KeysFileName;
            }
        }

        // GetKeys
        // NOTE: Makes a deep copy of the keys array and returns that
        public static List<Key_t> GetKeysArray()
        {
            lock (m_locker)
            {
                // Make a copy of the list of the keys
                List<Key_t> tmp = new List<Key_t>();
                for (int i = 0; i < m_DecryptedKeys.Count(); i++)
                {
                    Key_t s = new Key_t();
                    s.KeyName = m_DecryptedKeys[i].KeyName;
                    for (int j = 0; j < 8; j++)
                    {
                        // Copy each key separately
                        s.MasterKey[j] = m_DecryptedKeys[i].MasterKey[j];
                        s.SessionKey[j] = m_DecryptedKeys[i].SessionKey[j];

                    }

                    tmp.Add(s);
                }
                return tmp;
            }
        }

        /// <summary>
        /// Initialize fields of the DppModuleParameters class. Initialization should be done once.
        /// Otherwise you will get exception.
        /// </summary>
        /// <param name="errorInfo">[OUT] Error info structure</param>
        /// <param name="logDirPath">log path</param>
        /// <param name="hsm">Hardware security module parameter </param>
        /// <param name="hostAddresses">Prepaid Server addresses (";"-separated example: "x.x.x.x:yyyy;second.server.name:yyyy")</param>
        /// <param name="connectTimeoutMS"> Connection timeout in ms (0-default value: 15000ms) </param>
        /// <param name="responseTimeoutMS"> Server response timeout in ms (0-default value: 45000ms) </param>
        /// <param name="softwareVersion"> [OUT]  Module version </param>
        /// <param name="indexTCIP">Index to which TCP host to connect; default value is 0</param>
        /// <returns>status</returns>
        public static int Initialize(ref DppErrorInfo_t errorInfo,
                              string logDirPath,
                              string hsm,
                              string hostAddresses,
                              uint connectTimeoutMS,
                              uint responseTimeoutMS,
                              ref ushort? softwareVersion,
                              uint indexTCIP,
                              ApplicationMode buildmode)
        {
            // Lock
            lock (m_locker)
            {
                try
                {
                    try
                    {
                        // We don't allow this structure to be null
                        if (errorInfo == null)
                            return DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG);

                        // Just clean the error structure
                        errorInfo.Code = 0;
                        errorInfo.ActionCode = 0;
                        errorInfo.SysCode = 0;
                        errorInfo.Description = "";
                        errorInfo.DescriptionFromServer = "";

                        errorInfo.Code = DppGlobals.dppERR_SUCCESS;

                        // Store build mode
                        m_applicationMode = buildmode;

                        // .......................

                        // Module parameter object already initialized?
                        if (m_isInited)
                            throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_ALREADY_INITIALIZED), "Parameters module already initialized. Deinitialize first please.");

                        // Pass out software version if out param is not null
                        if (softwareVersion != null)
                            softwareVersion = m_softwareVersion;

                        // Is log directory empty? throw an exception
                        if (String.IsNullOrEmpty(logDirPath))
                            throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Log path not specified");

                        // List of host addresses string is null or empty?
                        if (String.IsNullOrEmpty(hostAddresses))
                            throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Host list not specified");

                        // If HSM is NULL throw a module error exception
                        // if it is empty string we are Okay
                        if (hsm == null)
                            throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "HSM not given");

                        // Extract HSM string and store COM port number in instance variable
                        if (TranslateHSM(hsm) < 0)
                            throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Wrong HSM specified");

                        // .......................  

                        // Parse host addresses and store them
                        string[] firstSplit = hostAddresses.Split(';');
                        for (int i = 0; i < firstSplit.Length; i++)
                        {
                            string[] secondSplit = firstSplit[i].Split(':');
                            if (secondSplit.Length != 2)
                                throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "ParseHostAddresses: List of socket addresses is in not correct format");

                            SocketStructure sockstruct = new SocketStructure();
                            sockstruct.IP = secondSplit[0].Trim();
                            sockstruct.port = Int32.Parse(secondSplit[1]);
                            m_HostAddresses.Add(sockstruct);
                        }

                        // List of host addresses empty?
                        if (m_HostAddresses.Count() == 0)
                        {
                            throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Host address list not specified");
                        }


                        // Set time out parameters
                        m_connectTimeoutMS = (connectTimeoutMS != 0 ? connectTimeoutMS : DppGlobals.ConnectTimeOutDefault);
                        m_responseTimeoutMS = (responseTimeoutMS != 0 ? responseTimeoutMS : DppGlobals.ResponseTimeOutDefault);


                        // Set log dir path of the logger, also store the path 
                        m_logDirPath = logDirPath;
                        DppLogger.LogDirectory = logDirPath;

                        // Software HSM?
                        if (m_applicationMode != ApplicationMode.POSTER_VER)
                        {

                            // Get name of the key file
                            // Note: Since module is not initialized yet, we need to pass along some parameters
                            // otherwise other classes can't use getters to access them
                            DppModuleParameters.GetKeyFileName(buildmode, m_gComPortNumber);

                            // Read key file
                            DppModuleParameters.ReadKeyFile();
                        }

                        m_indexHost = indexTCIP;

                        // Mark as initialized - this is final step
                        m_isInited = true;


                    }
                    // Catch module error
                    catch (DppModuleException ex)
                    {
                        ex.FillErrorStruct(ref errorInfo);
                    }
                    // Catch OS error
                    catch (DppOSException ex)
                    {
                        ex.FillErrorStruct(ref errorInfo);
                    }
                    // Server error
                    catch (DppServerException ex)
                    {
                        ex.FillErrorStruct(ref errorInfo);
                    }
                    // Catch general exception
                    catch (Exception ex)
                    {
                        DppUtilities.FillErrorStructWithGeneralException(ref ex, ref errorInfo);
                    }
                }
                catch (Exception ex)
                {
                    // Some unexpected exception occured probably in the catch clauses, return error code
                    return DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_UNKNOWN);
                }


                // Return the module code from the data structure.
                return errorInfo.Code;
            }

        }



        /// <summary>
        /// Deinitialize function
        /// </summary>
        /// <param name="errorInfo">[OUT] error structure</param>
        /// <param name="pIndexTCIP">[IN/OUT] index of host</param>
        /// <returns>status</returns>
        public static int DeInit(ref DppErrorInfo_t errorInfo, ref uint? pIndexTCIP)
        {
            // Lock
            lock (m_locker)
            {
                try
                {
                    try
                    {
                        // Just clean the error structure
                        errorInfo.Code = 0;
                        errorInfo.ActionCode = 0;
                        errorInfo.SysCode = 0;
                        errorInfo.Description = "";
                        errorInfo.DescriptionFromServer = "";

                        // Pass out index
                        if (pIndexTCIP != null)
                            pIndexTCIP = m_indexHost;

                        m_indexHost = 0;

                        // Clear out log directory
                        m_logDirPath = "";
                        DppLogger.LogDirectory = "";

                        // Clear out other parameters
                        m_HostAddresses.Clear();
                        m_connectTimeoutMS = 0;
                        m_responseTimeoutMS = 0;


                        // Software HSM?
                        if (m_applicationMode != ApplicationMode.POSTER_VER)
                        {
                            // Yes, clear decrypted keys
                            m_DecryptedKeys.Clear();
                        }

                        m_isInited = false;
                    }
                    catch (DppModuleException ex)
                    {
                        ex.FillErrorStruct(ref errorInfo);
                    }
                    catch (DppOSException ex)
                    {
                        ex.FillErrorStruct(ref errorInfo);
                    }
                    catch (DppServerException ex)
                    {
                        ex.FillErrorStruct(ref errorInfo);
                    }
                    catch (Exception ex)
                    {
                        DppUtilities.FillErrorStructWithGeneralException(ref ex, ref errorInfo);
                    }
                }
                catch (Exception ex)
                {
                    // Some unexpected exception occured probably in the catch clauses, return error code
                    return DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_UNKNOWN);
                }


                return errorInfo.Code;
            }
        }

        // Extract COM port number from supplied string. 
        // Supplied string should be in form "TYPE=COM;NUMBER=3"
        private static int TranslateHSM(string hsm)
        {
            if (m_applicationMode == ApplicationMode.SOFTWARE_HSM)
            {
                // Exit if this is software HASP build
                return 0;
            }

            // Perform splitting and extraction
            string[] split1 = hsm.Split(';');
            if (split1.Length == 2)
            {
                string[] splitTmp1 = split1[0].Split('=');
                if (splitTmp1[1] != "COM")
                    return -1;
                string[] splitTmp2 = split1[1].Split('=');
                if (splitTmp2[0] != "NUMBER")
                    return -1;

                // Extract the port number
                m_gComPortNumber = int.Parse(splitTmp2[1]);

            }
            else
            {
                return -1;
            }

            return 0;
        }

        /// <summary>
        /// Parse keys from the key file
        /// </summary>
        /// <param name="inBuffer">byte array representation of a text file which contains keys stored as hex string on separate lines</param>
        /// <param name="bufferSize">size of the byte array</param>
        private static void ParseTextFile(byte[] inBuffer, uint bufferSize)
        {
            string line = "";
            using (Stream stream = new MemoryStream(inBuffer))
            {
                using (StreamReader reader = new StreamReader(stream))
                {
                    while (true)
                    {
                        // Read text file line by line
                        line = reader.ReadLine();
                        if (line == null)
                        {
                            break;
                        }

                        string[] parameters = line.Split(';');
                        if (parameters.Length == 3)
                        {
                            Key_t k = new Key_t();

                            // Copy key name
                            k.KeyName = parameters[0];

                            // Copy master key
                            byte[] mk = DppUtilities.HexStringToByteArray(parameters[1]);
                            Array.Copy(mk, k.MasterKey, 8);

                            // Copy session key
                            byte[] sk = DppUtilities.HexStringToByteArray(parameters[2]);
                            Array.Copy(sk, k.SessionKey, 8);

                            // Add to the global array of keys
                            m_DecryptedKeys.Add(k);
                        }

                    }
                }
            }


        }

        /// <summary>
        /// Retrieve path of the file where keys are stored
        /// </summary>
        private static void GetKeyFileName(ApplicationMode b, int compport)
        {
            // Get folder where DLL resides and make sure path is terminated
            string dllFolder = System.IO.Path.GetDirectoryName(new System.Uri(System.Reflection.Assembly.GetExecutingAssembly().CodeBase).LocalPath);
            if (!dllFolder.EndsWith("\\")) dllFolder += "\\";

            // Call to get serial number function
            DppPosterApi.GetSerialNumber(ref dllFolder, 0 /* should be size of string but not needed in C#*/, 0, DppGlobals.HS_READ_TIMEOUT, 0, b, compport);

            // Store the result in a global variable
            m_KeysFileName = dllFolder + ".enc";
        }

        /// <summary>
        /// Read the key file and get keys out of it.
        /// </summary>
        /// <returns></returns>
        private static int ReadKeyFile()
        {

            // Clear the global keys array 
            m_DecryptedKeys.Clear();

            // Software HASP?
            if (m_applicationMode != ApplicationMode.SOFTWARE_HSM)
            {
                if (m_gComPortNumber <= 0) throw new Exception("Wrong port number");
            }

            // Open file for reading data
            using (FileStream stream = File.Open(m_KeysFileName, FileMode.Open, FileAccess.Read, FileShare.Read))
            {
                // Get file length
                long length = new System.IO.FileInfo(m_KeysFileName).Length;

                byte[] buffer = new byte[length + 1];

                // Read the file contents
                DppUtilities.ReadFileFully(stream, buffer, (int)length);

                if (m_applicationMode == ApplicationMode.SOFTWARE_HSM)
                {
                    // Decrypt file contents
                    DppCryptography.DecryptKeyFile(buffer, (uint)length, buffer);
                }


                // Parse keys from the text file
                ParseTextFile(buffer, (uint)length);
            }


            return 0;

        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

Locking

As I read the code I can see that any public method has big lock (m_locker) which should be fine for thread safety. Any transactional usage would need another locking, but that is not problem of your static class - well, you could somehow expose the locking (e.g. creating IDisposable helper for code like using(DppModuleParameters.Locker()) transaction()) but it looks fine to me as it is.

Private members do not use locking. That is fine too.

Style

Well, looks like you are half the way between C++ and C#. Namespace starting with lowerCase, UPPER_CASE_WITH_UNDERSCORE constants (as enum in C# is more like enum class in C++, not good old enum). DppGlobals.dppERR_INVALID_ARG and DppUtilities.MAKE_HRESULT ...hmm, nothing to add - that's C++ not C#.

Second Look (added)

Well, it is thread-safe as it can be, if that is your only concern, but the whole class is... ehm, ugly, sorry. Why DeInit? Why Get/Set methods instead of properties? Why locking at all? I would rather think about singleton pattern (init once, live forever), remove locks inside simple getters (but leave the simple check to throw if you forget to call Init). Or it can be full class - just create it with proper args and get the info parsed untill you free the class.

I can see only one setter - SetIndexHost, but the m_indexHost is only used in getter and DeInit, but I guess you can use it to index some of the lists... so, why not returning readonly collections? Maybe I am missing something here (the expected usage), but I would really design it in completely different way:

  1. Normal Class (not static)
  2. Read-only (getters and reaonly collections)
  3. Tiny view objects if you really need to change the index - create helper class that can access the collections and return the indexed value.

Something more.... immutable

Static reference to normal class

What I had in mind was static reference to the class, where static Lib TheLib = new Lib() is like calling Init and theLib = null is like DeInit and you can even have public Lib Default { get; set; } in it. But if you cannot redesign, you cannot. You got my thoughts, now it is up to you.

\$\endgroup\$
  • \$\begingroup\$ Plus the infamous m_ prefix and super-terse names (tmp, s, sjk, k, sockstruct, hsm...). I'm glad this style doesn't belong to C# "native" coding convention... \$\endgroup\$ – Konrad Morawski Nov 6 '15 at 10:48
  • \$\begingroup\$ is that what you say sufficient? lock in public methods? private methods don't use locking? I know for different kind of thread safety users of this class must also ensure synchronization \$\endgroup\$ – user88911 Nov 7 '15 at 19:51
  • \$\begingroup\$ Locking in public methods ensure, that they are properly synchronized for any single usage. Private methods are called from public methods holding the lock. Other lock would be needed for transaction not already provided by your class. \$\endgroup\$ – firda Nov 7 '15 at 19:57
  • \$\begingroup\$ @firda: Thanks for the feedback! I am a little but beginner with .NET - so if you don't see any functional defects, then I would leave it as it is. I can't invest now to redesign it. What do you think? Thanks again \$\endgroup\$ – user88911 Nov 9 '15 at 8:07
  • \$\begingroup\$ I rewrote this class from C++ and the C++ version had init/deinit - and it was used like that, so decided to leave it in \$\endgroup\$ – user88911 Nov 9 '15 at 8:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy