Coding Sanity

Right, now that we've got the basics of reliability covered, we can move on to SafeHandle. If you've ever done any interop you should know all about handles. They're effectively pointers to internal OS resources. Pretty much everything you do in Windows has a handle involved somewhere. If you open a file, the OS gives you a file handle that you pass into functions that read/write etc from that file. If you have a window it's got a handle, and it's device context has a handle too. Happily enough, .NET hides all this from you.

Sometimes, however, you need some functionality that the .NET classes just don't offer. When this happens you inevitably need to use and abuse these Windows handles. The problem is that many people don't close the handles properly, and even those that do often don't handle error conditions gracefully, ensuring that the handle is closed. The problem is that losing handles is effectively a memory leak. The reason for this is that many handles actually represent data stored in the kernel or UI subsystem about the thing you're messing with. If you don't close that handle, that memory may not be released. In addition it may not be easy to notice since it's kernel memory not app memory.

Luckily the OS is pretty good at shutting down handles used by processes once they finish. All well and good unless you're running a high-availability service. Worse if you're using these handles inside something like SQL Server. So, what's the solution here? Well, ideally you'd go off and write a disposable class using critical finalizers to control access to your handle. Now, even in the dire circumstances of a massive failure in your code or environment, your handle will likely be released. Bit of a pain for a little extra reliability though.

Enter SafeHandle

So, MS have provided us with a little class that does all that for you. It's called SafeHandle, and can be found in the System.Runtime.InteropServices namespace. It inherits from CriticalFinalizerObject, and places reliability guarantees on most of it's methods. The idea is that you wrap your handle inside a SafeHandle subclass, and it will ensure that it gets cleaned up OK. There are two subclasses of SafeHandle (which are also abstract) called SafeHandleMinusOneIsInvalid, and SafeHandleZeroOrMinusOneIsInvalid. They can be found in the Microsoft.Win32.SafeHandles namespace.

The reason for these subclasses is the wacky world of Windows handles. When a handle is returned from a function, sometimes 0 is returned to indicate failure, and sometimes -1 is given for a failure. Depending on the type of handle their failure indicators could be different. Basically these subclasses of SafeHandle just override the IsInvalid property of SafeHandle with the relevant check.

So, what's required when implementing a SafeHandle or one of it's derived subclasses? Basically all you have to do is implement the ReleaseHandle method. Since SafeHandle doesn't know what kind of handle it's dealing with, it doesn't know how to close it, so you have to provide it that logic. ReleaseHandle implements a ReliabilityContract, so you have to stick to the limitations I discussed in my earlier post.

Type Safety & Interop

Another thing about handles is that since they're generally (up to now) passed around as IntPtrs, there's no type safety. Nothing stops you from passing a window handle to a function expecting a file handle. Oh, your code will fail, but it'll be a runtime failure, not compile time. Interop has a great new feature where you can declare an interop method as taking a SafeHandle. Interop will know to actually cast it to the relevant underlying HANDLE type. This allows you to declare your interop methods in a type safe manner.

As an example, I wanted to be notified when a certain registry key changed. Windows has a function called RegNotifyChangeKeyValue where it can block until a change occurs, or signal an event. Unfortunately the RegistryKey class doesn't offer this functionality. So I created a SafeRegistryHandle class that inherited from SafeHandleZeroOrMinusOneIsInvalid, and put a call to RegCloseKey in the ReleaseHandle override. Then I created an interop declaration on my class that looked like this:

[DllImport("advapi32.dll", SetLastError = true)]
private static extern int RegNotifyChangeKeyValue(SafeRegistryHandle hKey, bool watchSubtree, int dwNotifyFilter, SafeWaitHandle hEvent, bool fAsynchronous);

So now I have a type-safe declaration for RegNotifyChangeKey, and I'm assured that the handle I've opened will be closed (given the caveats for Constrained Execution Regions).

If you go and look in Reflector you'll see that the RegistryKey class uses a SafeRegistryHandle class itself. Unfortunately it's an internal class so I had to reinvent the wheel a touch, but the point is that this technique of using SafeHandles should be your method of choice when dealing with OS handles.

Please go and have a look at this article on putting connection strings into the query string. The author, Peter Bromberg Ph.D. is an MVP who has worked in the banking and financial industry for 20 years.

So what exactly is wrong with this?

Never trust user input

This is one of the cardinal tenets of security. What we're doing here is using untrusted user input to connect to a database. Let's look at some examples of what we can do. First off, let's say that the web site is going to put sensitive information into the database. Well, easy, we just give it a connection string for a box which we own and voila the sensitive information is on our box.

If the web server connects to the DB whilst in a trusted user account, and we pass through a connection string containing "Integrated Security=true", then we now have elevated privileges. So, if the web page had a SQL Injection problem, we can now play around to our hearts content.

Sniffing

Now, let's say that I'm running a packet sniffer. If I'm able to see the traffic to the box coming past, I can get this connection string and use it for myself. I'm willing to bet you that someone, somewhere will pass me the sa password this way. Yippee! I now 0wn your SQL Server.

Encryption?

Okay, so we've looked at a very few of the reasons why having the connection string in the query string is a VBI (Very Bad Idea). Now let's have a look at the "encryption" Peter uses. I do not like his naming his class and methods with "Encryption" in them, since they are nothing of the sort. Encoding is a better term for this. You know, like base64 encoding, or UTF8 and so forth.

Encoding is better because it doesn't imply that there's encryption involved here, since there isn't. Why do I say that? Well, let's start with the "encryption" methods Peter looked at:

XOR Encryption

He rightly rejected this one. We take the plaintext, and our key and XOR them together. So, for example if our key was "hello world", and our plaintext was "server=mine", we'd take the ASCII of "s" and XOR that with the ASCII of "h", which would give us an apostrophe. The second letter would give us a bit of a problem though, it would be "e" XOR "e" which would be NULL, ending our string right there. In any case this encryption method is "trivially simple" to crack. It does have one advantage though, it's a proper encryption method, since in theory at least the key is required to decrypt. Of course, with even a large random key, it's easy to brute force this one.

ROT13 Encryption

This involves treating the alphabet as a loop (so after "Z" is "A"), and simply shifting each character right 13 places. Thus "A" becomes "N", and "B" becomes "O". Nicely though, simply reapplying the cipher gives you the cleartext. In essence, to crack this code requires one CPU instruction per letter. Nice and quick to encrypt and decrypt. It belongs to a class of ciphers known as "Caeser ciphers" or "shift ciphers", yep, that's right it's a cipher dating back to before 44 BC. ROT13 is about as secure as writing the plaintext. However, it is at least a cipher, if a spectacularly poor one.

ROT39 Encryption

Same as ROT13, but expanded to use the ASCII character set from 48 "0" to 125 "}". Just as easy to crack, slightly more difficult to identify with the naked eye.

ASCII to Hex "Encryption"

This is the "encryption" method Peter settled on. The reason I put encryption in quotes is that this is not an encryption method, it's an encoding method. In .NET just take each 2 character piece, and call byte.Parse("63", NumberStyles.HexNumber) to get "c". If you look at his code this is exactly what he is doing. It's plaintext, just not "normal" plaintext. It's worse than even ROT13 for goodness sakes!

Conclusion

Now Peter does say in his article that this is obfuscation rather than encryption. He then suggests that proper encryption (e.g. Triple-DES) could be used, but goes on to say that this would be "overkill". I dunno, passing connection strings around on a query string suggests a VERY strong need for overkill to me. If I had to pass such sensitive data around in the querystring, I'd...

Well, I wouldn't. Flat out no. No matter what encryption, no matter what security, nu-uh, no way. To be fair to Peter he does point out that "it's best not to put sensitive data on the querystring at all". Good, so why suggest it in the newsgroups, and why write an article showing exactly how to do that? Especially one with far too few caveats on the dangers IMO.

Please, please, please, do NOT follow the suggestions in his article. That's not to say you shouldn't use his code, nor obfuscate stuff on the querystring, please do; just not sensitive data. You could perhaps obfuscate things like whether the user is in edit mode or not, or whether to display comments or not. It should not be used for anything that is trusted. If he'd strongly pointed out that, I would be a whole lot happier with his article.