Skip to content

Commit 76bb69a

Browse files
authored
Watch file-based certificates for changes (#49979)
* Watch file-based certificates for changes When `IConfiguration` specifies certificates by path and `reloadOnChange` is true, monitor those paths for changes and trigger reloads. The reload behavior is the same as if the path in the configuration file were changed, rather than the contents of the referenced file. - Does not apply to code-based configuration, which never triggers reloads - Does not trigger on deletion - that would just tear down the server - Does not look through symlinks to determine whether the target has changed - On by default (when `reloadOnChange` is true) but can be disabled with app context switch `Microsoft.AspNetCore.Server.Kestrel.DisableCertificateFileWatching` Fixes #32351 * Annotate CertificateConfig with MemberNotNullWhenAttribute * Index log messages from 1 * Make log message placeholders PascalCase * Append 'Unsynchronized' to test hook names * Add explanatory comment * Fix tests now that structured logging property names are PascalCase * Add a test at the KestrelConfigurationLoader level * Try a different way of updating a file's last write time to address CI flakiness * Speculatively use deterministically distinct names in flaky tests * Make more aggressive changes to files * Format log messages consistently * Remove redundant @s * Add an abstraction layer between CertificatePathWatcher and the file system to make testing more reliable * Clarify use of delays in KestrelConfigurationLoaderTests.CertificateChangedOnDisk * Address PR feedback * Preemptively add CertificateChangedOnDisk to the helix retry list
1 parent 6241625 commit 76bb69a

12 files changed

+1209
-14
lines changed

eng/test-configuration.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
{"testName": {"contains": "InjectedStartup_DefaultApplicationNameIsEntryAssembly"}},
2222
{"testName": {"contains": "HEADERS_Received_SecondRequest_ConnectProtocolReset"}},
2323
{"testName": {"contains": "ClientUsingOldCallWithNewProtocol"}},
24+
{"testName": {"contains": "CertificateChangedOnDisk"}},
2425
{"testAssembly": {"contains": "IIS"}},
2526
{"testAssembly": {"contains": "Template"}},
2627
{"failureMessage": {"contains":"(Site is started but no worker process found)"}},
Lines changed: 328 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,328 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Diagnostics;
5+
using Microsoft.Extensions.Configuration;
6+
using Microsoft.Extensions.FileProviders;
7+
using Microsoft.Extensions.FileProviders.Physical;
8+
using Microsoft.Extensions.Hosting;
9+
using Microsoft.Extensions.Logging;
10+
using Microsoft.Extensions.Primitives;
11+
12+
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal;
13+
14+
internal sealed partial class CertificatePathWatcher : IDisposable
15+
{
16+
private readonly Func<string, IFileProvider?> _fileProviderFactory;
17+
private readonly string _contentRootDir;
18+
private readonly ILogger<CertificatePathWatcher> _logger;
19+
20+
private readonly object _metadataLock = new();
21+
22+
/// <remarks>Acquire <see cref="_metadataLock"/> before accessing.</remarks>
23+
private readonly Dictionary<string, DirectoryWatchMetadata> _metadataForDirectory = new();
24+
/// <remarks>Acquire <see cref="_metadataLock"/> before accessing.</remarks>
25+
private readonly Dictionary<string, FileWatchMetadata> _metadataForFile = new();
26+
27+
private ConfigurationReloadToken _reloadToken = new();
28+
private bool _disposed;
29+
30+
public CertificatePathWatcher(IHostEnvironment hostEnvironment, ILogger<CertificatePathWatcher> logger)
31+
: this(
32+
hostEnvironment.ContentRootPath,
33+
logger,
34+
dir => Directory.Exists(dir) ? new PhysicalFileProvider(dir, ExclusionFilters.None) : null)
35+
{
36+
}
37+
38+
/// <remarks>
39+
/// For testing.
40+
/// </remarks>
41+
internal CertificatePathWatcher(string contentRootPath, ILogger<CertificatePathWatcher> logger, Func<string, IFileProvider?> fileProviderFactory)
42+
{
43+
_contentRootDir = contentRootPath;
44+
_logger = logger;
45+
_fileProviderFactory = fileProviderFactory;
46+
}
47+
48+
/// <summary>
49+
/// Returns a token that will fire when any watched <see cref="CertificateConfig"/> is changed on disk.
50+
/// The affected <see cref="CertificateConfig"/> will have <see cref="CertificateConfig.FileHasChanged"/>
51+
/// set to <code>true</code>.
52+
/// </summary>
53+
public IChangeToken GetChangeToken()
54+
{
55+
return _reloadToken;
56+
}
57+
58+
/// <summary>
59+
/// Update the set of <see cref="CertificateConfig"/>s being watched for file changes.
60+
/// If a given <see cref="CertificateConfig"/> appears in both lists, it is first removed and then added.
61+
/// </summary>
62+
/// <remarks>
63+
/// Does not consider targets when watching files that are symlinks.
64+
/// </remarks>
65+
public void UpdateWatches(List<CertificateConfig> certificateConfigsToRemove, List<CertificateConfig> certificateConfigsToAdd)
66+
{
67+
var addSet = new HashSet<CertificateConfig>(certificateConfigsToAdd, ReferenceEqualityComparer.Instance);
68+
var removeSet = new HashSet<CertificateConfig>(certificateConfigsToRemove, ReferenceEqualityComparer.Instance);
69+
70+
// Don't remove anything we're going to re-add anyway.
71+
// Don't remove such items from addSet to guard against the (hypothetical) possibility
72+
// that a caller might remove a config that isn't already present.
73+
removeSet.ExceptWith(certificateConfigsToAdd);
74+
75+
if (addSet.Count == 0 && removeSet.Count == 0)
76+
{
77+
return;
78+
}
79+
80+
lock (_metadataLock)
81+
{
82+
// Adds before removes to increase the chances of watcher reuse.
83+
// Since removeSet doesn't contain any of these configs, this won't change the semantics.
84+
foreach (var certificateConfig in addSet)
85+
{
86+
AddWatchUnsynchronized(certificateConfig);
87+
}
88+
89+
foreach (var certificateConfig in removeSet)
90+
{
91+
RemoveWatchUnsynchronized(certificateConfig);
92+
}
93+
}
94+
}
95+
96+
/// <summary>
97+
/// Start watching a certificate's file path for changes.
98+
/// <paramref name="certificateConfig"/> must have <see cref="CertificateConfig.IsFileCert"/> set to <code>true</code>.
99+
/// </summary>
100+
/// <remarks>
101+
/// Internal for testing.
102+
/// </remarks>
103+
internal void AddWatchUnsynchronized(CertificateConfig certificateConfig)
104+
{
105+
Debug.Assert(certificateConfig.IsFileCert, "AddWatch called on non-file cert");
106+
107+
var path = Path.Combine(_contentRootDir, certificateConfig.Path);
108+
var dir = Path.GetDirectoryName(path)!;
109+
110+
if (!_metadataForDirectory.TryGetValue(dir, out var dirMetadata))
111+
{
112+
// If we wanted to detected deletions of this whole directory (which we don't since we ignore deletions),
113+
// we'd probably need to watch the whole directory hierarchy
114+
115+
var fileProvider = _fileProviderFactory(dir);
116+
if (fileProvider is null)
117+
{
118+
_logger.DirectoryDoesNotExist(dir, path);
119+
return;
120+
}
121+
122+
dirMetadata = new DirectoryWatchMetadata(fileProvider);
123+
_metadataForDirectory.Add(dir, dirMetadata);
124+
125+
_logger.CreatedDirectoryWatcher(dir);
126+
}
127+
128+
if (!_metadataForFile.TryGetValue(path, out var fileMetadata))
129+
{
130+
// PhysicalFileProvider appears to be able to tolerate non-existent files, as long as the directory exists
131+
132+
var disposable = ChangeToken.OnChange(
133+
() => dirMetadata.FileProvider.Watch(Path.GetFileName(path)),
134+
static tuple => tuple.Item1.OnChange(tuple.Item2),
135+
ValueTuple.Create(this, path));
136+
137+
fileMetadata = new FileWatchMetadata(disposable);
138+
_metadataForFile.Add(path, fileMetadata);
139+
dirMetadata.FileWatchCount++;
140+
141+
// We actually don't care if the file doesn't exist - we'll watch in case it is created
142+
fileMetadata.LastModifiedTime = GetLastModifiedTimeOrMinimum(path, dirMetadata.FileProvider);
143+
144+
_logger.CreatedFileWatcher(path);
145+
}
146+
147+
if (!fileMetadata.Configs.Add(certificateConfig))
148+
{
149+
_logger.ReusedObserver(path);
150+
return;
151+
}
152+
153+
_logger.AddedObserver(path);
154+
155+
_logger.ObserverCount(path, fileMetadata.Configs.Count);
156+
_logger.FileCount(dir, dirMetadata.FileWatchCount);
157+
}
158+
159+
private DateTimeOffset GetLastModifiedTimeOrMinimum(string path, IFileProvider fileProvider)
160+
{
161+
try
162+
{
163+
return fileProvider.GetFileInfo(Path.GetFileName(path)).LastModified;
164+
}
165+
catch (Exception e)
166+
{
167+
_logger.LastModifiedTimeError(path, e);
168+
}
169+
170+
return DateTimeOffset.MinValue;
171+
}
172+
173+
private void OnChange(string path)
174+
{
175+
// Block until any in-progress updates are complete
176+
lock (_metadataLock)
177+
{
178+
if (!_metadataForFile.TryGetValue(path, out var fileMetadata))
179+
{
180+
_logger.UntrackedFileEvent(path);
181+
return;
182+
}
183+
184+
// Existence implied by the fact that we're tracking the file
185+
var dirMetadata = _metadataForDirectory[Path.GetDirectoryName(path)!];
186+
187+
// We ignore file changes that don't advance the last modified time.
188+
// For example, if we lose access to the network share the file is
189+
// stored on, we don't notify our listeners because no one wants
190+
// their endpoint/server to shutdown when that happens.
191+
// We also anticipate that a cert file might be renamed to cert.bak
192+
// before a new cert is introduced with the old name.
193+
// This also helps us in scenarios where the underlying file system
194+
// reports more than one change for a single logical operation.
195+
var lastModifiedTime = GetLastModifiedTimeOrMinimum(path, dirMetadata.FileProvider);
196+
if (lastModifiedTime > fileMetadata.LastModifiedTime)
197+
{
198+
fileMetadata.LastModifiedTime = lastModifiedTime;
199+
}
200+
else
201+
{
202+
if (lastModifiedTime == DateTimeOffset.MinValue)
203+
{
204+
_logger.EventWithoutLastModifiedTime(path);
205+
}
206+
else if (lastModifiedTime == fileMetadata.LastModifiedTime)
207+
{
208+
_logger.RedundantEvent(path);
209+
}
210+
else
211+
{
212+
_logger.OutOfOrderEvent(path);
213+
}
214+
return;
215+
}
216+
217+
var configs = fileMetadata.Configs;
218+
foreach (var config in configs)
219+
{
220+
config.FileHasChanged = true;
221+
}
222+
}
223+
224+
// AddWatch and RemoveWatch don't affect the token, so this doesn't need to be under the semaphore.
225+
// It does however need to be synchronized, since there could be multiple concurrent events.
226+
var previousToken = Interlocked.Exchange(ref _reloadToken, new());
227+
previousToken.OnReload();
228+
}
229+
230+
/// <summary>
231+
/// Stop watching a certificate's file path for changes (previously started by <see cref="AddWatchUnsynchronized"/>.
232+
/// <paramref name="certificateConfig"/> must have <see cref="CertificateConfig.IsFileCert"/> set to <code>true</code>.
233+
/// </summary>
234+
/// <remarks>
235+
/// Internal for testing.
236+
/// </remarks>
237+
internal void RemoveWatchUnsynchronized(CertificateConfig certificateConfig)
238+
{
239+
Debug.Assert(certificateConfig.IsFileCert, "RemoveWatch called on non-file cert");
240+
241+
var path = Path.Combine(_contentRootDir, certificateConfig.Path);
242+
var dir = Path.GetDirectoryName(path)!;
243+
244+
if (!_metadataForFile.TryGetValue(path, out var fileMetadata))
245+
{
246+
_logger.UnknownFile(path);
247+
return;
248+
}
249+
250+
var configs = fileMetadata.Configs;
251+
252+
if (!configs.Remove(certificateConfig))
253+
{
254+
_logger.UnknownObserver(path);
255+
return;
256+
}
257+
258+
_logger.RemovedObserver(path);
259+
260+
// If we found fileMetadata, there must be a containing/corresponding dirMetadata
261+
var dirMetadata = _metadataForDirectory[dir];
262+
263+
if (configs.Count == 0)
264+
{
265+
fileMetadata.Dispose();
266+
_metadataForFile.Remove(path);
267+
dirMetadata.FileWatchCount--;
268+
269+
_logger.RemovedFileWatcher(path);
270+
271+
if (dirMetadata.FileWatchCount == 0)
272+
{
273+
dirMetadata.Dispose();
274+
_metadataForDirectory.Remove(dir);
275+
276+
_logger.RemovedDirectoryWatcher(dir);
277+
}
278+
}
279+
280+
_logger.ObserverCount(path, configs.Count);
281+
_logger.FileCount(dir, dirMetadata.FileWatchCount);
282+
}
283+
284+
/// <remarks>Test hook</remarks>
285+
internal int TestGetDirectoryWatchCountUnsynchronized() => _metadataForDirectory.Count;
286+
287+
/// <remarks>Test hook</remarks>
288+
internal int TestGetFileWatchCountUnsynchronized(string dir) => _metadataForDirectory.TryGetValue(dir, out var metadata) ? metadata.FileWatchCount : 0;
289+
290+
/// <remarks>Test hook</remarks>
291+
internal int TestGetObserverCountUnsynchronized(string path) => _metadataForFile.TryGetValue(path, out var metadata) ? metadata.Configs.Count : 0;
292+
293+
void IDisposable.Dispose()
294+
{
295+
if (_disposed)
296+
{
297+
return;
298+
}
299+
_disposed = true;
300+
301+
foreach (var dirMetadata in _metadataForDirectory.Values)
302+
{
303+
dirMetadata.Dispose();
304+
}
305+
306+
foreach (var fileMetadata in _metadataForFile.Values)
307+
{
308+
fileMetadata.Dispose();
309+
}
310+
}
311+
312+
private sealed class DirectoryWatchMetadata(IFileProvider fileProvider) : IDisposable
313+
{
314+
public readonly IFileProvider FileProvider = fileProvider;
315+
public int FileWatchCount;
316+
317+
public void Dispose() => (FileProvider as IDisposable)?.Dispose();
318+
}
319+
320+
private sealed class FileWatchMetadata(IDisposable disposable) : IDisposable
321+
{
322+
public readonly IDisposable Disposable = disposable;
323+
public readonly HashSet<CertificateConfig> Configs = new(ReferenceEqualityComparer.Instance);
324+
public DateTimeOffset LastModifiedTime = DateTimeOffset.MinValue;
325+
326+
public void Dispose() => Disposable.Dispose();
327+
}
328+
}

0 commit comments

Comments
 (0)