Skip to content

Commit 6f77505

Browse files
authored
Reuse known keys in XmlKeyManager (#54490)
* Flatten key type hierarchy We weren't actually using polymorphism and this makes cloning easier. * Reuse known keys in XmlKeyManager Keys read from the IXmlRepository (as opposed to created) use deferred decryption but may still be decrypted multiple times. Since they're immutable (other than revocation), keep track of the keys we've seen and reuse them next time they're returned from the IXmlRepository. In the absence of deletion support, we don't expect the list of keys to ever shrink, so there's no reason to prune the cache. While this is likely a perf win, it's a small perf win and only once a day, so it would probably not be worthwhile on that basis alone. However, this should also reduce the number of calls to `IXmlDecryptor.Decrypt` (which may, e.g., make an Azure KeyVault request). Each time the key ring is refreshed, it requests all keys from the key manager, clobbering all keys that may have already been decrypted (at least the default key will have been). In the single app-instance case, for example, there should be no need to call Decrypt at all for any key created during the current execution of the app (old keys will have to be decrypted the first time they're used). This shouldn't affect security because (a) the key ring already stores all the `IKeys` all the time and (b) `IKey` wraps the actual data in an `ISecret`. It would have been nice to pass the key ring's list of known keys to the key manager, rather than having it store its own cache, but the key ring's storage format would require abstraction violations that would work poorly with our public extension points.
1 parent 69ce648 commit 6f77505

File tree

6 files changed

+352
-150
lines changed

6 files changed

+352
-150
lines changed

src/DataProtection/DataProtection/src/KeyManagement/DeferredKey.cs

Lines changed: 0 additions & 60 deletions
This file was deleted.

src/DataProtection/DataProtection/src/KeyManagement/Key.cs

Lines changed: 130 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,156 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Xml.Linq;
67
using Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption;
78
using Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ConfigurationModel;
9+
using Microsoft.AspNetCore.DataProtection.KeyManagement.Internal;
10+
using Microsoft.AspNetCore.DataProtection.XmlEncryption;
811

912
namespace Microsoft.AspNetCore.DataProtection.KeyManagement;
1013

1114
/// <summary>
12-
/// The basic implementation of <see cref="IKey"/>, where the <see cref="IAuthenticatedEncryptorDescriptor"/>
13-
/// has already been created.
15+
/// The basic implementation of <see cref="IKey"/>.
1416
/// </summary>
15-
internal sealed class Key : KeyBase
17+
internal sealed class Key : IKey
1618
{
19+
private readonly Lazy<IAuthenticatedEncryptorDescriptor> _lazyDescriptor;
20+
private readonly IEnumerable<IAuthenticatedEncryptorFactory> _encryptorFactories;
21+
22+
private IAuthenticatedEncryptor? _encryptor;
23+
24+
/// <summary>
25+
/// The basic implementation of <see cref="IKey"/>, where the <see cref="IAuthenticatedEncryptorDescriptor"/>
26+
/// has already been created.
27+
/// </summary>
1728
public Key(
1829
Guid keyId,
1930
DateTimeOffset creationDate,
2031
DateTimeOffset activationDate,
2132
DateTimeOffset expirationDate,
2233
IAuthenticatedEncryptorDescriptor descriptor,
2334
IEnumerable<IAuthenticatedEncryptorFactory> encryptorFactories)
24-
: base(keyId,
35+
: this(keyId,
2536
creationDate,
2637
activationDate,
2738
expirationDate,
2839
new Lazy<IAuthenticatedEncryptorDescriptor>(() => descriptor),
2940
encryptorFactories)
3041
{
3142
}
43+
44+
/// <summary>
45+
/// The basic implementation of <see cref="IKey"/>, where the incoming XML element
46+
/// hasn't yet been fully processed.
47+
/// </summary>
48+
public Key(
49+
Guid keyId,
50+
DateTimeOffset creationDate,
51+
DateTimeOffset activationDate,
52+
DateTimeOffset expirationDate,
53+
IInternalXmlKeyManager keyManager,
54+
XElement keyElement,
55+
IEnumerable<IAuthenticatedEncryptorFactory> encryptorFactories)
56+
: this(keyId,
57+
creationDate,
58+
activationDate,
59+
expirationDate,
60+
new Lazy<IAuthenticatedEncryptorDescriptor>(GetLazyDescriptorDelegate(keyManager, keyElement)),
61+
encryptorFactories)
62+
{
63+
}
64+
65+
private Key(
66+
Guid keyId,
67+
DateTimeOffset creationDate,
68+
DateTimeOffset activationDate,
69+
DateTimeOffset expirationDate,
70+
Lazy<IAuthenticatedEncryptorDescriptor> lazyDescriptor,
71+
IEnumerable<IAuthenticatedEncryptorFactory> encryptorFactories)
72+
{
73+
KeyId = keyId;
74+
CreationDate = creationDate;
75+
ActivationDate = activationDate;
76+
ExpirationDate = expirationDate;
77+
_lazyDescriptor = lazyDescriptor;
78+
_encryptorFactories = encryptorFactories;
79+
}
80+
81+
public DateTimeOffset ActivationDate { get; }
82+
83+
public DateTimeOffset CreationDate { get; }
84+
85+
public DateTimeOffset ExpirationDate { get; }
86+
87+
public bool IsRevoked { get; private set; }
88+
89+
public Guid KeyId { get; }
90+
91+
public IAuthenticatedEncryptorDescriptor Descriptor
92+
{
93+
get
94+
{
95+
return _lazyDescriptor.Value;
96+
}
97+
}
98+
99+
public IAuthenticatedEncryptor? CreateEncryptor()
100+
{
101+
if (_encryptor == null)
102+
{
103+
foreach (var factory in _encryptorFactories)
104+
{
105+
var encryptor = factory.CreateEncryptorInstance(this);
106+
if (encryptor != null)
107+
{
108+
_encryptor = encryptor;
109+
break;
110+
}
111+
}
112+
}
113+
114+
return _encryptor;
115+
}
116+
117+
internal void SetRevoked()
118+
{
119+
IsRevoked = true;
120+
}
121+
122+
internal Key Clone()
123+
{
124+
return new Key(
125+
keyId: KeyId,
126+
creationDate: CreationDate,
127+
activationDate: ActivationDate,
128+
expirationDate: ExpirationDate,
129+
lazyDescriptor: _lazyDescriptor,
130+
encryptorFactories: _encryptorFactories)
131+
{
132+
IsRevoked = IsRevoked,
133+
};
134+
}
135+
136+
private static Func<IAuthenticatedEncryptorDescriptor> GetLazyDescriptorDelegate(IInternalXmlKeyManager keyManager, XElement keyElement)
137+
{
138+
// The <key> element will be held around in memory for a potentially lengthy period
139+
// of time. Since it might contain sensitive information, we should protect it.
140+
var encryptedKeyElement = keyElement.ToSecret();
141+
142+
try
143+
{
144+
return GetLazyDescriptorDelegate;
145+
}
146+
finally
147+
{
148+
// It's important that the lambda above doesn't capture 'descriptorElement'. Clearing the reference here
149+
// helps us detect if we've done this by causing a null ref at runtime.
150+
keyElement = null!;
151+
}
152+
153+
IAuthenticatedEncryptorDescriptor GetLazyDescriptorDelegate()
154+
{
155+
return keyManager.DeserializeDescriptorFromKeyElement(encryptedKeyElement.ToXElement());
156+
}
157+
}
32158
}

src/DataProtection/DataProtection/src/KeyManagement/KeyBase.cs

Lines changed: 0 additions & 77 deletions
This file was deleted.

0 commit comments

Comments
 (0)