robinrodricks / FluentStorage

A polycloud .NET cloud storage abstraction layer. Provides Blob storage (AWS S3, GCP, FTP, SFTP, Azure Blob/File/Event Hub/Data Lake) and Messaging (AWS SQS, Azure Queue/ServiceBus). Supports .NET 5+ and .NET Standard 2.0+. Pure C#.
MIT License
263 stars 33 forks source link

Encryption for stored blobs fails to decrypt #35

Closed dammitjanet closed 12 months ago

dammitjanet commented 12 months ago

Ok - this is an issue I had with Storage.Net and recently with Fluent.Storage

My use case is that I need to store sensitive files in an encrypted state, even though the blob storage I use will be completely private, just for a 2nd level encryption to satisfy the customers Information Governance. Anyway not being an encryption guru, it's taken me a bit of time to work out the issue, and it's a long standing one going back to the original implementation.

Since most of the time we will be using Dependency Injection (DI) to instantiate our objects, each time the SymmetricEncryptionSink (or in my case the AesSymmetricEncryptionSink) is created, the IV is set

        public AesSymmetricEncryptionSink(string key) {
            _cryptoAlgorithm = Aes.Create();
            _cryptoAlgorithm.Key = Convert.FromBase64String(key);
            _cryptoAlgorithm.GenerateIV();
        }

This means that if a blob is stored, then it can only be unencrypted using the same instance of the Sink or with an instance of the Sink with the same IV (secret key)

So if you have the case (and this looks to be quite important given that we are pushing to AWS/Azure storage, then if you try and retrieve the blob at another time, you can't read the blob even though the original key might be the same, the secret generated at the time of encryption is no longer the secret that the sink is trying to use the decrypt.

Almost every instance of the use of Storage.Net has got this wrong and has used a pattern like this, which creates a transient (I think) and not a singleton, this one is from Elsa that doesn't use encryption

builder.Services.Configure<BlobStorageWorkflowProviderOptions>(options => options.BlobStorageFactory = () => StorageFactory.Blobs.DirectoryFiles(Path.Combine(builder.Environment.ContentRootPath, "Workflows")));

So there are previous examples see that I found, and in that instances its basically the issue described above.

There are two solutions I think.

1) Add additional parameter constructors with (key and secret) to the existing encryption sinks (and associated extension methods) 2) document how to create a singleton with DI that will last for the application lifetime (useful for local caching that can be discarded) 3) document 1) and show the extended lifetime and how this impacts the generation of the keys

for my use case (which is in fact 2) above

builder.Services.AddSingleton<IBlobStorage>(StorageFactory.Blobs
        .DirectoryFiles(Path.Combine(builder.Environment.ContentRootPath, "Cache"))
        .WithAesSymmetricEncryption(CryptoSeed.GenerateNewKey()));
dammitjanet commented 12 months ago

I'm got a PR ready to address most of these concerns along with unit tests

robinrodricks commented 12 months ago

Awesome!

dammitjanet commented 12 months ago

PR done, there is a fair bit in there

if you think there is any mileage in it, you can mail me at my username@me.com I've got some ideas for a roadmap

robinrodricks commented 12 months ago

I'm not sure what mileage means but appreciate your contributions.