C# Encryption check

Security vulnerabilities tend to lie either in the cryptosystem (i.e. the algorithms used – the part that has already been implemented in the .NET Framework), or in the overall use of the encrypted data by the cryptographic system (e.g. storage policies and communication protocols). There's not much to go on there because all you've done is used a few pre-implemented algorithms to encrypt some arbitrary data :)

One suggestion I could make, though, is that you make use of using blocks when dealing with IDisposable objects, such as streams.
 
Last edited:
Only thing I would say is have all the file / file stream etc in a 'finally' code block. Everything else looks fine (although im not an encryption guru) :)
 
Goksly said:
Only thing I would say is have all the file / file stream etc in a 'finally' code block. Everything else looks fine (although im not an encryption guru) :)
Click to expand...

That's done automatically if you put them inside a using block. Should look something like this (taken from my own code):

Code: 
public byte[] Encrypt(byte[] plainText)
{
    using (ICryptoTransform encryptor = m_Rijndael.CreateEncryptor())
    using (MemoryStream memoryStream = new MemoryStream())
    using (CryptoStream cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write))
    {
        cryptoStream.Write(plainText, 0, plainText.Length);
        cryptoStream.FlushFinalBlock();
        return memoryStream.ToArray();
    }
}

Another couple of suggestions to make it more concise:
  • RijndaelManaged has a GenerateIV method and IV property; use them instead of generating your own IV.
  • Rfc2898DeriveBytes can generate a random salt for you; let it do this to minimise unnecessary code.
 
Last edited:
I thought the using block just called Dispose on the object and didnt do anything with exception handling / calling Dispose when an exception is thrown?
 
Goksly said:
I thought the using block just called Dispose on the object and didnt do anything with exception handling / calling Dispose when an exception is thrown?
Click to expand...

Nope, a using block like this:
Code: 
using (Foo bar = new Foo())
{
    bar.Baz();
}

...is syntactic sugar for this:
Code: 
Foo bar = new Foo();
try
{
    bar.Baz();
}
finally
{
    bar.Dispose();
}
 
Actually, if you want to be really pedantic it should really be this:
Code: 
{
    Foo bar = new Foo();
    try
    {
        bar.Baz();
    }
    finally
    {
        bar.Dispose();
    }
}

:p
 
Inquisitor said:
Security vulnerabilities tend to lie either in the cryptosystem (i.e. the algorithms used – the part that has already been implemented in the .NET Framework), or in the overall use of the encrypted data by the cryptographic system (e.g. storage policies and communication protocols). There's not much to go on there because all you've done is used a few pre-implemented algorithms to encrypt some arbitrary data :)

One suggestion I could make, though, is that you make use of using blocks when dealing with IDisposable objects, such as streams.
Click to expand...

Thanks, so far I am just using it to encrypt an object storing data (like a list) then storing the serialized EncryptedObject to the disk. The password would always be entered manually into a textbox in a form then sent to the function.
I can't see anyway to secure the data or password when it is held in memory (SecureString seems like a real pain/impossible to use for most situations)

It was just making sure what I have done is reasonably secure. Like if encrypting serialized objects might be a bad idea, or (like I have seen in some tutorials) generating the IV from the password. Or using PasswordDerivedBytes instead of Rfc2898DeriveBytes.

However I didn't realise you could have multiple using statements for one scope, that is really nice :D
 
Vai said:
Thanks, so far I am just using it to encrypt an object storing data (like a list) then storing the serialized EncryptedObject to the disk. The password would always be entered manually into a textbox in a form then sent to the function.
I can't see anyway to secure the data or password when it is held in memory (SecureString seems like a real pain/impossible to use for most situations)

It was just making sure what I have done is reasonably secure. Like if encrypting serialized objects might be a bad idea, or (like I have seen in some tutorials) generating the IV from the password. Or using PasswordDerivedBytes instead of Rfc2898DeriveBytes.

However I didn't realise you could have multiple using statements for one scope, that is really nice :D
Click to expand...

Well ultimately, the plaintext has to be in memory at some point, otherwise you won't be able to do anything with it. The problem with keeping it secure is how long it stays in memory and how it is manipulated once there. Unfortunately, since the CLR is a managed environment, you have no control over exactly what goes on on the managed heap (i.e. the block of memory managed by the CLR). Objects may be moved around on the heap, leaving residue where they used to be, and you have no control over when objects are removed (besides, the memory is not actually guaranteed to be 'erased' – until something takes its place, the original data remain there). In addition to this, .NET's String class is immutable, so modifying a string only creates a copy, leaving the old one in memory until garbage collected.

Consequently, if it is to be secure, the plaintext must be stored on the unmanaged heap, where you have direct control over it. Essentially, what you want to do is allocate the memory, put the plaintext there, operate on the plaintext, and then overwrite it immediately afterwards so as to minimise the possibility of someone reading the memory.

This is all fine and dandy, except you can't use any kind of managed data structure (specifically reference types, e.g. strings, arrays, etc.) if you're dealing with the unmanaged heap; you must instead work with raw binary, either via the Marshal class or using pointers, which requires unsafe code. (For the purposes of secure representations of objects, any value type can also be used, as these are stored on the stack, which is not managed, unlike the heap, but can also be stored on the unmanaged heap and accessed by way of pointers).

Because of this, you're very limited in what you can do with secure representations of objects in the FCL, as the vast majority of it uses only managed types. In the case of PasswordDerivedBytes and Rfc2898DeriveBytes, the password must be specified as either a string or an array of bytes (both of which are allocated on the managed heap).

That said, however, you really shouldn't worry about having the password in memory unless you're deploying your application in an environment that you do not trust, and even then you'd be being overly paranoid unless the data you're trying to protect are very sensitive (which in an untrustworthy environment would be a very unusual scenario anyway).

That was probably a huge waste of typing and brain power, but hey, I found it interesting :p
 
Last edited:
It's important not to over-engineer... something like that would be used for a serious application (bank software etc) but if he just wants a user name and password for a school project it would be very ott! :)
 
NathanE said:
Or you could just use the System.Security.SecureString class...
Click to expand...

The problem is that there's not enough support in the FCL for this to be useful. At some point along the line you'll have to convert it to a normal string or array of bytes if you want it to use it with with FCL classes, unless you want write your own code that can use it without having to convert it to such a managed type.

This is exactly why SecureString has no methods that allow you to retrieve the plaintext string directly (and why you can only add to it with individual characters); it would defeat the point in using the SecureString class in the first place. Rather, you're supposed to use something like Marshal.SecureStringToGlobalAllocUnicode, which writes its contents to unmanaged memory as a null-terminated UTF-16 string and gives you a pointer to the first byte.
 
Last edited:
You must log in or register to reply here.
Back
Top Bottom