patterncsharpMinor
Securing a file encryption(ECIES) protocol using Scrypt + Inferno
Viewed 0 times
encryptioneciesfileprotocolsecuringinfernoscryptusing
Problem
I'm evaluating the security level of a file encryption app, which implements Scrypt and Inferno, in an ECIES context. The encrypted files are either stored on the user's PC/laptop, either sent with gmail. I must reach a security level appropriate for files containing sensitive data like credit card number, medical infos etc... Nowadays about any hacker can intercept an email, and a laptop can be stolen or seized, so I'm assuming that about anyone could have physical access to the encrypted file(s).
The workflow is: (assuming pre-distribution of public keys)
The current implementation works well on the surface, but in the given context, what should I improve/change for even better security?
DHM key storage/retrieval:
```
const int _saltSize = 48; // 16 is often recommended, better safe than sorry
const int _keySize = 48;
const int _iterations = 1048576; // maybe randomize + store?
private static byte[] Hash(string password, int iterations, byte[] salt = null)
{
var pass = password.ToBytes();
var saltAndHash = new byte[0];
if (salt == null) // if encrypting, store salt
{
salt = new CryptoRandom().NextBytes(_saltSize);
saltAndHash = Utils.Combine(saltAndHash, salt);
}
int blockSize = 8; // maybe 16 with less iterations considering modern GPUs?
int parallel = 1;
return Utils.Combine(saltAndHash, SCrypt.ComputeDerivedKey(pass, salt, iterations, blockSize, parallel, null, _keySize));
}
internal static void SaveMasterKey(Keyring k, string password, string file)
{
// Keyring is simply holding the session keys in CngKey props
var hashed = Hash(pas
The workflow is: (assuming pre-distribution of public keys)
- start app on user PC/laptop(offline, no server).
- create/load master key pair to/from file + encrypt/decrypt using hashed password.
- create/load a file to work on.
- ETM with receiver's public key.
- send as gmail attachment(or store on PC if receiver is sender)
The current implementation works well on the surface, but in the given context, what should I improve/change for even better security?
DHM key storage/retrieval:
```
const int _saltSize = 48; // 16 is often recommended, better safe than sorry
const int _keySize = 48;
const int _iterations = 1048576; // maybe randomize + store?
private static byte[] Hash(string password, int iterations, byte[] salt = null)
{
var pass = password.ToBytes();
var saltAndHash = new byte[0];
if (salt == null) // if encrypting, store salt
{
salt = new CryptoRandom().NextBytes(_saltSize);
saltAndHash = Utils.Combine(saltAndHash, salt);
}
int blockSize = 8; // maybe 16 with less iterations considering modern GPUs?
int parallel = 1;
return Utils.Combine(saltAndHash, SCrypt.ComputeDerivedKey(pass, salt, iterations, blockSize, parallel, null, _keySize));
}
internal static void SaveMasterKey(Keyring k, string password, string file)
{
// Keyring is simply holding the session keys in CngKey props
var hashed = Hash(pas
Solution
The biggest issues I see are some conformity/readability issues.
For example, the following
Please don't do that. It makes it significantly harder to follow. If you want to omit braces I understand but don't inline them like that.
I don't know what
For example, the following
if/else structure:if (forSender) ephemeralBundle = k.SenderDHM.GetSharedEphemeralDhmSecret();
else ephemeralBundle = k.ReceiverDHM.GetSharedEphemeralDhmSecret();Please don't do that. It makes it significantly harder to follow. If you want to omit braces I understand but don't inline them like that.
var ephemeralPublic = new byte[104];
using (var fs = new FileStream(file, FileMode.Open, FileAccess.Read))
{
fs.Read(ephemeralPublic, 0, 104);
var ephemeralSymmetric = k.SenderDHM.GetSharedDhmSecret(ephemeralPublic.ToPublicKeyFromBlob());
if (Authenticate(file, ephemeralSymmetric, 104))
{
using (var etm = new EtM_DecryptTransform(ephemeralSymmetric))
using (var cs = new CryptoStream(fs, etm, CryptoStreamMode.Read))
{
var decrypt = new byte[fs.Length - 104];I don't know what
104 means, but you use it a lot and now to me it's very magic. Try to get it in a const that reflects the meaning of it.Code Snippets
if (forSender) ephemeralBundle = k.SenderDHM.GetSharedEphemeralDhmSecret();
else ephemeralBundle = k.ReceiverDHM.GetSharedEphemeralDhmSecret();var ephemeralPublic = new byte[104];
using (var fs = new FileStream(file, FileMode.Open, FileAccess.Read))
{
fs.Read(ephemeralPublic, 0, 104);
var ephemeralSymmetric = k.SenderDHM.GetSharedDhmSecret(ephemeralPublic.ToPublicKeyFromBlob());
if (Authenticate(file, ephemeralSymmetric, 104))
{
using (var etm = new EtM_DecryptTransform(ephemeralSymmetric))
using (var cs = new CryptoStream(fs, etm, CryptoStreamMode.Read))
{
var decrypt = new byte[fs.Length - 104];Context
StackExchange Code Review Q#149451, answer score: 8
Revisions (0)
No revisions yet.