Skip to content
This repository was archived by the owner on Oct 17, 2018. It is now read-only.

Make Microsoft.AspNetCore.DataProtection.DataProtectionProvider class static #120

Closed
javiercn opened this issue Feb 24, 2016 · 9 comments
Closed
Assignees
Milestone

Comments

@javiercn
Copy link
Member

https://github.com/aspnet/DataProtection/blob/dev/src/Microsoft.AspNetCore.DataProtection.Extensions/DataProtectionProvider.cs

https://docs.asp.net/en/latest/security/data-protection/configuration/non-di-scenarios.html

This class is meant to be used in Non-DI scenarios as an entry point for creating a IDataProtectionProvider instance that can be used acrross the app to protect and unprotect data. As such, making it static will avoid any confusion of mixing it with DI enabled scenarios. The advantages are:

  • Clearly separates DI scenarios from non DI scenarios. (You can't register a static class in a DI Container)
  • Makes the class cleaner (The current implementation creates a whole data protection stack inside the constructor, and wraps the default implementation without any perceived benefit.
  • Is arguably less discoverable, it's easier to 'dot' into a static class and see a method that you can use to construct an instance of the thing that you want, than having to use 'new' on the type.
  • By doing all the setup in a constructor, we are cornering ourselves in the future if we wan't to do more complicated things inside the setup, like for example, caching existing DataProtectionProviders.

As a reference, here is the implementation as a static class

using System;
using System.IO;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.AspNetCore.DataProtection
{
    public static class DataProtectionProvider
    {
        public static IDataProtectionProvider Create(DirectoryInfo keyDirectory)
        {
            return Create(keyDirectory, setupAction: null);
        }

        public static IDataProtectionProvider Create(
            DirectoryInfo keyDirectory,
            Action<DataProtectionConfiguration> setupAction)
        {
            if (keyDirectory == null)
            {
                throw new ArgumentNullException(nameof(keyDirectory));
            }

            var serviceCollection = new ServiceCollection();
            var builder = serviceCollection.AddDataProtection();
            builder.PersistKeysToFileSystem(keyDirectory);

            if(setupAction != null)
            {
                setupAction(builder);
            }

            return serviceCollection.BuildServiceProvider().GetRequiredService<IDataProtectionProvider>();
        }
    }
}
@danroth27
Copy link
Member

@blowdart

@blowdart
Copy link
Member

By doing all the setup in a constructor, we are cornering ourselves in the future if we wan't to do more complicated things inside the setup, like for example, caching existing DataProtectionProviders.

This is the simple "Give me something and get the hell out of my way" class. We don't want to be doing anything complicated in it at all, beyond allowing X509 protection of the key ring.

What if this is what someone wants to register as a service in DI?

@javiercn
Copy link
Member Author

@blowdart It doesn't make sense as a DI-able service, what would be the point? It's easier for a customer to write their own DataProtector wrapper if they need to than reusing this one.

@blowdart
Copy link
Member

Well it doesn't affect anything really, so I'm ok either way

@javiercn javiercn added this to the 1.0.0-rc2 milestone Feb 25, 2016
@javiercn javiercn added the bug label Feb 25, 2016
@javiercn javiercn removed this from the 1.0.0-rc2 milestone Feb 25, 2016
@muratg muratg added this to the 1.0.0-rc2 milestone Mar 3, 2016
javiercn added a commit to dotnet/AspNetCore.Docs that referenced this issue Mar 16, 2016
Update code to reflect changes in aspnet/DataProtection#120
shirhatti pushed a commit to shirhatti/Docs that referenced this issue Mar 17, 2016
Update code to reflect changes in aspnet/DataProtection#120
@gitfortee
Copy link

Hey Folks,

When i use this non-DI Api and provide a DirectoryInfo, will the encrypted content (after decryption) work in other machines too? or does it only work in the machine in which the content was encrypted.?

@gitfortee
Copy link

gitfortee commented Oct 25, 2017

@javiercn

I really liked your idea as it is useful when you try to decrypt connection string to use with services.AddDbContext.UseSqlServer()

However, need a clarification..

When i use this non-DI Api and provide a DirectoryInfo, will the encrypted content (after decryption) work in other machines too? or does it only work in the machine in which the content was encrypted.?

I want to use the same encrypted password across other machines.. And I am bit doubtful if the encryption is bound to the machine in which the password is encrypted.. and thus the encrypted password can't be shared with other machines(environments)..

@gitfortee
Copy link

gitfortee commented Oct 25, 2017

@javiercn @blowdart @strohhut

Looks like you "can not" copy the "Keys" directory as part of deployment. And it also appears you need to perform encryption "whenever you deploy the code to a new machine".. Is there a workaround for that?

@javiercn
Copy link
Member Author

You can put your keys in a storage location that is shared across your machines, like Redis, blob storage or similar (I believe). @blowdart can provide more details on this.

In essence if you want multiple machines to encrypt/decrypt some piece of data, the keys need to be shared.

@blowdart
Copy link
Member

Storage of the key ring and protection of the key ring are separate. So if you want to share the protection then it'd either be an X509Certificate or some other mechanism. This is documented. If the docs are confusing please file a new issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants