Feature Request: ctor injection IXmlDecryptor not working as well as for IXmlEncryptor

[aspnet-hello]

From @guardrex on Monday, May 30, 2016 1:57:49 PM

Consider the following replacements for IXmlEncryptor and IXmlDecryptor ...

public class CustomXmlEncryptor : IXmlEncryptor
{
    public CustomXmlEncryptor()
    {
    }

    public CustomXmlEncryptor(IOptions<CustomDataProtectionOptions> dataProtectionOptions)
    {
        _key = dataProtectionOptions.Value.Key;
    }

    private byte[] _key;

    public EncryptedXmlInfo Encrypt(XElement plaintextElement)
    {
        // Code here uses _key to encrypt
    }
}

public class CustomXmlDecryptor : IXmlDecryptor
{
    public CustomXmlDecryptor()
    {
    }

    public CustomXmlDecryptor(IOptions<CustomDataProtectionOptions> dataProtectionOptions)
    {
        _key = dataProtectionOptions.Value.Key;
    }

    private byte[] _key;

    public XElement Decrypt(XElement encryptedElement)
    {
        // Code here uses _key to decrypt
    }
}

Both derived classes require a parameterless ctor. The CustomXmlEncryptor always gets the correct _key value via DI with my options-based ctor. However, the CustomXmlDecryptor always gets created using its parameterless ctor, thus it never gets a value in _key from DI.

I have a pathetic workaround using a global static, but I'd like to get that CustomXmlDecryptor on DI. Am I doing it wrong? Is this an unknown bug? Is this a known bug covered by Refactor DI support in DataProtection (DataProtection #134)?

Copied from original issue: aspnet/DataProtection#154

[aspnet-hello]

From @hotchkj on Tuesday, June 7, 2016 12:37:17 PM

I also hit this when implementing a custom decryptor. I worked around it by taking advantage of the SimpleActivator fallback that calls a constructor with IServiceProvider, but naturally this requires DI in place.

Interested to know what the right approach is.

[aspnet-hello]

From @guardrex on Tuesday, June 7, 2016 12:40:45 PM

If it's a bug, I wish it would get some :heart:. My workaround is crappier than your's: a global static to get the value into the ctor. (yuck!)

[aspnet-hello]

From @tillig on Thursday, June 9, 2016 8:20:42 AM

I think I just ran into this, too, and also when implementing a custom decryptor. I'm implementing a custom IActivator since, in RC2 at least, it doesn't look like there's an IActivator registered in the service collection by default so everything IActivator is used for boils down to Activator.CreateInstance(). I tried implementing a custom activator but down in the internal key management stack it's not getting my registered activator and still uses the default one. This exception is thrown without calling my custom IActivator even though other service construction calls do go through it.

System.Security.Cryptography.CryptographicException: Assertion failed: CreateInstance returned null.
   at Microsoft.AspNetCore.Cryptography.CryptoUtil.Fail(String message)
   at Microsoft.AspNetCore.Cryptography.CryptoUtil.Fail[T](String message)
   at Microsoft.AspNetCore.DataProtection.ActivatorExtensions.CreateInstance[T](IActivator activator, String implementationTypeName)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.Microsoft.AspNetCore.DataProtection.KeyManagement.Internal.IInternalXmlKeyManager.DeserializeDescriptorFromKeyElement(XElement keyElement)
Microsoft.AspNetCore.DataProtection.KeyManagement.DefaultKeyResolver: Warning: Key {35528037-ba8f-4b9f-908e-1baa2935fb57} is ineligible to be the default key because its CreateEncryptorInstance method failed.

System.Security.Cryptography.CryptographicException: Assertion failed: CreateInstance returned null.
   at Microsoft.AspNetCore.Cryptography.CryptoUtil.Fail(String message)
   at Microsoft.AspNetCore.Cryptography.CryptoUtil.Fail[T](String message)
   at Microsoft.AspNetCore.DataProtection.ActivatorExtensions.CreateInstance[T](IActivator activator, String implementationTypeName)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.Microsoft.AspNetCore.DataProtection.KeyManagement.Internal.IInternalXmlKeyManager.DeserializeDescriptorFromKeyElement(XElement keyElement)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.DeferredKey.<>c__DisplayClass1_0.<GetLazyEncryptorDelegate>b__0()
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.LazyInitValue()
   at Microsoft.AspNetCore.DataProtection.KeyManagement.DefaultKeyResolver.CanCreateAuthenticatedEncryptor(IKey key)

[aspnet-hello]

From @muratg on Thursday, June 9, 2016 9:24:03 AM

@blowdart thoughts?

[aspnet-hello]

From @blowdart on Thursday, June 9, 2016 10:16:21 AM

It was to work around some deficiencies with the DI stack at the time, including https://github.com/aspnet/DependencyInjection/issues/90, https://github.com/aspnet/DependencyInjection/issues/197 and the removal of ITypeActivator.

If the underlying deficiencies have been resolved since the original code was written then we could revisit this, however keep in mind that the DataProtection stack also needs to run in existing ASP.NET 4.x environments to support the compatibility layer, so your test matrix can't include just Core apps, and only DI scenarios.

[aspnet-hello]

From @guardrex on Thursday, June 9, 2016 10:36:30 AM

Can you tell me one thing guidance-wise: I don't like having a global static in my CustomXmlDecryptor, even if I am only contending with this DI problem temporarily. Yuck .....

public CustomXmlDecryptor()
{
    _key = Global.DataProtectionKey;
}

What is the correct workaround: Service locator pattern? What does that look like in this context to get something close to my goal of ...

public CustomXmlDecryptor(IOptions<CustomDataProtectionOptions> dataProtectionOptions)
{
    _key = dataProtectionOptions.Value.Key;
}

[aspnet-hello]

From @blowdart on Thursday, June 9, 2016 10:41:20 AM

As I wasn't the dev I have no opinions on the right way to do this to be honest. All I can do is point you to the decryptors we provide.

[aspnet-hello]

From @tillig on Thursday, June 9, 2016 10:48:02 AM

I can PR this thing if folks are interested, once it's a bit more polished. But, yeah, it's service location.

The DI problems mean you basically get either a parameterless constructor or a constructor that takes an IServiceProvider. So what I did was:

Create an "options" object that just has the certificate I want to pass around. I may add other options later, but for now the purpose it serves is being a strong type that isn't just X509Certificate2 in my service collection.

public class CertificateEncryptionOptions
{
  public X509Certificate2 Certificate { get; set; }
}

At app startup I register that with the service collection.

var opts = new CertificateEncryptionOptions { Certificate = cert };
services.AddSingleton(opts);

Then in my constructor for the encryptor/decryptor, I manually resolve the options.

public CertificateXmlDecryptor(IServiceProvider services)
{
  var options = services.GetRequiredService<CertificateEncryptionOptions>();
  // Get the certificate and do stuff.
}

[aspnet-hello]

From @guardrex on Thursday, June 9, 2016 10:52:25 AM

@tillig Ok, cool. I'll give that a shot this afternoon.

This issue is probably not getting a bug label because the issues that @blowdart listed and the one that I listed, Refactor DI support in DataProtection (DataProtection #134), are known.

I'll determine a suitable workaround with the service locator pattern that addresses my OP, post that here, then close this issue ... probably later today.

[aspnet-hello]

From @guardrex on Thursday, June 9, 2016 1:48:02 PM

Ok, so this seems to work ...

public CustomXmlDecryptor(IServiceProvider services)
{
    var dataProtectionOptions = 
        services.GetRequiredService<IOptions<CustomDataProtectionOptions>>();
    _key = dataProtectionOptions.Value.Key;
}

Are there any comments (gotchas?) on this workaround approach before I close?

[aspnet-hello]

From @tillig on Wednesday, June 15, 2016 8:44:02 AM

For future reference I posted my custom encryptors and Redis key storage repository.

[aspnet-hello]

From @guardrex on Wednesday, October 26, 2016 9:41:41 AM

I was informed that this is a feature request (see https://github.com/aspnet/DataProtection/issues/134#issuecomment-256368813).

[aspnet-hello]

From @muratg on Tuesday, November 1, 2016 9:31:33 AM

Backlogging for now.

[mkArtakMSFT]

Hi. Thanks for contacting us. We're closing this issue as there was not much community interest in this ask for quite a while now. You can learn more about our triage process and how we handle issues by reading our Triage Process writeup.