patterncsharpMinor
Rijndael compression/encryption class
Viewed 0 times
compressionclassencryptionrijndael
Problem
Out of personal interest and as a learning exercise I've written a C# class (.NET 4) to perform encryption/decryption of a file along with some compression upon encryption. Most of my understanding of this has been based on web research so I'd like some feedback on coding style, optimizations and, if any cryptography experts out there, the security of this approach.
```
using System;
using System.IO;
using System.IO.Compression;
using System.Security.Cryptography;
using System.Text;
namespace FileCrypt {
class Cypto {
private const int SALTSIZE = 16;
private const int HASHSIZE = 20;
private const int EXTENSIONSIZE = 32;
private const int BITSPERBYTE = 8;
private const int ITERCOUNT = 32767;
///
/// compress and encrypt file into target with password protection
///
///
///
///
///
public static void Encrypt( FileInfo inFile, FileInfo outFile, string password ) {
try{
Rfc2898DeriveBytes keyGenerator = new Rfc2898DeriveBytes(password, SALTSIZE, ITERCOUNT);
Rijndael rijndael = Rijndael.Create();
rijndael.Padding = PaddingMode.PKCS7;
rijndael.Mode = CipherMode.CBC;
rijndael.IV = keyGenerator.GetBytes(rijndael.BlockSize / BITSPERBYTE);
rijndael.Key = keyGenerator.GetBytes(rijndael.KeySize / BITSPERBYTE);
using(var fileStream = outFile.Create()) {
byte[] salt = keyGenerator.Salt;
byte[] fileExtension = Encoding.ASCII.GetBytes(Convert.ToBase64String(Encoding.ASCII.GetBytes(inFile.Extension)));
byte[] paddedExtension = new byte[EXTENSIONSIZE];
if(fileExtension.Length > EXTENSIONSIZE) {
throw new Exception("File extension is too long for allocated memory. " +
"Consider compressing the file to a zip archive
```
using System;
using System.IO;
using System.IO.Compression;
using System.Security.Cryptography;
using System.Text;
namespace FileCrypt {
class Cypto {
private const int SALTSIZE = 16;
private const int HASHSIZE = 20;
private const int EXTENSIONSIZE = 32;
private const int BITSPERBYTE = 8;
private const int ITERCOUNT = 32767;
///
/// compress and encrypt file into target with password protection
///
///
///
///
///
public static void Encrypt( FileInfo inFile, FileInfo outFile, string password ) {
try{
Rfc2898DeriveBytes keyGenerator = new Rfc2898DeriveBytes(password, SALTSIZE, ITERCOUNT);
Rijndael rijndael = Rijndael.Create();
rijndael.Padding = PaddingMode.PKCS7;
rijndael.Mode = CipherMode.CBC;
rijndael.IV = keyGenerator.GetBytes(rijndael.BlockSize / BITSPERBYTE);
rijndael.Key = keyGenerator.GetBytes(rijndael.KeySize / BITSPERBYTE);
using(var fileStream = outFile.Create()) {
byte[] salt = keyGenerator.Salt;
byte[] fileExtension = Encoding.ASCII.GetBytes(Convert.ToBase64String(Encoding.ASCII.GetBytes(inFile.Extension)));
byte[] paddedExtension = new byte[EXTENSIONSIZE];
if(fileExtension.Length > EXTENSIONSIZE) {
throw new Exception("File extension is too long for allocated memory. " +
"Consider compressing the file to a zip archive
Solution
Overall it looks fine. You use constants and meainingful names and you encapsulate features properly but here and there, there are some small flaws. Let's take a look...
This needs to be disposed.
This needs to be disposed too. Actually you can test everthing for the
For this you should create a new exception like
You can then put the message there and just throw it without writing the message every time. You should avoid throwing the
The same here.
You can throw away the
This is useless. You can remove the entire
Rfc2898DeriveBytes keyGenerator = new Rfc2898DeriveBytes(password, SALTSIZE, ITERCOUNT);This needs to be disposed.
Rijndael rijndael = Rijndael.Create();This needs to be disposed too. Actually you can test everthing for the
Dispose method when working with encryptions and streams. There is much more then that which is currently not disposed.throw new Exception("File extension is too long for allocated memory. " +
"Consider compressing the file to a zip archive prior to encryption.");For this you should create a new exception like
class FileExtensionLengthOutOfRangeException : Exception
{
public override string Message => "File extension is too long for allocated memory. Consider compressing the file to a zip archive prior to encryption."
}You can then put the message there and just throw it without writing the message every time. You should avoid throwing the
Exception.throw new Exception("Incorrect password entered.");The same here.
InvalidPasswordException would be much more appropriate and alone by the name of the exception you'd know what's wrong. What more you'll be able to catch it with catch(InvalidPasswordException ex) if you need to.using (FileStream outStream = new FileStream(outFile.FullName, FileMode.OpenOrCreate, FileAccess.Write))
{
using (CryptoStream cryptoStream = new CryptoStream(sourceStream, rijndael.CreateDecryptor(), CryptoStreamMode.Read))
{
using (GZipStream gzipStream = new GZipStream(cryptoStream, CompressionMode.Decompress))
{
gzipStream.CopyTo(outStream);
}
}
}You can throw away the
{} of adjacent usings:using (FileStream outStream = new FileStream(outFile.FullName, FileMode.OpenOrCreate, FileAccess.Write))
using (CryptoStream cryptoStream = new CryptoStream(sourceStream, rijndael.CreateDecryptor(), CryptoStreamMode.Read))
using (GZipStream gzipStream = new GZipStream(cryptoStream, CompressionMode.Decompress))
{
gzipStream.CopyTo(outStream);
}catch
{
throw;
}This is useless. You can remove the entire
try/catch block and nothing would change.Code Snippets
Rfc2898DeriveBytes keyGenerator = new Rfc2898DeriveBytes(password, SALTSIZE, ITERCOUNT);Rijndael rijndael = Rijndael.Create();throw new Exception("File extension is too long for allocated memory. " +
"Consider compressing the file to a zip archive prior to encryption.");class FileExtensionLengthOutOfRangeException : Exception
{
public override string Message => "File extension is too long for allocated memory. Consider compressing the file to a zip archive prior to encryption."
}throw new Exception("Incorrect password entered.");Context
StackExchange Code Review Q#158632, answer score: 5
Revisions (0)
No revisions yet.