Skip to content

Commit b09505b

Browse files
tmdstmds
committed
Add tests.
1 parent bb692ed commit b09505b

File tree

4 files changed

+211
-31
lines changed

4 files changed

+211
-31
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
3030
}
3131

3232
bool canFallback = request.RequestUri.Host == _host && request.RequestUri.Port == _port && request.RequestUri.Scheme == "https";
33-
bool canRetry = true;
33+
bool canRetry = canFallback;
3434
do
3535
{
3636
try
@@ -45,7 +45,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
4545

4646
return await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
4747
}
48-
catch (HttpRequestException re) when (canFallback && canRetry && re.HttpRequestError == HttpRequestError.SecureConnectionError)
48+
catch (HttpRequestException re) when (canRetry && re.HttpRequestError == HttpRequestError.SecureConnectionError)
4949
{
5050
_fallbackToHttp = true;
5151
}

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

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,8 @@ internal Registry(Uri baseUri, ILogger logger, IRegistryAPI? registryAPI = null,
8686
BaseUri = baseUri;
8787

8888
_logger = logger;
89-
_settings = settings ?? new RegistrySettings();
90-
bool isInsecureRegistry = IsInsecureRegistry(RegistryName);
91-
_registryAPI = registryAPI ?? new DefaultRegistryAPI(RegistryName, BaseUri, isInsecureRegistry, logger);
89+
_settings = settings ?? new RegistrySettings(RegistryName);
90+
_registryAPI = registryAPI ?? new DefaultRegistryAPI(RegistryName, BaseUri, _settings.IsInsecure, logger);
9291
}
9392

9493
private static string DeriveRegistryName(Uri baseUri)
@@ -106,29 +105,6 @@ private static string DeriveRegistryName(Uri baseUri)
106105
}
107106
}
108107

109-
private static bool IsInsecureRegistry(string registryName)
110-
{
111-
// Allow insecure access to 'localhost'.
112-
if (registryName.StartsWith("localhost:", StringComparison.OrdinalIgnoreCase) ||
113-
registryName.Equals("localhost:", StringComparison.OrdinalIgnoreCase))
114-
{
115-
return true;
116-
}
117-
118-
// SDK_CONTAINER_INSECURE_REGISTRIES is a semicolon separated list of insecure registry names.
119-
string? insecureRegistriesEnv = Environment.GetEnvironmentVariable("SDK_CONTAINER_INSECURE_REGISTRIES");
120-
if (insecureRegistriesEnv is not null)
121-
{
122-
string[] insecureRegistries = insecureRegistriesEnv.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
123-
if (Array.Exists(insecureRegistries, registry => registryName.Equals(registry, StringComparison.OrdinalIgnoreCase)))
124-
{
125-
return true;
126-
}
127-
}
128-
129-
return DockerCli.IsInsecureRegistry(registryName);
130-
}
131-
132108
public Uri BaseUri { get; }
133109

134110
/// <summary>

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,20 @@ namespace Microsoft.NET.Build.Containers;
88

99
internal class RegistrySettings
1010
{
11+
public RegistrySettings(string? registryName = null, IEnvironmentProvider? environment = null)
12+
{
13+
environment ??= new EnvironmentProvider();
14+
15+
ChunkedUploadSizeBytes = environment.GetEnvironmentVariableAsNullableInt(EnvVariables.ChunkedUploadSizeBytes);
16+
ForceChunkedUpload = environment.GetEnvironmentVariableAsBool(EnvVariables.ForceChunkedUpload, defaultValue: false);
17+
ParallelUploadEnabled = environment.GetEnvironmentVariableAsBool(EnvVariables.ParallelUploadEnabled, defaultValue: true);
18+
19+
if (registryName is not null)
20+
{
21+
IsInsecure = IsInsecureRegistry(environment, registryName);
22+
}
23+
}
24+
1125
private const int DefaultChunkSizeBytes = 1024 * 64;
1226
private const int FiveMegs = 5_242_880;
1327

@@ -17,26 +31,56 @@ internal class RegistrySettings
1731
/// <remarks>
1832
/// Our default of 64KB is very conservative, so raising this to 1MB or more can speed up layer uploads reasonably well.
1933
/// </remarks>
20-
internal int? ChunkedUploadSizeBytes { get; init; } = Env.GetEnvironmentVariableAsNullableInt(EnvVariables.ChunkedUploadSizeBytes);
34+
internal int? ChunkedUploadSizeBytes { get; init; }
2135

2236
/// <summary>
2337
/// Allows to force chunked upload for debugging purposes.
2438
/// </summary>
25-
internal bool ForceChunkedUpload { get; init; } = Env.GetEnvironmentVariableAsBool(EnvVariables.ForceChunkedUpload, defaultValue: false);
39+
internal bool ForceChunkedUpload { get; init; }
2640

2741
/// <summary>
2842
/// Whether we should upload blobs in parallel (enabled by default, but disabled for certain registries in conjunction with the explicit support check below).
2943
/// </summary>
3044
/// <remarks>
3145
/// Enabling this can swamp some registries, so this is an escape hatch.
3246
/// </remarks>
33-
internal bool ParallelUploadEnabled { get; init; } = Env.GetEnvironmentVariableAsBool(EnvVariables.ParallelUploadEnabled, defaultValue: true);
47+
internal bool ParallelUploadEnabled { get; init; }
48+
49+
/// <summary>
50+
/// Allows ignoring https certificate errors and changing to http when the endpoint is not an https endpoint.
51+
/// </summary>
52+
internal bool IsInsecure { get; init; }
3453

3554
internal struct EnvVariables
3655
{
3756
internal const string ChunkedUploadSizeBytes = "SDK_CONTAINER_REGISTRY_CHUNKED_UPLOAD_SIZE_BYTES";
3857

3958
internal const string ForceChunkedUpload = "SDK_CONTAINER_DEBUG_REGISTRY_FORCE_CHUNKED_UPLOAD";
4059
internal const string ParallelUploadEnabled = "SDK_CONTAINER_REGISTRY_PARALLEL_UPLOAD";
60+
61+
internal const string InsecureRegistries = "SDK_CONTAINER_INSECURE_REGISTRIES";
62+
}
63+
64+
private static bool IsInsecureRegistry(IEnvironmentProvider environment, string registryName)
65+
{
66+
// Always allow insecure access to 'localhost'.
67+
if (registryName.StartsWith("localhost:", StringComparison.OrdinalIgnoreCase) ||
68+
registryName.Equals("localhost", StringComparison.OrdinalIgnoreCase))
69+
{
70+
return true;
71+
}
72+
73+
// SDK_CONTAINER_INSECURE_REGISTRIES is a semicolon separated list of insecure registry names.
74+
string? insecureRegistriesEnv = environment.GetEnvironmentVariable(EnvVariables.InsecureRegistries);
75+
if (insecureRegistriesEnv is not null)
76+
{
77+
string[] insecureRegistries = insecureRegistriesEnv.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
78+
if (Array.Exists(insecureRegistries, registry => registryName.Equals(registry, StringComparison.OrdinalIgnoreCase)))
79+
{
80+
return true;
81+
}
82+
}
83+
84+
return DockerCli.IsInsecureRegistry(registryName);
4185
}
4286
}

‎test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Net;
5+
using System.Net.Security;
6+
using System.Security.Cryptography;
7+
using System.Security.Cryptography.X509Certificates;
8+
using Microsoft.DotNet.Cli.Utils;
59
using Microsoft.Extensions.Logging;
610
using Microsoft.NET.Build.Containers.Resources;
11+
using System.Net.Sockets;
712
using Moq;
813

914
namespace Microsoft.NET.Build.Containers.UnitTests;
@@ -390,11 +395,166 @@ public async Task UploadBlobChunkedAsync_Failure()
390395
api.Verify(api => api.Blob.Upload.UploadChunkAsync(It.IsIn(absoluteUploadUri, uploadPath), It.IsAny<HttpContent>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
391396
}
392397

398+
[InlineData(true, true)]
399+
[InlineData(true, false)]
400+
[InlineData(false, true)]
401+
[InlineData(false, false)]
402+
[Theory]
403+
public async Task InsecureRegistry(bool serverIsHttps, bool isInsecureRegistry)
404+
{
405+
ILogger logger = _loggerFactory.CreateLogger(nameof(InsecureRegistry));
406+
407+
// Start a dummy HTTP server that response with 200 OK.
408+
using TcpListener listener = new TcpListener(IPAddress.Loopback, 0);
409+
listener.Start();
410+
IPEndPoint endpoint = (listener.LocalEndpoint as IPEndPoint)!;
411+
Uri registryUri = new Uri($"https://{endpoint.Address}:{endpoint.Port}");
412+
SslServerAuthenticationOptions? sslOptions = null!;
413+
if (serverIsHttps)
414+
{
415+
var key = RSA.Create(2048);
416+
var request = new CertificateRequest("CN=localhost", key, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
417+
X509Certificate2 serverCertificate = request.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddYears(1));
418+
sslOptions = new SslServerAuthenticationOptions()
419+
{
420+
ServerCertificate = serverCertificate,
421+
ClientCertificateRequired = false
422+
};
423+
}
424+
_ = Task.Run(async () =>
425+
{
426+
while (true)
427+
{
428+
using TcpClient client = await listener.AcceptTcpClientAsync();
429+
try
430+
{
431+
using Stream stream = serverIsHttps ? new SslStream(client.GetStream(), leaveInnerStreamOpen: false) : client.GetStream();
432+
if (stream is SslStream sslStream)
433+
{
434+
await sslStream.AuthenticateAsServerAsync(sslOptions!, default(CancellationToken));
435+
}
436+
await stream.WriteAsync("HTTP/1.0 200 OK\r\nContent-Length: 0\r\n\r\n"u8.ToArray());
437+
}
438+
catch
439+
{ }
440+
}
441+
});
442+
443+
RegistrySettings settings = new()
444+
{
445+
IsInsecure = isInsecureRegistry
446+
};
447+
Registry registry = new(registryUri, logger, settings: settings);
448+
449+
// Make a request.
450+
Task getManifest = registry.GetImageManifestAsync(repositoryName: "dotnet/runtime", reference: "latest", runtimeIdentifier: "linux-x64", manifestPicker: null!, cancellationToken: default!);
451+
452+
if (isInsecureRegistry)
453+
{
454+
// Falls back to http (when serverIsHttps is false) or ignores https certificate errors (when serverIsHttps is true).
455+
// Results in throwing: CONTAINER2003: The manifest for dotnet/runtime:latest from registry hwas an unknown type.
456+
await Assert.ThrowsAsync<NotImplementedException>(() => getManifest);
457+
}
458+
else
459+
{
460+
// Does not fall back and throws HttpRequestException.
461+
var requestException = await Assert.ThrowsAsync<HttpRequestException>(() => getManifest);
462+
Assert.Equal(HttpRequestError.SecureConnectionError, requestException.HttpRequestError);
463+
}
464+
}
465+
466+
[InlineData("localhost", null, true)]
467+
[InlineData("localhost:5000", null, true)]
468+
[InlineData("public.ecr.aws", null, false)]
469+
[InlineData("public.ecr.aws", "public.ecr.aws", true)]
470+
[InlineData("public.ecr.aws", "Public.ecr.aws", true)] // ignore case
471+
[InlineData("public.ecr.aws", "public.ecr.aws;docker.io", true)] // multiple registries
472+
[InlineData("public.ecr.aws", ";public.ecr.aws ; docker.io ", true)] // ignore whitespace
473+
[InlineData("public.ecr.aws", "public.ecr.aws2;docker.io ", false)] // full name match
474+
[Theory]
475+
public void IsRegistryInsecure(string registryName, string? insecureRegistriesEnvvar, bool expectedInsecure)
476+
{
477+
var environment = new Dictionary<string, string>();
478+
if (insecureRegistriesEnvvar is not null)
479+
{
480+
environment["SDK_CONTAINER_INSECURE_REGISTRIES"] = insecureRegistriesEnvvar;
481+
}
393482

483+
var registrySettings = new RegistrySettings(registryName, new MockEnvironmentProvider(environment));
484+
485+
Assert.Equal(expectedInsecure, registrySettings.IsInsecure);
486+
}
394487

395488
private static NextChunkUploadInformation ChunkUploadSuccessful(Uri requestUri, Uri uploadUrl, int? contentLength, HttpStatusCode code = HttpStatusCode.Accepted)
396489
{
397490
return new(uploadUrl);
398491
}
399492

493+
private class MockEnvironmentProvider : IEnvironmentProvider
494+
{
495+
private readonly IDictionary<string, string> _environmentVariables;
496+
497+
public MockEnvironmentProvider(IDictionary<string, string> environmentVariables)
498+
{
499+
_environmentVariables = environmentVariables;
500+
}
501+
502+
public bool GetEnvironmentVariableAsBool(string name, bool defaultValue)
503+
{
504+
var str = Environment.GetEnvironmentVariable(name);
505+
if (string.IsNullOrEmpty(str))
506+
{
507+
return defaultValue;
508+
}
509+
510+
switch (str.ToLowerInvariant())
511+
{
512+
case "true":
513+
case "1":
514+
case "yes":
515+
return true;
516+
case "false":
517+
case "0":
518+
case "no":
519+
return false;
520+
default:
521+
return defaultValue;
522+
}
523+
}
524+
525+
public string? GetEnvironmentVariable(string name)
526+
{
527+
string? value;
528+
_environmentVariables.TryGetValue(name, out value);
529+
return value;
530+
}
531+
532+
public string? GetEnvironmentVariable(string variable, EnvironmentVariableTarget target)
533+
=> GetEnvironmentVariable(variable);
534+
535+
public int? GetEnvironmentVariableAsNullableInt(string variable)
536+
{
537+
if (GetEnvironmentVariable(variable) is string strValue && int.TryParse(strValue, out int intValue))
538+
{
539+
return intValue;
540+
}
541+
542+
return null;
543+
}
544+
545+
public void SetEnvironmentVariable(string variable, string value, EnvironmentVariableTarget target)
546+
=> throw new NotImplementedException();
547+
548+
public IEnumerable<string> ExecutableExtensions
549+
=> throw new NotImplementedException();
550+
551+
public string GetCommandPath(string commandName, params string[] extensions)
552+
=> throw new NotImplementedException();
553+
554+
public string GetCommandPathFromRootPath(string rootPath, string commandName, params string[] extensions)
555+
=> throw new NotImplementedException();
556+
557+
public string GetCommandPathFromRootPath(string rootPath, string commandName, IEnumerable<string> extensions)
558+
=> throw new NotImplementedException();
559+
}
400560
}

0 commit comments

Comments
 (0)