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

Add Azure Storage / Azure Key Vault extensibility to DataProtection #92

Closed
GrabYourPitchforks opened this issue Sep 8, 2015 · 53 comments

Comments

@GrabYourPitchforks
Copy link
Contributor

These solve two different but related problems. The DataProtection stack requires that all machines in the environment are able to point to the same key repository so that they can share key information. Out-of-box the DataProtection stack has support for the file system (including UNC paths) and the Windows registry. By adding support for Azure Storage, applications running within distributed environments would be able to use that as an alternative repository.

Once all machines agree on a key repository, they're faced with the problem of key protection at rest, since we generally don't want the keys sitting around in plaintext in the repository. The DataProtection stack has built-in support for using Windows DPAPI, CNG DPAPI, or an X.509 certificate. Adding support for Azure Key Vault would offer a mechanism for easing the burden of secret management in a distributed environment, and it would complement support for Azure Storage.

Not sure if this would be better suited as a sample application, but throwing the idea out there.

@muratg muratg added this to the 1.0.0 backlog milestone Sep 9, 2015
@muratg
Copy link

muratg commented Sep 29, 2015

I don't think this will fit in V1. We should probably write a sample around it when we document these things better.

@muratg muratg modified the milestones: Backlog, 1.0.0 backlog Sep 29, 2015
@guardrex
Copy link

👍 I think this is somewhat critical given that so much is moving to Azure right now. A very common scenario will be using Antiforgery with forms in web farm apps across Azure VM's, which creates and validates tokens with the data protection system. Azure Files might cut it for a network share (?) but the docs state that Core CLR cannot use the X.509 certificate bits to secure the keys on the share (if I'm reading that correctly).

Even if it were very manual at this point, a sample for hacking up an Azure Key Vault-to-Data Protection IKeyManager or XmlKeyManager implementation would be very helpful in the interim. AFAIK there is no sample anywhere of how this is done, correct?

@GrabYourPitchforks
Copy link
Contributor Author

@guardrex There's a workaround for doing certificate-based encryption if you're on Core CLR. However, it requires that you be on Windows 8.1 / Windows Server 2012 R2. See https://docs.asp.net/en/latest/security/data-protection/implementation/key-encryption-at-rest.html#certificate-based-encryption-with-windows-dpapi-ng.

You're correct in that there's currently no sample for using Azure Key Vault.

@guardrex
Copy link

@GrabYourPitchforks Thanks ... I'll give that a shot. I'm going to try this with Azure File Storage.

@guardrex
Copy link

@GrabYourPitchforks This isn't entirely surprising: I setup an Azure File Storage share with a key directory and confirmed the share was working on the VM, then used this ...

configure.PersistKeysToFileSystem(new DirectoryInfo(@"\\<my-storage-account-name>.file.core.windows.net\dataprotection\keys\"));

... and it looks like its choking on the age-old problem of permissions. With .NET/IIS working on ApplicationPoolIdentity or NetworkService, it doesn't seem to have permission to access the share, so navigating to a view with Antiforgery configured, it fails with ...

IOException: The parameter is incorrect
    System.IO.Win32FileSystem.CreateDirectory(String fullPath)
    Microsoft.AspNet.DataProtection.Repositories.FileSystemXmlRepository.<GetAllElementsCore>d__15.MoveNext()
    System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
    System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
    Microsoft.AspNet.DataProtection.Repositories.FileSystemXmlRepository.GetAllElements()
    Microsoft.AspNet.DataProtection.KeyManagement.XmlKeyManager.GetAllKeys()
    Microsoft.AspNet.DataProtection.KeyManagement.KeyRingProvider.CreateCacheableKeyRingCore(DateTimeOffset now, IKey keyJustAdded)
    Microsoft.AspNet.DataProtection.KeyManagement.KeyRingProvider.Microsoft.AspNet.DataProtection.KeyManagement.ICacheableKeyRingProvider.GetCacheableKeyRing(DateTimeOffset now)
    Microsoft.AspNet.DataProtection.KeyManagement.KeyRingProvider.GetCurrentKeyRingCore(DateTime utcNow)
    Microsoft.AspNet.DataProtection.KeyManagement.KeyRingBasedDataProtector.Protect(Byte[] plaintext)

Yeah, you have these flashbacks to the good 'ole days of impersonation! Yuck. I'll investigate making the share available to the app further; but given your familiarity with the packages and implementation, do you have any tips?

@guardrex
Copy link

Let's say I go with a custom IXmlRepository and use Azure Table Storage. Something like this:

In ConfigureServices(...) of Startup.cs

services.AddDataProtection();
var serviceDescriptor = new ServiceDescriptor(typeof(IXmlRepository), typeof(ICustomXmlRepository), ServiceLifetime.Singleton);
services.Add(serviceDescriptor);

XmlRepository.cs [The FetchAll() and Insert() are methods for grabbing and saving the keys. This app will have its own Azure Table for the keys, and multiple instances of the app will be hitting the table.]

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Xml.Linq;
using Microsoft.AspNet.DataProtection.Repositories;
using ShorehamPublic.Models;

namespace MyApp.Repositories
{
    public class ICustomXmlRepository : IXmlRepository
    {
        public IReadOnlyCollection<XElement> GetAllElements()
        {
            IList<XElement> keyList = new List<XElement>();
            IEnumerable<DataProtectionKey> keyEntities = DataProtectionKey.FetchAll().Result;
            foreach (var key in keyEntities)
            {
                keyList.Add(XElement.Parse(key.Data));
            }
            IReadOnlyCollection<XElement> readElements = new ReadOnlyCollection<XElement>(keyList);
            return readElements;
        }

        public async void StoreElement(XElement element, string friendlyName)
        {
            await DataProtectionKey.Insert(element.ToString());
        }
    }
}
  1. This seems to work ok in light testing, but what would the gotchas be for having instances of the app using this approach?
  2. If this is the only app (with multiple instances) using a table of keys, I don't need to supply an application name in configuration, correct?
  3. I noted <!-- Warning: the key below is in an unencrypted form. --> above the key value. What's the best way to handle this? Can I encrypt with out-of-the-box methods/configuration, or should I encrypt and decrypt the whole XElement in my table storage model methods?
  4. What about removing the table entities? I've created the entities with a timestamp RowKey. I presume I should clear the old keys on my own ... keys older than 90 days can be deleted from the table?
  5. Did I make a royal code smell anywhere? ... E.g., Did I register the custom IXmlRepository correctly? Does the implementation of the custom repository look ok?

@GrabYourPitchforks
Copy link
Contributor Author

/cc @blowdart

I reworked the existing Azure extensions for DataProtection I've been working on and migrated the project to https://github.com/GrabYourPitchforks/DataProtection.Azure/tree/dev. Feel free to give that a shot if you're looking for something quick and dirty. Maybe one day it can be contributed back into the main project. :)

In particular, the system ends up being more reliable if you use blobs instead of tables. Since the data is XML-based it makes more sense for the data to be in a file (blob) format rather than as a key/value pair in a table. This also makes read operations much faster since you'll end up reading all available data in a single operation rather than traversing all entries in a table.

To your other questions: if there's only one application pointed at a repository, you don't need to worry about the app name. You also don't need to worry about cleaning up old keys. The system's automatic key management will never delete old keys since deleting a key would render existing encrypted data undecipherable.

You can use the existing key encryption methods like certs. The system is designed such that repositories and key encryption methods are separate concerns and can be mixed-and-matched freely.

I'm also working on Azure Key Vault support in the project mentioned above, but it's not there yet. The combination of Azure Storage + Azure Key Vault would be a nice end-to-end "everything's hosted in the cloud and is automatic" story.

@guardrex
Copy link

@GrabYourPitchforks That's great. Thanks.

Since the data is XML-based it makes more sense for the data to be in a file (blob) format rather than as a key/value pair in a table. This also makes read operations much faster since you'll end up reading all available data in a single operation rather than traversing all entries in a table.

That table storage version I did seems to work, but I didn't stress test it yet. I feel you though on the "traversing entities," which is why I envision a single table per app for this and used a common PartitionKey for all entities in the table (i.e., to pull back all entities in one bite from a single edge server). I'm also taking the entire Xml and just plopping it into a property of the entity. I'm not trying to parse it and not dealing with key-value pairs. I'll do a stress test on it and see how it holds up.

The system's automatic key management will never delete old keys since deleting a key would render existing encrypted data undecipherable.

This is very interesting! The only use I have for this for the moment is for Antiforgery. If I recall correctly, the Antiforgery header/cookie system is setup for 90-day expiration out-of-the-box (?); so if I were only using Data Protection for Antiforgery, I guess I could kill old keys at 90-days for sure ... but that would be a strange case where someone opens a webform and leaves it open for as long as 89 days and finally POST's back to the server hoping for an Antiforgery validation to work. In any case, it would be easy for me to drop old keys, since my RowKey is actually a timestamp and easy to filter/delete on periodically.

I'm also working on Azure Key Vault support in the project mentioned above, but it's not there yet. The combination of Azure Storage + Azure Key Vault would be a nice end-to-end "everything's hosted in the cloud and is automatic" story.

Yeah, that's going to be excellent. That will be the gold standard for Azure-hosted apps IMHO. I'll give your DataProtection.Azure a shot, and I'll keep an eye out for Azure Storage + Azure Key Vault.

One ? though ... about the <!-- Warning: the key below is in an unencrypted form. --> note above the key value in the Xml. What's the best way to handle this if I want it encrypted? Is there something in-the-box, or should I just encrypt/decrypt the whole Xml payload just before dropping into storage and just before handing back to Data Protection?

@GrabYourPitchforks
Copy link
Contributor Author

Call any of the ProtectKeysWith* APIs, such as ProtectKeysWithCertificate, during the configuration routine. You can mix and match ProtectKeysWith* and PersistKeysTo* calls. The system is designed such the data repository doesn't know anything about how keys are encrypted at rest, and the key protection mechanism doesn't know anything about how the key files are stored.

@guardrex
Copy link

@GrabYourPitchforks Good grief. Sorry about that ... we just touched on that above. Thanks for your help ... I'm good now.

@blowdart
Copy link
Member

@muratg We should revisit this for RC2. People are asking about it more and more.

@guardrex
Copy link

👍 for Azure Table Storage as one of the offerings.

@muratg
Copy link

muratg commented Nov 13, 2015

I like @GrabYourPitchforks' extension that uses blob storage. We should consider getting it in our branch. Putting this in RC2 for now to consider in RC2.

@muratg muratg modified the milestones: 1.0.0-rc2, Backlog Nov 13, 2015
@guardrex
Copy link

@muratg I know ... it's a "file"-type structure/format. I was just saying if multiple options could exist, it would be nice to have a choice.

@muratg
Copy link

muratg commented Nov 13, 2015

@guardrex agreed!

Azure Key Vault support would also be nice BTW.

@guardrex
Copy link

@muratg Oh, yes, of course. I really like Azure Key Vault. Would love to use that for this.

@cwe1ss
Copy link

cwe1ss commented Mar 8, 2016

👍 on Azure Key Vault.

Is there any progress update on this topic?

@dfederm
Copy link

dfederm commented May 6, 2016

Ah, so this is why my users have to log in again every time I slotswap... Any way for Azure Websites to just support this out of the box across slots, or at least sticky to the slot?

@cwe1ss
Copy link

cwe1ss commented Aug 24, 2016

Of course, but it's using a different medium and no one can commit it to Source control (at least you must be pretty stupid to do so 😄). But handling certificates with expirations etc is also quite some work unfortunately.
I'm mostly using Service Fabric which allows to install certificates. But as far as I know you can upload certificates to Azure Web Apps - I think you must also set some magic appSettings key to actually be able to use them though.

@blowdart
Copy link
Member

blowdart commented Aug 24, 2016

Blob storage will work between slots. And Web Apps do support certs.

@gdoron
Copy link

gdoron commented Sep 6, 2016

@gdoron The slots thing azure knows about (and are hopefully working on)

@blowdart FYI I was told by an Escalation Engineer from the ASFS program that if it will be implemented it probably won't happen in the near future.
He explained to me that WebApp should be agnostic to the application and platform it's being used for, so WebApp shouldn't handle ASP.NET Core differently.

He suggested I won't wait for the fix and use the code from the PR: #163

I think it's reasonable and makes sense.

@blowdart
Copy link
Member

blowdart commented Sep 6, 2016

Yup. That'll will become part of the next core release.

As for the WebApp being agnostic, well I'd disagree, but it's Azure's choice.

@henningst
Copy link

I have just spent a few hours debugging a System.OperationCanceledException: The operation was canceled. which turned out to be caused by this:

fail: Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware[0]
      An unhandled exception has occurred: The antiforgery token could not be decrypted.
System.InvalidOperationException: The antiforgery token could not be decrypted. ---> System.Security.Cryptography.CryptographicException: The key {9725081b-7caf-4642-ae55-93cf9c871c36} was not found in the key ring.

I now understand that this is happening because I'm deploying to a separate staging slot and then swapping to production to avoid downtime. I though the keys stored in /ASP.NET/DataProtection-Keys/ were synchronized between all sites, but this is obviously not the case for slots.

Will this be handled for me by the Web App platform in the future? If so, when?

What is the current recommended approach to handle this issue? I've seen a blob storage implementation here, but I would feel more comfortable if there was an "official" implementation I could use.

@guardrex
Copy link

@henningst There's an official implementation for Azure Blob Storage. If you would like an Azure Table Storage implementation, you can hack your own key repository, but that's not recommended by MS.

@henningst
Copy link

Exactly what I was looking for! Thanks @guardrex!

@muratg muratg modified the milestones: 2.0.0-preview2, 2.0.0-preview1 Apr 21, 2017
@muratg
Copy link

muratg commented May 9, 2017

For the KeyVault extensibility we're waiting on some KeyVault SDK changes. Removing this from 2.0.0-preview2 for now to bring it back to triage.

@muratg muratg removed this from the 2.0.0-preview2 milestone May 9, 2017
@muratg muratg added this to the 2.1.0 milestone Aug 29, 2017
@muratg
Copy link

muratg commented Aug 29, 2017

We should have requirements in by now putting this in 2.1 milestone and assigning it to @pakrym.

cc @glennc @DamianEdwards @blowdart

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

9 participants