Skip to content

Commit 4384b9b

Browse files
MichalPavlikMichalPavlik
committed
Merged PR 40255: Fixing MSRC issue with registry credential re-use
**Description** Added new set of env variables to allow separately specifying credentials for Pull and Push Registry operations. The old set of variables is used as a fallback for the Push Registry operations, and the pull operations _in the specific case_ where the push and pull registries are the same. **Customer Impact** Customers that used the environment variable approach (not incredibly common, but some key use cases like `azd` (they are changing) and used different source and target registries (e.g. mcr.microsoft.com for the source, ghcr.io or ACR for target) would send credentials intended for the target to the source. **Regression** No - this behavior has been around as long as we supported pull registry authentication. **Risk** Low - this includes new tests for the use of the various env vars in the various combinations of push/pull registry domain.
2 parents ef7dad9 + 80e3001 commit 4384b9b

File tree

13 files changed

+148
-26
lines changed

13 files changed

+148
-26
lines changed

‎src/Containers/Microsoft.NET.Build.Containers/AuthHandshakeMessageHandler.cs

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Concurrent;
5+
using System.ComponentModel;
56
using System.Diagnostics;
67
using System.Diagnostics.CodeAnalysis;
78
using System.Net;
@@ -36,12 +37,14 @@ private sealed record AuthInfo(string Realm, string? Service, string? Scope);
3637

3738
private readonly string _registryName;
3839
private readonly ILogger _logger;
40+
private readonly RegistryMode _registryMode;
3941
private static ConcurrentDictionary<string, AuthenticationHeaderValue?> _authenticationHeaders = new();
4042

41-
public AuthHandshakeMessageHandler(string registryName, HttpMessageHandler innerHandler, ILogger logger) : base(innerHandler)
43+
public AuthHandshakeMessageHandler(string registryName, HttpMessageHandler innerHandler, ILogger logger, RegistryMode mode) : base(innerHandler)
4244
{
4345
_registryName = registryName;
4446
_logger = logger;
47+
_registryMode = mode;
4548
}
4649

4750
/// <summary>
@@ -136,8 +139,7 @@ public DateTimeOffset ResolvedExpiration {
136139
private async Task<(AuthenticationHeaderValue, DateTimeOffset)?> GetAuthenticationAsync(string registry, string scheme, AuthInfo? bearerAuthInfo, CancellationToken cancellationToken)
137140
{
138141
// Allow overrides for auth via environment variables
139-
string? credU = Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectUser);
140-
string? credP = Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectPass);
142+
(string? credU, string? credP) = GetDockerCredentialsFromEnvironment(_registryMode);
141143

142144
// fetch creds for the host
143145
DockerCredentials? privateRepoCreds;
@@ -175,6 +177,46 @@ public DateTimeOffset ResolvedExpiration {
175177
}
176178
}
177179

180+
/// <summary>
181+
/// Gets docker credentials from the environment variables based on registry mode.
182+
/// </summary>
183+
internal static (string? credU, string? credP) GetDockerCredentialsFromEnvironment(RegistryMode mode)
184+
{
185+
if (mode == RegistryMode.Push)
186+
{
187+
string? credU = Environment.GetEnvironmentVariable(ContainerHelpers.PushHostObjectUser);
188+
string? credP = Environment.GetEnvironmentVariable(ContainerHelpers.PushHostObjectPass);
189+
if (string.IsNullOrEmpty(credU) || string.IsNullOrEmpty(credP))
190+
{
191+
// Fallback to the old environment variables
192+
return (Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectUser),
193+
Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectPass));
194+
}
195+
return (credU, credP);
196+
}
197+
else if (mode == RegistryMode.Pull)
198+
{
199+
return (Environment.GetEnvironmentVariable(ContainerHelpers.PullHostObjectUser),
200+
Environment.GetEnvironmentVariable(ContainerHelpers.PullHostObjectPass));
201+
}
202+
else if (mode == RegistryMode.PullFromOutput)
203+
{
204+
string? credU = Environment.GetEnvironmentVariable(ContainerHelpers.PullHostObjectUser);
205+
string? credP = Environment.GetEnvironmentVariable(ContainerHelpers.PullHostObjectPass);
206+
if (string.IsNullOrEmpty(credU) || string.IsNullOrEmpty(credP))
207+
{
208+
// Fallback to the old environment variables
209+
return (Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectUser),
210+
Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectPass));
211+
}
212+
return (credU, credP);
213+
}
214+
else
215+
{
216+
throw new InvalidEnumArgumentException(nameof(mode), (int)mode, typeof(RegistryMode));
217+
}
218+
}
219+
178220
/// <summary>
179221
/// Implements the Docker OAuth2 Authentication flow as documented at <see href="https://docs.docker.com/registry/spec/auth/oauth/"/>.
180222
/// </summary

‎src/Containers/Microsoft.NET.Build.Containers/ContainerBuilder.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ public static async Task<int> ContainerizeAsync(
4343
logger.LogTrace("Trace logging: enabled.");
4444

4545
bool isLocalPull = string.IsNullOrEmpty(baseRegistry);
46-
Registry? sourceRegistry = isLocalPull ? null : new Registry(baseRegistry, logger);
46+
RegistryMode sourceRegistryMode = baseRegistry.Equals(outputRegistry, StringComparison.InvariantCultureIgnoreCase) ? RegistryMode.PullFromOutput : RegistryMode.Pull;
47+
Registry? sourceRegistry = isLocalPull ? null : new Registry(baseRegistry, logger, sourceRegistryMode);
4748
SourceImageReference sourceImageReference = new(sourceRegistry, baseImageName, baseImageTag);
4849

4950
DestinationImageReference destinationImageReference = DestinationImageReference.CreateFromSettings(

‎src/Containers/Microsoft.NET.Build.Containers/ContainerHelpers.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,14 @@ namespace Microsoft.NET.Build.Containers;
1616
public static class ContainerHelpers
1717
{
1818
internal const string HostObjectUser = "SDK_CONTAINER_REGISTRY_UNAME";
19-
2019
internal const string HostObjectPass = "SDK_CONTAINER_REGISTRY_PWORD";
2120

21+
internal const string PushHostObjectUser = "SDK_CONTAINER_PUSH_REGISTRY_UNAME";
22+
internal const string PushHostObjectPass = "SDK_CONTAINER_PUSH_REGISTRY_PWORD";
23+
24+
internal const string PullHostObjectUser = "SDK_CONTAINER_PULL_REGISTRY_UNAME";
25+
internal const string PullHostObjectPass = "SDK_CONTAINER_PULL_REGISTRY_PWORD";
26+
2227
internal const string DockerRegistryAlias = "docker.io";
2328

2429
/// <summary>

‎src/Containers/Microsoft.NET.Build.Containers/DestinationImageReference.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ public static DestinationImageReference CreateFromSettings(
7575
}
7676
else if (!string.IsNullOrEmpty(outputRegistry))
7777
{
78-
destinationImageReference = new DestinationImageReference(new Registry(outputRegistry, loggerFactory.CreateLogger<Registry>()), repository, imageTags);
78+
destinationImageReference = new DestinationImageReference(
79+
new Registry(outputRegistry, loggerFactory.CreateLogger<Registry>(), RegistryMode.Push),
80+
repository,
81+
imageTags);
7982
}
8083
else
8184
{

‎src/Containers/Microsoft.NET.Build.Containers/Registry/DefaultRegistryAPI.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ internal class DefaultRegistryAPI : IRegistryAPI
2222
// Making this a round 30 for convenience.
2323
private static TimeSpan LongRequestTimeout = TimeSpan.FromMinutes(30);
2424

25-
internal DefaultRegistryAPI(string registryName, Uri baseUri, ILogger logger)
25+
internal DefaultRegistryAPI(string registryName, Uri baseUri, ILogger logger, RegistryMode mode)
2626
{
2727
bool isAmazonECRRegistry = baseUri.IsAmazonECRRegistry();
2828
_baseUri = baseUri;
2929
_logger = logger;
30-
_client = CreateClient(registryName, baseUri, logger, isAmazonECRRegistry);
30+
_client = CreateClient(registryName, baseUri, logger, isAmazonECRRegistry, mode);
3131
Manifest = new DefaultManifestOperations(_baseUri, registryName, _client, _logger);
3232
Blob = new DefaultBlobOperations(_baseUri, registryName, _client, _logger);
3333
}
@@ -36,7 +36,7 @@ internal DefaultRegistryAPI(string registryName, Uri baseUri, ILogger logger)
3636

3737
public IManifestOperations Manifest { get; }
3838

39-
private static HttpClient CreateClient(string registryName, Uri baseUri, ILogger logger, bool isAmazonECRRegistry = false)
39+
private static HttpClient CreateClient(string registryName, Uri baseUri, ILogger logger, bool isAmazonECRRegistry, RegistryMode mode)
4040
{
4141
var innerHandler = new SocketsHttpHandler()
4242
{
@@ -55,7 +55,7 @@ private static HttpClient CreateClient(string registryName, Uri baseUri, ILogger
5555
};
5656
}
5757

58-
HttpMessageHandler clientHandler = new AuthHandshakeMessageHandler(registryName, innerHandler, logger);
58+
HttpMessageHandler clientHandler = new AuthHandshakeMessageHandler(registryName, innerHandler, logger, mode);
5959

6060
if (isAmazonECRRegistry)
6161
{

‎src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@
1010

1111
namespace Microsoft.NET.Build.Containers;
1212

13+
internal enum RegistryMode
14+
{
15+
Push,
16+
Pull,
17+
PullFromOutput
18+
}
19+
1320
internal sealed class Registry
1421
{
1522
private const string DockerHubRegistry1 = "registry-1.docker.io";
@@ -27,11 +34,23 @@ internal sealed class Registry
2734
/// </summary>
2835
public string RegistryName { get; }
2936

30-
internal Registry(string registryName, ILogger logger, IRegistryAPI? registryAPI = null, RegistrySettings? settings = null) :
31-
this(ContainerHelpers.TryExpandRegistryToUri(registryName), logger, registryAPI, settings)
37+
internal Registry(string registryName, ILogger logger, RegistryMode mode, RegistrySettings? settings = null) : this(
38+
ContainerHelpers.TryExpandRegistryToUri(registryName), logger, new RegistryApiFactory(mode), settings)
39+
{ }
40+
41+
internal Registry(string registryName, ILogger logger, IRegistryAPI registryAPI, RegistrySettings? settings = null) : this(
42+
ContainerHelpers.TryExpandRegistryToUri(registryName), logger, new RegistryApiFactory(registryAPI), settings)
3243
{ }
3344

34-
internal Registry(Uri baseUri, ILogger logger, IRegistryAPI? registryAPI = null, RegistrySettings? settings = null)
45+
internal Registry(Uri baseUri, ILogger logger, IRegistryAPI registryAPI, RegistrySettings? settings = null) :
46+
this(baseUri, logger, new RegistryApiFactory(registryAPI), settings)
47+
{ }
48+
49+
internal Registry(Uri baseUri, ILogger logger, RegistryMode mode, RegistrySettings? settings = null) :
50+
this(baseUri, logger, new RegistryApiFactory(mode), settings)
51+
{ }
52+
53+
private Registry(Uri baseUri, ILogger logger, RegistryApiFactory factory, RegistrySettings? settings = null)
3554
{
3655
RegistryName = DeriveRegistryName(baseUri);
3756

@@ -44,7 +63,7 @@ internal Registry(Uri baseUri, ILogger logger, IRegistryAPI? registryAPI = null,
4463

4564
_logger = logger;
4665
_settings = settings ?? new RegistrySettings();
47-
_registryAPI = registryAPI ?? new DefaultRegistryAPI(RegistryName, BaseUri, logger);
66+
_registryAPI = factory.Create(RegistryName, BaseUri, logger);
4867
}
4968

5069
private static string DeriveRegistryName(Uri baseUri)
@@ -442,4 +461,24 @@ private async Task PushAsync(BuiltImage builtImage, SourceImageReference source,
442461
_logger.LogInformation(Strings.Registry_ManifestUploaded, RegistryName);
443462
}
444463
}
464+
465+
private readonly ref struct RegistryApiFactory
466+
{
467+
private readonly IRegistryAPI? _registryApi;
468+
private readonly RegistryMode? _mode;
469+
public RegistryApiFactory(IRegistryAPI registryApi)
470+
{
471+
_registryApi = registryApi;
472+
}
473+
474+
public RegistryApiFactory(RegistryMode mode)
475+
{
476+
_mode = mode;
477+
}
478+
479+
public IRegistryAPI Create(string registryName, Uri baseUri, ILogger logger)
480+
{
481+
return _registryApi ?? new DefaultRegistryAPI(registryName, baseUri, logger, _mode!.Value);
482+
}
483+
}
445484
}

‎src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImage.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ internal async Task<bool> ExecuteAsync(CancellationToken cancellationToken)
4848
return !Log.HasLoggedErrors;
4949
}
5050

51-
Registry? sourceRegistry = IsLocalPull ? null : new Registry(BaseRegistry, logger);
51+
RegistryMode sourceRegistryMode = BaseRegistry.Equals(OutputRegistry, StringComparison.InvariantCultureIgnoreCase) ? RegistryMode.PullFromOutput : RegistryMode.Pull;
52+
Registry? sourceRegistry = IsLocalPull ? null : new Registry(BaseRegistry, logger, sourceRegistryMode);
5253
SourceImageReference sourceImageReference = new(sourceRegistry, BaseImageName, BaseImageTag);
5354

5455
DestinationImageReference destinationImageReference = DestinationImageReference.CreateFromSettings(

‎src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/CreateNewImageTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ public async System.Threading.Tasks.Task CreateNewImage_RootlessBaseImage()
225225
var logger = loggerFactory.CreateLogger(nameof(CreateNewImage_RootlessBaseImage));
226226

227227
// Build a rootless base runtime image.
228-
Registry registry = new Registry(DockerRegistryManager.LocalRegistry, logger);
228+
Registry registry = new(DockerRegistryManager.LocalRegistry, logger, RegistryMode.Push);
229229

230230
ImageBuilder imageBuilder = await registry.GetImageManifestAsync(
231231
DockerRegistryManager.RuntimeBaseImage,

‎src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public async Task GetFromRegistry()
2222
{
2323
var loggerFactory = new TestLoggerFactory(_testOutput);
2424
var logger = loggerFactory.CreateLogger(nameof(GetFromRegistry));
25-
Registry registry = new Registry(DockerRegistryManager.LocalRegistry, logger);
25+
Registry registry = new(DockerRegistryManager.LocalRegistry, logger, RegistryMode.Push);
2626
var ridgraphfile = ToolsetUtils.GetRuntimeGraphFilePath();
2727

2828
// Don't need rid graph for local registry image pulls - since we're only pushing single image manifests (not manifest lists)
@@ -73,9 +73,9 @@ public async Task WriteToPrivateBasicRegistry()
7373
// login to that registry
7474
ContainerCli.LoginCommand(_testOutput, "--username", "testuser", "--password", "testpassword", registryName).Execute().Should().Pass();
7575
// push an image to that registry using username/password
76-
Registry localAuthed = new Registry(new Uri($"https://{registryName}"), logger, settings: new() { ParallelUploadEnabled = false, ForceChunkedUpload = true });
76+
Registry localAuthed = new(new Uri($"https://{registryName}"), logger, RegistryMode.Push, settings: new() { ParallelUploadEnabled = false, ForceChunkedUpload = true });
7777
var ridgraphfile = ToolsetUtils.GetRuntimeGraphFilePath();
78-
Registry mcr = new Registry(DockerRegistryManager.BaseImageSource, logger);
78+
Registry mcr = new(DockerRegistryManager.BaseImageSource, logger, RegistryMode.Pull);
7979

8080
var sourceImage = new SourceImageReference(mcr, DockerRegistryManager.RuntimeBaseImage, DockerRegistryManager.Net6ImageTag);
8181
var destinationImage = new DestinationImageReference(localAuthed, DockerRegistryManager.RuntimeBaseImage,new[] { DockerRegistryManager.Net6ImageTag });

‎src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/EndToEndTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public async Task ApiEndToEndWithRegistryPushAndPull()
4646

4747
// Build the image
4848

49-
Registry registry = new Registry(DockerRegistryManager.LocalRegistry, logger);
49+
Registry registry = new(DockerRegistryManager.LocalRegistry, logger, RegistryMode.Push);
5050

5151
ImageBuilder imageBuilder = await registry.GetImageManifestAsync(
5252
DockerRegistryManager.RuntimeBaseImage,
@@ -93,7 +93,7 @@ public async Task ApiEndToEndWithLocalLoad()
9393

9494
// Build the image
9595

96-
Registry registry = new Registry(DockerRegistryManager.LocalRegistry, logger);
96+
Registry registry = new(DockerRegistryManager.LocalRegistry, logger, RegistryMode.Push);
9797

9898
ImageBuilder imageBuilder = await registry.GetImageManifestAsync(
9999
DockerRegistryManager.RuntimeBaseImage,
@@ -134,7 +134,7 @@ public async Task ApiEndToEndWithArchiveWritingAndLoad()
134134

135135
// Build the image
136136

137-
Registry registry = new Registry(DockerRegistryManager.LocalRegistry, logger);
137+
Registry registry = new(DockerRegistryManager.LocalRegistry, logger, RegistryMode.Push);
138138

139139
ImageBuilder imageBuilder = await registry.GetImageManifestAsync(
140140
DockerRegistryManager.RuntimeBaseImage,
@@ -479,7 +479,7 @@ public async Task CanPackageForAllSupportedContainerRIDs(string dockerPlatform,
479479
string publishDirectory = BuildLocalApp(tfm: ToolsetInfo.CurrentTargetFramework, rid: rid);
480480

481481
// Build the image
482-
Registry registry = new(DockerRegistryManager.BaseImageSource, logger);
482+
Registry registry = new(DockerRegistryManager.BaseImageSource, logger, RegistryMode.Pull);
483483
var isWin = rid.StartsWith("win");
484484
ImageBuilder? imageBuilder = await registry.GetImageManifestAsync(
485485
DockerRegistryManager.RuntimeBaseImage,

0 commit comments

Comments
 (0)