Skip to content

Commit 3c2a2be

Browse files
committed
Warn user if two apps are sharing the same secrets. Fixes #2740
1 parent dc87f34 commit 3c2a2be

File tree

4 files changed

+48
-4
lines changed

4 files changed

+48
-4
lines changed

src/WebJobs.Script.WebHost/Properties/Resources.Designer.cs

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/WebJobs.Script.WebHost/Properties/Resources.resx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,11 @@
117117
<resheader name="writer">
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120+
<data name="ErrorSameSecrets" xml:space="preserve">
121+
<value>Two or more function apps are sharing the same secrets ({0})</value>
122+
</data>
120123
<data name="ErrorTooManySecretBackups" xml:space="preserve">
121-
<value>Repository has more than {0} non-decryptable secrets backups ({1}).</value>
124+
<value>Repository has more than {0} non-decryptable secrets backups ({1}). {2}</value>
122125
</data>
123126
<data name="FunctionSecretsSchemaV0" xml:space="preserve">
124127
<value>{

src/WebJobs.Script.WebHost/Security/SecretManager.cs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ private async Task PersistSecretsAsync<T>(T secrets, string keyScope = null, boo
409409

410410
if (secretBackups.Length >= ScriptConstants.MaximumSecretBackupCount)
411411
{
412-
string message = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, string.IsNullOrEmpty(keyScope) ? "host" : keyScope);
412+
string message = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, string.IsNullOrEmpty(keyScope) ? "host" : keyScope, await AnalizeSnapshots<T>(secretBackups));
413413
TraceEvent traceEvent = new TraceEvent(TraceLevel.Verbose, message);
414414
_traceWriter.Verbose(message);
415415
_logger?.LogDebug(message);
@@ -475,6 +475,31 @@ private void OnSecretsChanged(object sender, SecretsChangedEventArgs e)
475475
}
476476
}
477477

478+
private async Task<string> AnalizeSnapshots<T>(string[] secretBackups) where T : ScriptSecrets
479+
{
480+
string analizeResult = string.Empty;
481+
try
482+
{
483+
List<T> shapShots = new List<T>();
484+
foreach (string secretPath in secretBackups)
485+
{
486+
string secretString = await _repository.ReadAsync(ScriptSecretsType.Function, Path.GetFileNameWithoutExtension(secretPath));
487+
shapShots.Add(ScriptSecretSerializer.DeserializeSecrets<T>(secretString));
488+
}
489+
string[] hosts = shapShots.Select(x => x.HostName).Distinct().ToArray();
490+
if (hosts.Length > 1)
491+
{
492+
analizeResult = string.Format(Resources.ErrorSameSecrets, string.Join(",", hosts));
493+
}
494+
}
495+
catch
496+
{
497+
// best effort
498+
}
499+
return analizeResult;
500+
}
501+
502+
478503
public async Task PurgeOldSecretsAsync(string rootScriptPath, TraceWriter traceWriter, ILogger logger)
479504
{
480505
if (!Directory.Exists(rootScriptPath))

test/WebJobs.Script.Tests/Security/SecretManagerTests.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,8 @@ public async Task GetHostSecrets_WhenTooManyBackups_ThrowsException()
589589
using (var directory = new TempDirectory())
590590
{
591591
string functionName = "testfunction";
592-
string expectedTraceMessage = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, functionName);
592+
string expectedTraceMessage = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, functionName,
593+
string.Format(Resources.ErrorSameSecrets, "test0,test1"));
593594
string functionSecretsJson =
594595
@"{
595596
'keys': [
@@ -625,13 +626,19 @@ public async Task GetHostSecrets_WhenTooManyBackups_ThrowsException()
625626
await Task.Delay(500);
626627
}
627628

629+
string hostName = "test" + (i % 2).ToString();
630+
_settingsManager.SetSetting(EnvironmentSettingNames.AzureWebsiteHostName, hostName);
628631
functionSecrets = await secretManager.GetFunctionSecretsAsync(functionName);
629632
}
630633
}
631634
catch (InvalidOperationException ex)
632635
{
633636
ioe = ex;
634637
}
638+
finally
639+
{
640+
_settingsManager.SetSetting(EnvironmentSettingNames.AzureWebsiteHostName, null);
641+
}
635642

636643
Assert.True(ioe != null, string.Join(Environment.NewLine, traceWriter.GetTraces()));
637644
}

0 commit comments

Comments
 (0)