Archive

Archive for the ‘Coding’ Category

PasswordHash - how to correctly create a salted password hash

June 8th, 2010

Most people I’ve seen online compute a simple hash of password + salt for persistence and authentication. This is the accepted standard in a straight-forward solution:

    byte[] Hash(string password)
    {
        byte[] pass = System.Text.Encoding.UTF8.GetBytes(password);
        //Create the salt to use
        byte[] salt = new byte[32];
        new RNGCryptoServiceProvider().GetBytes(salt);
        //Create the hash of password and salt
        HashAlgorithm hashAlgo = new SHA256Managed();
        hashAlgo.TransformBlock(salt, 0, salt.Length, salt, 0);
        hashAlgo.TransformFinalBlock(pass, 0, pass.Length);
        byte[] hash = _hashAlgo.Hash;
        hashAlgo.Initialize();
        //Copy the combined salt + hash to a single array
        byte[] result = new byte[salt.Length + hash.Length];
        Array.Copy(salt, result, salt.Length);
        Array.Copy(hash, 0, result, salt.Length, hash.Length);
        return result;
    }

The only deviation i’ve seen from this is to either use existing data (say a primary key in the database) for the salt, or to ‘hide’ the salt at some offset in the result. Both of these ideas are valid yet amount to not much more than a little obfuscation. I don’t recommend it.

Now if you REALLY want to secure your passwords and/or protect against brute force attacks there is another approach. The approach is quite common in generating crypto keys from passwords but can just as easily be used for hashing the password. The idea/concept is expressed in RFC2898 by the introduction of the iteration count. By introducing this into the password hash we increase the computational complexity required to test a password by several orders of magnitude. The straight-forward way of achieving this in the BCL is as follows:

    byte[] Hash(string password)
    {
        //Create the salt to use
        byte[] salt = new byte[20];
        new RNGCryptoServiceProvider().GetBytes(salt);
        //Combine the salt with the iteration-count
        byte[] iterationBytes = BitConverter.GetBytes(1010);
        byte[] iterationSalt = (byte[])salt.Clone();
        for (int i = 0; i < iterationSalt.Length; i++)
            iterationSalt[i] ^= iterationBytes[i % iterationBytes.Length];
        //Create the hash of password and salt
        DeriveBytes deriveBytes = new Rfc2898DeriveBytes(password, iterationSalt, 1010);
        byte[] hash = deriveBytes.GetBytes(20);
        //Copy the combined salt + hash to a single array
        byte[] result = new byte[salt.Length + hash.Length];
        Array.Copy(salt, result, salt.Length);
        Array.Copy(hash, 0, result, salt.Length, hash.Length);
        return result;
    }

This will work very well but will be more costly to calculate than the first algorithm. I would not want to use this in a system that verifies passwords on every request (i.e. Basic Auth over SSL); however, for token-auth systems this will prove much more durable to brute force attacks. By combining the iteration count to the salt (or key) you make the attacker repeat the entire iteration sequence for each iteration count attempted. This makes it significantly more difficult for them to steal the password hashs and crack the passwords without also stealing the software or otherwise knowing the iteration count. Even knowing the iteration count it’s now much more expensive to compute each attempt at matching an entry in their password dictionary.

Recently I’ve added the PasswordHash class to wrap up some of this behavior. Note that this class does not combine the iteration count with the salt, yet still provides significant security benefits over a simple hash.

Coding, Design, Library

HashDerivedBytes - replacing Rfc2898DeriveBytes, but why?

June 8th, 2010

So recently I’ve been working heavily with some of the cryptography stuff in .Net and adding a large amount of it to my open source library. One of the many things I needed to perform was simply encrypting and decrypting a piece of data with a password.

It seems everyone out there is using Rfc2898DeriveBytes by now and not using the older PasswordDeriveBytes. So it’s use is simple enough, construct it with a password, ask it for a number of bytes, and off you go. When things get interesting is correctly providing salt to the derived bytes algorithm. I didn’t want to deal with this all over my code and thus built the PasswordKey class. It auto-magically prepends the salt to the stream when encrypting data. When decrypting data it reads the salt, generates the key, and then decrypts the data.

At first everything went well, it seemed to work and, as per my norm, I quickly got my unit tests pounding on the implementation. Somewhere along the way though I must have used the Rfc2898DeriveBytes class in a way that was not expected as when I began running in release (optimized) mode I started seeing strange results. The tests were generating 32-bit keys with the same password, iteration, and salt combinations and were generating different keys? The funnest part was they only differed after the first 20 bytes. How could this be? If the input was wrong there is almost no likelihood the first 20 bytes would be the same, so how did I fail? I looked and looked, googled and googled some more. Read nearly every line of code in the class using Reflector.Net, nothing. Still to this day I don’t know why it’s breaking, I do know I can get two identical 20-byte keys, or two identical 40-byte keys, but I still get two different 32-byte keys… What should I do? Well I came up with two possible solutions:

1. The first ‘fix’ for this issue resulted in the PBKDF2 class that derives from Rfc2898DeriveBytes. Then by overriding the GetBytes() method I can always pass a value of 20 to the actual GetBytes() base method:

    public override byte[] GetBytes(int cb)
    {
        byte[] buffer = new byte[cb];
        for (int i = 0; i < cb; i += 20)
        {
            int step = Math.Min(20, cb - i);
            Array.Copy(base.GetBytes(20), 0, buffer, i, step);
        }
        return buffer;
    }

2. Then I started debating the value of using a 20-byte hash to generate 32 bytes of data. Seemed like this algorithm was just as applicable with any size hash right? Sure enough… So the second, and finally preferred solution was to recreate the algorithm with a variable hash algorithm. This gave birth to the HashDerivedBytes<THash> implementation which accepts THash to be any HMAC (Keyed hash) derived implementation. With this I’m able to force the use of the managed SHA256 hash routine and elevate the iterations by nearly an order of magnitude without substantial performance loss. In the end I’m pleased with the implementation and can verify it’s compliance with the original algorithm by comparing it’s output using SHA1 with that of the original Rfc2898 and produce compatible results.

Anyway if you have problems with the Rfc2898DeriveBytes class I’d like to hear from you just to make sure I’m not crazy. And now you know of two possible solutions if you do have problems ;)

Coding, Library

//funny…

May 10th, 2010

Recently I was killing time on stackoverflow and found this question:

What is the best comment in source code you have ever encountered?

There are a lot of great quotes in there. If you haven’t seen it already it worth a read when you want to kill some time. I’d also add my own to the list:

    // The DM warns the following encounter is EL25 and should only be attempted
    // by Epic level parties.

Coding, Home

Implementing a generic deep-clone for C# objects

May 8th, 2010

As promised in the previous post “How to implement a generic Shallow-Clone for an object in C#” this will demonstrate deep-cloning objects that don’t explicitly support the behavior.

Before I continue I will again state that I don’t recommend this approach. It will be many times slower than a properly implemented Clone() and can cause rather strange side-effects. For instance, deep-cloning a tree of objects that hold reference to a singleton service class would result in cloning that singleton. This problem simply stems from the fact that no generic code can have enough knowledge about the internal structure of classes to be able to accurately guess when and when not to clone a member reference. Thus if you going to code the logic of which members to clone or not, it’s best done in the objects themselves and is the primary reason I say a generic deep clone is a bad idea. Even if you don’t currently have a reference to a singleton now, what happens when someone adds one to the object graph? How will your application behave?

So with this established as a bad idea it is with great trepidation that I demonstrate how to achieve this. To start with let’s demonstrate usage in the Library:

using CSharpTest.Net.Cloning;

    //Uses serialization API if available or member-wise if non-serializable
    using (ObjectCloner cloner = new SerializerClone())
        fooCopy = cloner.Clone(foo);
    // 3.5 can use extension method instead of the code above:
    fooCopy = foo.DeepClone();

    //To ignore serialization routines, use the following:
    using (ObjectCloner cloner = new MemberwiseClone())
        fooCopy = cloner.Clone(foo);

Implementation details, there are not a lot to share, you can see the source easily enough at:
http://code.google.com/p/csharptest-net/source/browse/#svn/trunk/src/Library/Cloning

There are a few things worth pointing out. The ObjectCloner provides a base-class that handles the object graph and a few well-known types (delegates, arrays, primitives, etc). Derived from this is the MemberwiseClone class which provides a basic ‘memory copy’ type of copy operation. Again deriving from this is the SerializerClone which implements a pseudo serialization routine by attempting to mimic the .Net serializer’s default implementation. This results in three possible types of copy operations:

1. If ISerializable is implemented it will be used, allowing customization of the copy.
2. If [Serializable] decoration is present the FormatterServices.GetSerializableMembers is used to allow respecting the [NonSerialized] attribute.
3. Lastly if the object doesn’t support serialization a straight member-by-member copy is made.

This continues throughout the object graph until all object members have been copied into the new tree. Finally if appropriate calls to IDeserializationCallback will be made just prior to the top-level instance being returned.

One thing of note is that the object graph is made public to allow preventing some instances from being copied. For instance if you want to copy foo’s complete object graph except for any instances of ‘bar’ then you would do this:

using CSharpTest.Net.Cloning;

    using (ObjectCloner cloner = new SerializerClone())
    {
        // maps instances of bar to itself:
        cloner.Graph.Add(bar, bar);
        fooCopy = cloner.Clone(foo);
    }

Coding, Home, Library

How to implement a generic Shallow-Clone for an object in C#

April 14th, 2010

This implementation makes heavy use of the FormatterServices object used by serialization:

http://msdn.microsoft.com/en-us/library/system.runtime.serialization.formatterservices_members.aspx

It provides several helpful methods in this case that are aware of custom serialization options like [NonSerialized]. In the case of an object not being marked [Serializable] you have to create field list manually. Here is an example shallow object clone:

using System.Runtime.Serialization;
using System.Reflection;

    static class Clonable
    {
        public static T Clone<T>(this T instance)
        {
            object copy;
            Type type = instance.GetType();

            if (instance is ICloneable)
                return (T)((ICloneable)instance).Clone();

            List<MemberInfo> fields = new List<MemberInfo>();
            if (type.GetCustomAttributes(typeof(SerializableAttribute), false).Length == 0)
            {
                Type t = type;
                while (t != typeof(Object))
                {
                    fields.AddRange(t.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly));
                    t = t.BaseType;
                }
            }
            else
                fields.AddRange(FormatterServices.GetSerializableMembers(instance.GetType()));

            copy = FormatterServices.GetUninitializedObject(instance.GetType());
            object[] values = FormatterServices.GetObjectData(instance, fields.ToArray());
            FormatterServices.PopulateObjectMembers(copy, fields.ToArray(), values);

            return (T)copy;
        }
    }

As with anything, use this carefully. This method run against some objects will cause native handles to be copied and can then lead to application crashes in unmanaged code.

If you need a deep clone things get a lot more complicated. I don’t recommend it; rather you should insist that objects are decorated with [Serialiable] and simply use serialization to copy the object. That being said, I will post a working deep-clone as a follow up to this post.

Coding, Home

Throw InnerException without loosing the stack trace

February 5th, 2010

How to throw the InnerException of a TargetInvocationException without loosing stack details?

I’ve been plagued by this problem for some time. There are a few common solutions to this:

  1. Option #1 - just leave it alone. The downfall here is that specific types of exceptions cannot be caught easily in calling code. In some instances this can be a very big problem.
  2. Option #2 - re-throwing the InnerExcpetion property. This at least preserves the type of the exception and thus code above you in the call stack will correctly catch and handle exceptions. The problem here is that the stack information previously held in that exception is lost.
  3. Option #3 - Avoiding the problem. If you know the types of the calling parameters you can construct a delegate from the MethodInfo. By calling the delegate (not using DynamicInvoke) the issue is avoided. Again this only works if you have compile-time knowledge of the parameters.

 
Most of the time one of the above has been an acceptable solution; However, recently I ran into the case where none of the above would work. The code has been around a while using option #2 above since the arguments are unknown. Changing the behavior to not throw the original exception type was out since it could break existing client code. The problem that was killing me was the loss of the stack when debugging and monitoring log information. The loss of this information was making spend hours trying to figure out where the thing failed.

So I needed to use MethodInfo.Invoke(), needed the stack and the original excpetion type to be persevered… but how?

Well the first thing I came up with is the following routine which gets down-and-ugly with the Exception class. The catch-block finds the inner-most TargetInvocationException and extracts the InnerException. Then using the Serialization helpers it basically copies the fields of the object to an array. Now we re-throw the exception to set it as the ‘current’ exception when we next call throw without an argument. And finally after we’ve lost our stack we stuff it back in by calling the Serialization helper again to push all the fields back into the exception before calling throw one last time.

Bad Code, do not use

[System.Diagnostics.DebuggerNonUserCode]
[System.Diagnostics.DebuggerStepThrough]
private static void Invoke(MethodInfo method, Object target, params Object[] invokeArgs)
{
	try
	{
		method.Invoke(target, invokeArgs);
	}
	catch (TargetInvocationException te)
	{
		if (te.InnerException == null)
			throw;
		Exception innerException = te.InnerException;

		MemberInfo[] fields = FormatterServices.GetSerializableMembers(innerException.GetType());
		object[] values = FormatterServices.GetObjectData(innerException, fields);

		try { throw innerException; }
		catch(Exception exception)
		{
			FormatterServices.PopulateObjectMembers(exception, fields, values);
			throw;
		}
	}
}

This all worked well… However, I started to wonder about how this might effect some types of exceptions. I started thinking maybe I should serialize & deserialize the object first. I started thinking I was making this too complicated just to preserve a stack trace.

I finally just started reading the exception code and found they have exactly what I want already baked in… just not exposed. The method only preserves the stack, nothing else… perfect. So why not solve a reflection problem with reflection:

[System.Diagnostics.DebuggerNonUserCode]
[System.Diagnostics.DebuggerStepThrough]
private static void Invoke(MethodInfo method, Object target, params Object[] invokeArgs)
{
	try
	{
		method.Invoke(target, invokeArgs);
	}
	catch (TargetInvocationException te)
	{
		if (te.InnerException == null)
			throw;
		Exception innerException = te.InnerException;

		ThreadStart savestack = Delegate.CreateDelegate(typeof(ThreadStart), innerException, "InternalPreserveStackTrace", false, false) as ThreadStart;
		if(savestack != null) savestack();
		throw innerException;// -- now we can re-throw without trashing the stack
	}
}

The person that made this ‘internal’ should be flogged. How very easy of a solution that is and perfectly safe for all types of exceptions. It appears it will even work with remoting, serialization, cross app domains, etc.

My first request for .Net 5.0:

partial class Exception {
	public void PreserveStackTrace();
}

Updated: Apparently this isn’t a new hack, I should have done some google’ing ;)

Coding, Home, Testing

RE: What’s Holding Back Encryption?

January 18th, 2010

An excerpt of a recent slashdot post:

“… I wanted to ask the Slashdot community, what do you think the hold up is (regarding use of encryption)? Are the existing protocols somehow not good enough? Are the protocols fine, but not supported well enough in software? Is it too complicated to manage the various encryption protocols and keys? Is it ignorance or apathy on the part of the IT community, and that we’ve failed to demand it from our vendors?”

I think I’d go with a few core issues on this one…

First, a large part of the development community is is ignorant of the encryption technologies and how to make them work.

Second, a larger problem is the greed of the existing trusted root authorities. Most individuals are not willing to expend the ridiculous amount of money required just to assert their identity. IMHO it’s an evil exploitation.

Third, and most important. The meat of the problem is ‘identity’. How do I prove who I am? PKI cannot solve this. Why? Because I use several machines to interact with the web not one. If any of these machines, some of which I don’t control, are compromised then some other party can act on my behalf. No machine is secure enough for me to trust it with that kind of control over my online identity.

Thus we are where we are, PKI is expensive and not well understood by most developers. Further, even if this were overcome it would not server as proof of who I am. To truly address the issue a completely new technology/approach is required. I rather doubt we will see a solution in our lifetime.

Coding, Home

When do you Optimize your code?

November 23rd, 2009

The answer is simple… only when the profiler tells you to.

Optimizations often make code less reliable and often constrain implementations making them less flexible. Good software performance is not created by making lots of micro optimizations throughout your code. A good design from the architectural point of view is the critical key to success for performance.

Why would I say this? Why are optimizations bad? Allow me to demonstrate a few types of optimizations that should only be done after careful consideration and profiling:

  1. Lazy Loading: I hate the term as it’s not a lazy approach. Go with the lazy approach and don’t do this until it’s a problem. Writing a property to have strange side-effects on an object is just a bad idea to begin with. If the performance of loading a collection of data is so bad that you must avoid it, then use a method that does not cache the result. Make it clear to callers that the data is not cached but loading with each call. For instance if you want a collection of Students from a Teacher object call the method “FetchStudents()” rather than “GetStudents()”.
  2. Caching: WTF, so many people think this is some kind of great idea. Caching is evil it over complicates code and causes strange side-effects all over your code. Just to be clear I’m not talking about caching of computational results in a data store, I’m talking about caching results of data store requests. Don’t ever do it… ever. A noteworthy exception to the rule is dealing with read-only or write-once data.
  3. Micro Optimizations: This is hard to define but let’s review a few examples on stack overflow. See “Array more efficient than dictionary“, “Are doubles faster than floats“, or “Is if-else faster than switch“. These kinds of optimizations are just silly. Write readable, maintainable, testable code first, then analyse the code with a profile if you have a problem. I loved John Skeet’s response to the first of these questions entitled “Just how spiky is your traffic“.

 

Enough about what not to do… what should you do? Always be aware of cost of the algorithm you are choosing. Choosing the right data structure and/or algorithm at the onset of coding can result in highly predictable and scalable software. Inversely failing to do so can cause non-linear performance as the volume of data increases eventually yielding unusable software. We’ve all seen this happen in a database application with a non-indexed query. This can happen in your own C# code too so don’t be an idiot but don’t spend all your time focused on the fastest approach. Look instead for a predictable linear-growth solution that fits your needs.

Remember there are three competing interests in any piece of code. Flexibility, Reliability, and Performance. Any two of these can be achieved at a reasonable level at the sacrifice of the other, or you can excel at any one at the cost of the other two. The choice is yours to make and will change from task-to-task and project-to-project. I call this the ‘Software Quality Triangle’ because of it’s similarity to the age-old project manager’s triangle (Cost, Time, or Features). By graphically representing each concept as a point of an equilateral triangle and positioning the axis of rotation at it center you can gain perspective of what you will sacrifice over what you will gain. For instance, by rotating the triangle so that Performance is pointing straight up (it’s highest level) the other two points (Flexibility and Reliability) will drop.

Coding, Design, Home

How to correctly implement multi-threading in C#

October 9th, 2009

This ‘helpful’ (using this term loosely) post is for those you considering adding multi-threaded code to your application.  The intended audience is developers with less than mastery-level (10+ years) multi-threading experience.

The first question your up against is:

Do I need multi-threading in my application?

The answer is an emphatic NO you don’t.  If you accept this answer you can quit reading now; otherwise I have a different question for you:

Would you be willing to voluntarily walk across across 100 yards of molten lava, bare foot, carrying a 250lb backpack, uphill, with a blind fold on, all the while ignoring the perfectly good bridge over the lava you simply ignored because it was too easy?

Seriously though, avoid at any cost as much as you can adding multi-threading code.

Common ramifications of threading

  • Most applications that employ threading find a great majority of their bugs in the threading code.
  • Almost always the difficult/impossible to reproduce issues stem from broken threading.
  • More often that not, the application actually performs worse once you start multi-threading.
  • Most of the time threading wasn’t truly *required* in the first place.

If you still think multi-threading isn’t that hard read this article on a simple thing like initializing a static variable in a thread-safe manner:  http://www.yoda.arachsys.com/csharp/singleton.html

So you still want to do multi-threading?

Well, I tried and failed.  Now you need to go learn EXACTLY what the following are used for and when/why you would use them (and generally in this order):

  1. Thread
  2. ThreadPool
  3. ManualResetEvent
  4. AutoResetEvent
  5. EventWaitHandle
  6. WaitHandle
  7. Monitor
  8. Mutex
  9. Semaphore
  10. Interlocked
  11. BackgroundWorker
  12. AsyncOperation
  13. lock Statement
  14. volatile
  15. ThreadStaticAttribute
  16. Thread.MemoryBarrier
  17. Thread.VolatileRead
  18. Thread.VolatileWrite

 

Once you fully understand these your ready write what will prove to be a lot of broken multi-threaded code. Go wild, have fun, and enjoy it… just don’t put it in a product.  Failure is the ultimate, and often only, teacher.  Such is the case with multi-threaded code.  If you want to expedite your failures, use the CHESS program by Microsoft.Some things to consider:

While writing anything thread-safe, ask yourself the following questions:

  1. What can happen if two threads are executing each statement in step with each-other?
  2. Do I really need this lock()?
    1. You need a lock if you are going read AND write to memory that other threads will also access.
    2. Consider using Interlocked instead.
  3. What’s the least amount of code that MUST be in the lock()?
  4. Can I make this object/state immutable and avoid the locking?
  5. What’s the harm if I don’t lock it?
  6. Is there any way for a caller to have code execute inside the lock?
    1. Do not call delegates inside a lock unless you created it.
    2. Do not call methods/properties inside a lock on a class you didn’t create or don’t control.
    3. Never invoke to a Control from within a lock.
  7. Review all members accessed and changed inside a lock for one of these:
    1. Was the variable declared ‘volatile’?
    2. Did I use Thread.VolatileRead/Write?
    3. Did I use Thread.MemoryBarrier?
  8. Are you using System.Threading.Thread.MemoryBarrier correctly?
  9. If multiple locks are involved are they always obtained in the same order?
  10. Don’t use a private member (or expose if you must) to synchronize as this prevents ordering locks.

 
Things you may not need to make thread-safe:

1. Quite commonly you find people locking on initialization of a property when performing a lazy-load:

		private volatile object _data;
		public object Data
		{
			get
			{
				if (_data != null) return _data;
				lock (this)
				{
					if (_data == null)
						_data = new object();
				}
				return _data;
			}
		}

This is usually not necessary since the worst that happens is the data is loaded twice.  It depends greatly on what that object is and can do, but usually it’s safe to load two instances.  Think about it without concern for performance (the likely-hood is too small to be a performance impact). If you can load/create the data twice without causing issue then just do this instead:

		public object Data
		{
			get { return _data ?? _data = new object(); }
		}

2. Consider pre-fetching instead of lazy loading with shared state.  This can then be performed in the constructor which is only on a single thread.

3. Immutable objects (objects that do not change state) do not normally need locks to access.

4. Fire-and-forget objects work best, start a task and don’t wait for it.  You defeat the purpose of multi-threading with synchronization.

And finally, yes you should test it but don’t depend upon it. Get the whole team together if needed to do a line-by-line walk of the code. I’m sure I’ve missed a lot of things here, but this should be a good start.

Coding, Home

How to use System.Diagnostics.Process correctly

October 7th, 2009

I’ve seen many a question on stackoverflow and other places about running a process and capturing it’s output. Using the System.Diagnostics.Process correctly is not easy and most often it’s done wrong.

Some common mistakes with System.Diagnostics.Process:

1. Not capturing both output streams (error & output)
2. Not redirecting input can cause applications to hang
3. Not closing redirected input can cause applications to hang
4. Using OutputDataReceived/ErrorDataReceived without waiting for null
5. Not checking for null in OutputDataReceived/ErrorDataReceived handlers
6. Forgetting to set EnableRaisingEvents = true; when using the Exited event
7. Forgetting ErrorDialog, CreateNoWindow, or UseShellExecute settings
8. Incorrect handling of StandardOutput or StandardError stream readers

So with this said, here are some basic guidelines:

1. Use the OutputDataReceived/ErrorDataRecieved events NOT the StandardOutput or StandardError. This will save you a lot of headache and needless thread management.
2. Always capture all output AND input, if you don’t plan to provide input, close the stream immediately.
3. Your process isn’t done until it exited AND you have read all the data. OutputDataReceived CAN AND WILL be fired after a call to WaitForExit() returns. You will need wait handles for each output stream and set the wait handle once your receive (null) data.

To see how this done correctly review CSharpTest.Net.Processes. ProcessRunner.

Read more on Using the ProcessRunner class.

Coding, Home