Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I need to lock on my AesManaged instance _Aes to call CreateDecryptor() however the CreateDecryptor() method is not thread safe and this function could be called by multiple threads. Is this the correct way to create a disposable object while needing to use a lock on the parent creating the object?

byte[] encryptedTextBlob = GetEncryptedTextBlob();

ICryptoTransform decryptor = null;
lock(_Aes)
{
    decryptor = _Aes.CreateDecryptor();
}

using (decryptor)
using (var encryptedStream = new MemoryStream(encryptedTextBlob))
using (var decrypedStream = new CryptoStream(encryptedStream, decryptor, CryptoStreamMode.Read))
using (var textReader = new StreamReader(decrypedStream))
{
    return textReader.ReadToEnd();
}
share|improve this question

2 Answers 2

up vote 4 down vote accepted

I would say you're not using AES correctly - don't hold on to an instance of it in your class (guessing by the underscore in the name) but rather create it as needed, while holding AES's parameters (the key and IV) in your class:

using (var aes = new AesManaged() { IV = _IV, Key = _Key })
using (var decryptor = aes.CreateDecryptor())
using (var encryptedStream = new MemoryStream(encryptedTextBlob))
using (var decrypedStream = new CryptoStream(encryptedStream, decryptor, CryptoStreamMode.Read))
using (var textReader = new StreamReader(decrypedStream))
{
    return textReader.ReadToEnd();
}
share|improve this answer
    
I am getting the data from a 3rd party source that has a fixed IV and Key, that is why I used only the one copy of AesManaged, I thought creating all those copies of AesManaged all to be identical would be expensive. –  Scott Chamberlain Jan 25 '13 at 21:11
    
Understood, I will use that way. I have asked a new question without all of that Aes stuff in the way as I still would like to know the answer to my original question in case I need to use it in another situation. –  Scott Chamberlain Jan 25 '13 at 22:02

Jesse C. Slicer's answer addresses your main concern, but after seeing those multiple using blocks, you might be interested to know that you can create multiple variables in a single using statement. I think this is a bit easier to read, personally:

using (var aes = new AesManaged() { IV = _IV, Key = _Key },
       var decryptor = aes.CreateDecryptor(),
       var encryptedStream = new MemoryStream(encryptedTextBlob),
       var decrypedStream = new CryptoStream(encryptedStream, decryptor, CryptoStreamMode.Read),
       var textReader = new StreamReader(decrypedStream))
{
    return textReader.ReadToEnd();
}
share|improve this answer
1  
Interesting, I didn't know you could do this. But personally, I think multiple usings are better, because that way, changing the code is easier. –  svick Jan 25 '13 at 19:16
1  
Awesome. I love this. Addresses the primary concern I had with Jessie's answer (which is great, and readable, but I don't like leaving out braces...in fact, they're required by our coding standards.) –  Beska Jan 25 '13 at 21:49
    
@Beska the best part is it's been part of the standard for a long time: msdn.microsoft.com/en-us/library/yh598w02(v=vs.71).aspx –  Reacher Gilt Jan 25 '13 at 22:11
1  
@svick I see your point, but I think it would be too easy to accidentally an inject somewhere in the using cascades -- still totally valid c#, but altering the meaning of the program: –  Reacher Gilt Jan 25 '13 at 22:18
    
@ReacherGilt Unfortunately, this doesn't actually work in older .NET versions. Yes, you can do multiple usings seperated by commas back in VS 2003, like in your example and in the link...but it seems the types have to be the same, which limits the usefulness considerably. I assume this problem isn't encountered in later versions because of the user of the var type for all the variables in your example. –  Beska Jan 29 '13 at 20:24

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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