Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,7 @@ private ProcessStartInfo MakeDotnetStartInfo(List<string> args, string? workingD
}

// Configure the proxy settings, if applicable.
if (this.proxy != null)
{
logger.LogDebug($"Configuring environment variables for the registry proxy at {this.proxy.Address}");

startInfo.EnvironmentVariables["HTTP_PROXY"] = this.proxy.Address;
startInfo.EnvironmentVariables["HTTPS_PROXY"] = this.proxy.Address;
startInfo.EnvironmentVariables["SSL_CERT_FILE"] = this.proxy.CertificatePath;
}
proxy?.SetProcessEnvironment(startInfo);

return startInfo;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Security.Cryptography.X509Certificates;

namespace Semmle.Extraction.CSharp.DependencyFetching
Expand Down Expand Up @@ -30,5 +31,11 @@ public interface IRegistryProxy : IDisposable
/// The certificate used for the registry proxy.
/// </summary>
X509Certificate2? Certificate { get; }

/// <summary>
/// Configures the environment variables for a process to use the registry proxy.
/// </summary>
/// <param name="pi">The process start info to configure.</param>
void SetProcessEnvironment(ProcessStartInfo pi);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ internal sealed partial class NugetPackageRestorer : IDisposable
private readonly IFileProvider fileProvider;
private readonly FileContent fileContent;
private readonly IDotNet dotnet;
private readonly IRegistryProxy? registryProxy;
private readonly IDiagnosticsWriter diagnosticsWriter;
private readonly DependencyDirectory legacyPackageDirectory;
private readonly DependencyDirectory missingPackageDirectory;
Expand All @@ -40,6 +41,7 @@ public NugetPackageRestorer(
this.fileProvider = fileProvider;
this.fileContent = fileContent;
this.dotnet = dotnet;
this.registryProxy = registryProxy;
this.diagnosticsWriter = diagnosticsWriter;
this.logger = logger;
this.compilationInfoContainer = compilationInfoContainer;
Expand Down Expand Up @@ -133,7 +135,7 @@ public HashSet<AssemblyLookupLocation> Restore()

try
{
var packagesConfigRestore = PackagesConfigRestoreFactory.Create(fileProvider, legacyPackageDirectory, logger, feedManager);
var packagesConfigRestore = PackagesConfigRestoreFactory.Create(fileProvider, legacyPackageDirectory, logger, feedManager, registryProxy);
var count = packagesConfigRestore.InstallPackages();
if (packagesConfigRestore.PackageCount > 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ internal interface IPackagesConfigRestore
/// </summary>
internal class PackagesConfigRestoreFactory
{
public static IPackagesConfigRestore Create(IFileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager)
public static IPackagesConfigRestore Create(IFileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager, IRegistryProxy? registryProxy)
{
if (SystemBuildActions.Instance.IsWindows() || SystemBuildActions.Instance.IsMonoInstalled())
{
return new NugetExeWrapper(fileProvider, packageDirectory, logger, feedManager);
return new NugetExeWrapper(fileProvider, packageDirectory, logger, feedManager, registryProxy);
}

return new NoOpPackagesConfig(fileProvider.PackagesConfigs, logger);
Expand All @@ -52,6 +52,7 @@ private class NugetExeWrapper : IPackagesConfigRestore
{
private readonly string? nugetExe;
private readonly Semmle.Util.Logging.ILogger logger;
private readonly IRegistryProxy? registryProxy;

public int PackageCount => fileProvider.PackagesConfigs.Count;

Expand All @@ -70,12 +71,13 @@ private class NugetExeWrapper : IPackagesConfigRestore
/// <summary>
/// Create the package manager for a specified source tree.
/// </summary>
public NugetExeWrapper(IFileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager)
public NugetExeWrapper(IFileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager, IRegistryProxy? registryProxy)
{
this.fileProvider = fileProvider;
this.packageDirectory = packageDirectory;
this.logger = logger;
this.feedManager = feedManager;
this.registryProxy = registryProxy;

if (fileProvider.PackagesConfigs.Count > 0)
{
Expand Down Expand Up @@ -209,9 +211,13 @@ private bool TryRestoreNugetPackage(string packagesConfig)
UseShellExecute = false
};

// Configure the proxy settings, if applicable.
registryProxy?.SetProcessEnvironment(pi);

var threadId = Environment.CurrentManagedThreadId;
void onOut(string s) => logger.LogDebug(s, threadId);
void onError(string s) => logger.LogError(s, threadId);
logger.LogInfo($"Running '{pi.FileName} {string.Join(" ", pi.ArgumentList)}'");
var exitCode = pi.ReadOutput(out _, onOut, onError);
if (exitCode != 0)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Immutable;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Security.Cryptography.X509Certificates;
using Semmle.Util;
Expand Down Expand Up @@ -36,6 +37,8 @@ public class RegistryConfig

public string Address { get; }

private readonly ILogger logger;

/// <summary>
/// A dictionary mapping registry URLs to a boolean indicating whether they replace the base registry.
/// </summary>
Expand Down Expand Up @@ -65,8 +68,9 @@ public class RegistryConfig

public X509Certificate2? Certificate { get; private set; }

private RegistryProxy(IRegistryProxyConfiguration config, ILogger logger, TemporaryDirectory tempWorkingDirectory)
private RegistryProxy(IRegistryProxyConfiguration config, ILogger l, TemporaryDirectory tempWorkingDirectory)
{
logger = l;
Address = $"http://{config.Host}:{config.Port}";

if (!string.IsNullOrWhiteSpace(config.Certificate))
Expand Down Expand Up @@ -179,6 +183,28 @@ private RegistryProxy(IRegistryProxyConfiguration config, ILogger logger, Tempor
return result;
}

public void SetProcessEnvironment(ProcessStartInfo pi)
{
logger.LogDebug($"Configuring environment variables for the registry proxy at {Address}");

pi.EnvironmentVariables["HTTP_PROXY"] = Address;
pi.EnvironmentVariables["HTTPS_PROXY"] = Address;
Comment thread
michaelnebel marked this conversation as resolved.

// Also set the lower case variants of the environment variables
// This might be needed on Linux systems.
pi.EnvironmentVariables["http_proxy"] = Address;
pi.EnvironmentVariables["https_proxy"] = Address;

if (CertificatePath != null)
{
pi.EnvironmentVariables["SSL_CERT_FILE"] = CertificatePath;
}
else
{
logger.LogDebug("No SSL certificate is configured for the registry proxy.");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: This makes sense to log when we are setting the value for CertificatePath after retrieving the certificate from the CODEQL_PROXY_ var, but maybe not here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, we should just log when it is set instead - just to avoid that it happens silently (then we always know whether it happens or not).

}

public void Dispose()
{
Certificate?.Dispose();
Expand Down
4 changes: 4 additions & 0 deletions csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Security.Cryptography.X509Certificates;
Expand All @@ -17,6 +18,8 @@ public class RegistryProxyStub : IRegistryProxy
public string? CertificatePath { get; } = null;
public X509Certificate2? Certificate { get; } = null;

public void SetProcessEnvironment(ProcessStartInfo pi) { }

public void Dispose() { }
}

Expand All @@ -28,6 +31,7 @@ public class RegistryProxyStubWithBaseUrls : IRegistryProxy
public string? CertificatePath { get; } = null;
public X509Certificate2? Certificate { get; } = null;

public void SetProcessEnvironment(ProcessStartInfo pi) { }
public void Dispose() { }
}

Expand Down
36 changes: 36 additions & 0 deletions csharp/extractor/Semmle.Extraction.Tests/RegistryProxy.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Xunit;
using System;
using System.Diagnostics;
using System.IO;
using Semmle.Extraction.CSharp.DependencyFetching;
using Semmle.Util;
Expand Down Expand Up @@ -245,5 +246,40 @@ public void TestRegistryProxyUrlsReplacesBase()
"https://example.com/org/index.json",
], proxy.RegistryBaseURLs);
}

/// <summary>
/// Verifies that the registry proxy correctly sets the environment variables needed for a .NET process to use the proxy.
/// In this case, the environment variables for the HTTP and HTTPS proxies, as well as the SSL certificate file, should be correctly set.
/// The http proxies should be set to the proxy address, and the SSL certificate file should point to the certificate path.
/// The latter is tested by verifying that the SSL_CERT_FILE environment variable ends with "proxy.crt" as we can't check the absolute path
/// due to temporary directories.
/// </summary>
[Fact]
public void TestRegistryProxyProcessEnvironment()
{
// Setup
var config = new RegistryConfigurationStub
{
Port = "8080",
Host = "localhost",
Certificate = ExampleCertificate
};

// Execute
using var tempWorkingDirectory = MakeTemporaryDirectory();
using var proxy = RegistryProxy.Make(config, new LoggerStub(), new DiagnosticsWriterStub(), tempWorkingDirectory);

var pi = new ProcessStartInfo("nuget");
proxy?.SetProcessEnvironment(pi);

// Verify
Assert.NotNull(proxy);
Assert.Equal("http://localhost:8080", proxy.Address);
Assert.Equal("http://localhost:8080", pi.EnvironmentVariables["HTTP_PROXY"]);
Assert.Equal("http://localhost:8080", pi.EnvironmentVariables["HTTPS_PROXY"]);
Assert.Equal("http://localhost:8080", pi.EnvironmentVariables["http_proxy"]);
Assert.Equal("http://localhost:8080", pi.EnvironmentVariables["https_proxy"]);
Assert.EndsWith("proxy.crt", pi.EnvironmentVariables["SSL_CERT_FILE"]);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Proxy and certificate environment variables are now set for the subprocess that invokes the NuGet CLI, enabling it to access private registries during this part of the workflow.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Change "Proxy" to "Private registry proxy" to disambiguate from ordinary proxies that may be configured on a system. Alternatively, since "private registries" are already mentioned in the last part of the note, restructure the sentence to emphasise that part. For example:

Suggested change
* Proxy and certificate environment variables are now set for the subprocess that invokes the NuGet CLI, enabling it to access private registries during this part of the workflow.
* The subprocess for the NuGet CLI is now provided with the proxy and certificate environment variables needed to access private registries if any are configured.

Loading