patterncsharpMinor
Simplified secure encryption of a String
Viewed 0 times
simplifiedsecureencryptionstring
Problem
I have two code examples that I wrote for best practices encrypting a string that I'm looking for feedback both in terms of best practices and security. They are both using authenticated encryption. One is using the bouncy castle API to do AES-GCMand the other is for the limitation of only using the built in .NET API so it does encryption then authentication separately.
Each example has the ideal API of passing the key as a byte array (because it was randomly generated).
However each has a less secure helper method that uses a string password to derive the bytes for the key using PBKDF2 with salt and iterations. The salt is passed in the clear using the authenticated payload.
Bouncy Castle AES-GCM
```
/*
* This work (Modern Encryption of a String C#, by James Tuley),
* identified by James Tuley, is free of known copyright restrictions.
* https://gist.github.com/4336842
* http://creativecommons.org/publicdomain/mark/1.0/
*/
using System;
using System.IO;
using System.Text;
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.Crypto.Engines;
using Org.BouncyCastle.Crypto.Generators;
using Org.BouncyCastle.Crypto.Modes;
using Org.BouncyCastle.Crypto.Parameters;
using Org.BouncyCastle.Security;
namespace Encryption
{
public static class AESGCM
{
private static readonly SecureRandom Random = new SecureRandom();
//Preconfigured Encryption Parameters
public static readonly int NonceBitSize = 128;
public static readonly int MacBitSize = 128;
public static readonly int KeyBitSize = 256;
//Preconfigured Password Key Derivation Parameters
public static readonly int SaltBitSize = 128;
public static readonly int Iterations = 10000;
public static readonly int MinPasswordLength = 12;
///
/// Helper that generates a random new key on each call.
///
///
public static byte[] NewKey()
{
var key = new byte[KeyBitSize / 8];
Random.NextBytes(key);
return key;
}
///
/// S
Each example has the ideal API of passing the key as a byte array (because it was randomly generated).
However each has a less secure helper method that uses a string password to derive the bytes for the key using PBKDF2 with salt and iterations. The salt is passed in the clear using the authenticated payload.
Bouncy Castle AES-GCM
```
/*
* This work (Modern Encryption of a String C#, by James Tuley),
* identified by James Tuley, is free of known copyright restrictions.
* https://gist.github.com/4336842
* http://creativecommons.org/publicdomain/mark/1.0/
*/
using System;
using System.IO;
using System.Text;
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.Crypto.Engines;
using Org.BouncyCastle.Crypto.Generators;
using Org.BouncyCastle.Crypto.Modes;
using Org.BouncyCastle.Crypto.Parameters;
using Org.BouncyCastle.Security;
namespace Encryption
{
public static class AESGCM
{
private static readonly SecureRandom Random = new SecureRandom();
//Preconfigured Encryption Parameters
public static readonly int NonceBitSize = 128;
public static readonly int MacBitSize = 128;
public static readonly int KeyBitSize = 256;
//Preconfigured Password Key Derivation Parameters
public static readonly int SaltBitSize = 128;
public static readonly int Iterations = 10000;
public static readonly int MinPasswordLength = 12;
///
/// Helper that generates a random new key on each call.
///
///
public static byte[] NewKey()
{
var key = new byte[KeyBitSize / 8];
Random.NextBytes(key);
return key;
}
///
/// S
Solution
As far as best practices go, I see a few offhand:
One more thing I just noticed:
static public XXXis idiomatically written aspublic static XXX.
- Since all your methods are
static, the class should be madestaticas well to prevent (useless) instances from being created.
- Are your bit size numbers near the top of the classes intended to be modified? If so, make them
privateand havepublicproperties to access them. If not, make themconst.
One more thing I just noticed:
Rfc2898DeriveBytes inherits from a class which implements IDisposable in .NET 4 and beyond. So its use should be wrapped in a using block if you're targeting that version of the framework. .NET 2 (and 3.0/3.5), don't worry about it.Context
StackExchange Code Review Q#14892, answer score: 7
Revisions (0)
No revisions yet.