Skip to content

Commit

Permalink
Merged PR 546518: Making external sandboxed process executor work on …
Browse files Browse the repository at this point in the history
…non-Windows

Making external sandboxed process executor work on non-Windows.

This work is needed for AnyBuild.
  • Loading branch information
narasamdya committed Apr 15, 2020
1 parent 61d3b17 commit 0f59d16
Show file tree
Hide file tree
Showing 10 changed files with 193 additions and 48 deletions.
3 changes: 3 additions & 0 deletions Public/Src/Deployment/Tests.Linux/tests.linux.dsc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace Tests.Linux {
createDef(importFrom("BuildXL.Core.UnitTests").Cache.dll, true),
createDef(importFrom("BuildXL.Core.UnitTests").Cache.Plugin.Core.dll, true),
createDef(importFrom("BuildXL.Core.UnitTests").Processes.test_BuildXL_Processes_dll, true),
createDef(importFrom("BuildXL.Core.UnitTests").ExternalToolTest.dll, true),
createDef(importFrom("BuildXL.Core.UnitTests").Scheduler.IntegrationTest.dll, true,
/* deploySeparately */ false,
/* testClasses */ undefined,
Expand Down Expand Up @@ -233,6 +234,8 @@ namespace Tests.Linux {
"MY_DIR=$(cd `dirname ${BASH_SOURCE[0]}` && pwd)",
"source $MY_DIR/xunit_runner.sh",
"",
"find . -name SandboxedProcessExecutor -exec chmod +x {} \\;",
"",
"numTestFailures=0",
"trap \"((numTestFailures++))\" ERR",
"",
Expand Down
4 changes: 1 addition & 3 deletions Public/Src/Deployment/buildXL.dsc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ namespace BuildXL {
importFrom("BuildXL.Tools").BxlPipGraphFragmentGenerator.exe,
importFrom("BuildXL.Cache.VerticalStore").Analyzer.exe,

...addIfLazy(qualifier.targetRuntime === "win-x64", () => [
importFrom("BuildXL.Tools").SandboxedProcessExecutor.exe,
]),
importFrom("BuildXL.Tools").SandboxedProcessExecutor.exe,

// tools
...addIfLazy(qualifier.targetRuntime === "win-x64", () => [{
Expand Down
17 changes: 15 additions & 2 deletions Public/Src/Engine/Processes/ExternalSandboxedProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,29 @@ protected void SerializeSandboxedProcessInfoToFile()
/// <summary>
/// Deserializes sandboxed process result from file.
/// </summary>
/// <returns></returns>
protected SandboxedProcessResult DeserializeSandboxedProcessResultFromFile()
{
string file = SandboxedProcessResultsFile;

Func<BuildXLReader, AbsolutePath> readPath = reader =>
{
bool isAbsolutePath = reader.ReadBoolean();
if (isAbsolutePath)
{
return reader.ReadAbsolutePath();
}
else
{
string path = reader.ReadString();
return AbsolutePath.Create(SandboxedProcessInfo.PathTable, path);
}
};

try
{
using (FileStream stream = File.OpenRead(file))
{
return SandboxedProcessResult.Deserialize(stream);
return SandboxedProcessResult.Deserialize(stream, readPath);
}
}
catch (IOException ioException)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class ExternalToolSandboxedProcessExecutor
/// <summary>
/// Relative path to the default tool.
/// </summary>
public const string DefaultToolRelativePath = @"SandboxedProcessExecutor.exe";
public static readonly string DefaultToolRelativePath = "SandboxedProcessExecutor" + (!OperatingSystemHelper.IsUnixOS ? ".exe" :string.Empty);

/// <summary>
/// Tool path.
Expand Down
65 changes: 50 additions & 15 deletions Public/Src/Engine/Processes/SandboxedProcessPipExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -792,20 +792,22 @@ public async Task<SandboxedProcessPipExecutionResult> RunAsync(
}
}

private bool SandboxedProcessNeedsExecuteExternal
=> // Execution mode is external
m_sandboxConfig.AdminRequiredProcessExecutionMode.ExecuteExternal()
// Only pip that requires admin privilege.
&& m_pip.RequiresAdmin;

private bool ShouldSandboxedProcessExecuteExternal
=> SandboxedProcessNeedsExecuteExternal
// Container is disabled.
&& !m_containerConfiguration.IsIsolationEnabled
// Windows only.
&& !OperatingSystemHelper.IsUnixOS;

private bool ShouldSandboxedProcessExecuteInVm => ShouldSandboxedProcessExecuteExternal && m_sandboxConfig.AdminRequiredProcessExecutionMode.ExecuteExternalVm();
private bool SandboxedProcessNeedsExecuteExternal =>
// Execution mode is external
m_sandboxConfig.AdminRequiredProcessExecutionMode.ExecuteExternal()
// Only pip that requires admin privilege.
&& m_pip.RequiresAdmin;

private bool ShouldSandboxedProcessExecuteExternal =>
SandboxedProcessNeedsExecuteExternal
// Container is disabled.
&& !m_containerConfiguration.IsIsolationEnabled;

private bool ShouldSandboxedProcessExecuteInVm =>
ShouldSandboxedProcessExecuteExternal
&& m_sandboxConfig.AdminRequiredProcessExecutionMode.ExecuteExternalVm()
// Windows only.
&& !OperatingSystemHelper.IsUnixOS;

private async Task<SandboxedProcessPipExecutionResult> RunInternalAsync(
SandboxedProcessInfo info,
Expand Down Expand Up @@ -1010,7 +1012,7 @@ private async Task<SandboxedProcessPipExecutionResult> RunExternalAsync(
}
else
{
Contract.Assert(m_sandboxConfig.AdminRequiredProcessExecutionMode == AdminRequiredProcessExecutionMode.ExternalVM);
Contract.Assert(ShouldSandboxedProcessExecuteInVm);

// Initialize VM once.
await m_vmInitializer.LazyInitVmAsync.Value;
Expand Down Expand Up @@ -2227,6 +2229,11 @@ private void TryPrepareFileAccessMonitoringForPip(Process pip, HashSet<AbsoluteP
}
}

if (OperatingSystemHelper.IsUnixOS)
{
AddUnixSpecificSandboxedProcessFileAccessPolicies();
}

m_fileAccessManifest.MonitorChildProcesses = !pip.HasUntrackedChildProcesses;

if (!string.IsNullOrEmpty(m_detoursFailuresFile))
Expand Down Expand Up @@ -2259,6 +2266,34 @@ void addUntrackedScope(AbsolutePath untrackedScope, HashSet<AbsolutePath> proces
}
}

private void AddUnixSpecificSandboxedProcessFileAccessPolicies()
{
Contract.Requires(OperatingSystemHelper.IsUnixOS);

if (ShouldSandboxedProcessExecuteExternal)
{
// When executing the pip using external tool, the file access manifest tree is sealed by
// serializing it as bytes. Thus, after the external tool deserializes the manifest tree,
// the manifest cannot be modified further.

// CODESYNC: SandboxedProcessUnix.cs
// The sandboxed process for unix modifies the manifest tree. We do the same modification here.
m_fileAccessManifest.AddPath(
AbsolutePath.Create(m_pathTable, SandboxedProcessUnix.ShellExecutable),
mask: FileAccessPolicy.MaskNothing,
values: FileAccessPolicy.AllowReadAlways);

AbsolutePath stdInFile = AbsolutePath.Create(
m_pathTable,
SandboxedProcessUnix.GetStdInFilePath(m_pip.WorkingDirectory.ToString(m_pathTable), m_pip.SemiStableHash));

m_fileAccessManifest.AddPath(
stdInFile,
mask: FileAccessPolicy.MaskNothing,
values: FileAccessPolicy.AllowAll);
}
}

/// <summary>
/// If a supplied path is under real/redirected user profile, creates a corresponding path under redirected/real user profile.
/// If profile redirect is disabled or the path is not under real/redirected user profile, returns AbsolutePath.Invalid.
Expand Down
26 changes: 12 additions & 14 deletions Public/Src/Engine/Processes/SandboxedProcessResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,18 @@ public sealed class SandboxedProcessResult
/// <summary>
/// Serializes this instance to a given <paramref name="stream"/>.
/// </summary>
public void Serialize(Stream stream)
public void Serialize(Stream stream, Action<BuildXLWriter, AbsolutePath> writePath = null)
{
using (var writer = new BuildXLWriter(false, stream, true, true))
{
Serialize(writer);
Serialize(writer, writePath);
}
}

/// <summary>
/// Serializes this instance to a given <paramref name="writer"/>.
/// </summary>
public void Serialize(BuildXLWriter writer)
public void Serialize(BuildXLWriter writer, Action<BuildXLWriter, AbsolutePath> writePath = null)
{
writer.Write(ExitCode);
writer.Write(Killed);
Expand All @@ -177,9 +177,9 @@ public void Serialize(BuildXLWriter writer)
writer.Write(JobAccountingInformation, (w, v) => v.Serialize(w));
writer.Write(StandardOutput, (w, v) => v.Serialize(w));
writer.Write(StandardError, (w, v) => v.Serialize(w));
writer.Write(FileAccesses, (w, v) => w.WriteReadOnlyList(v.ToList(), (w2, v2) => v2.Serialize(writer, processMap, writePath: null)));
writer.Write(ExplicitlyReportedFileAccesses, (w, v) => w.WriteReadOnlyList(v.ToList(), (w2, v2) => v2.Serialize(writer, processMap, writePath: null)));
writer.Write(AllUnexpectedFileAccesses, (w, v) => w.WriteReadOnlyList(v.ToList(), (w2, v2) => v2.Serialize(writer, processMap, writePath: null)));
writer.Write(FileAccesses, (w, v) => w.WriteReadOnlyList(v.ToList(), (w2, v2) => v2.Serialize(writer, processMap, writePath: writePath)));
writer.Write(ExplicitlyReportedFileAccesses, (w, v) => w.WriteReadOnlyList(v.ToList(), (w2, v2) => v2.Serialize(writer, processMap, writePath: writePath)));
writer.Write(AllUnexpectedFileAccesses, (w, v) => w.WriteReadOnlyList(v.ToList(), (w2, v2) => v2.Serialize(writer, processMap, writePath: writePath)));
writer.Write(Processes, (w, v) => w.WriteReadOnlyList(v, (w2, v2) => w2.Write(processMap[v2])));
writer.Write(DetouringStatuses, (w, v) => w.WriteReadOnlyList(v, (w2, v2) => v2.Serialize(w2)));
writer.WriteNullableString(DumpFileDirectory);
Expand All @@ -198,20 +198,18 @@ public void Serialize(BuildXLWriter writer)
/// <summary>
/// Deserializes an instance of <see cref="SandboxedProcessResult"/>.
/// </summary>
public static SandboxedProcessResult Deserialize(Stream stream)
public static SandboxedProcessResult Deserialize(Stream stream, Func<BuildXLReader, AbsolutePath> readPath = null)
{
using (var reader = new BuildXLReader(false, stream, true))
{
return Deserialize(reader);
return Deserialize(reader, readPath);
}
}

/// <summary>
/// Deserializes an instance of <see cref="SandboxedProcessResult"/>.
/// </summary>
/// <param name="reader"></param>
/// <returns></returns>
public static SandboxedProcessResult Deserialize(BuildXLReader reader)
public static SandboxedProcessResult Deserialize(BuildXLReader reader, Func<BuildXLReader, AbsolutePath> readPath = null)
{
int exitCode = reader.ReadInt32();
bool killed = reader.ReadBoolean();
Expand All @@ -224,9 +222,9 @@ public static SandboxedProcessResult Deserialize(BuildXLReader reader)
JobObject.AccountingInformation? jobAccountingInformation = reader.ReadNullableStruct(r => JobObject.AccountingInformation.Deserialize(r));
SandboxedProcessOutput standardOutput = reader.ReadNullable(r => SandboxedProcessOutput.Deserialize(r));
SandboxedProcessOutput standardError = reader.ReadNullable(r => SandboxedProcessOutput.Deserialize(r));
IReadOnlyList<ReportedFileAccess> fileAccesses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ReportedFileAccess.Deserialize(r2, allReportedProcesses, readPath: null)));
IReadOnlyList<ReportedFileAccess> explicitlyReportedFileAccesses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ReportedFileAccess.Deserialize(r2, allReportedProcesses, readPath: null)));
IReadOnlyList<ReportedFileAccess> allUnexpectedFileAccesses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ReportedFileAccess.Deserialize(r2, allReportedProcesses, readPath: null)));
IReadOnlyList<ReportedFileAccess> fileAccesses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ReportedFileAccess.Deserialize(r2, allReportedProcesses, readPath: readPath)));
IReadOnlyList<ReportedFileAccess> explicitlyReportedFileAccesses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ReportedFileAccess.Deserialize(r2, allReportedProcesses, readPath: readPath)));
IReadOnlyList<ReportedFileAccess> allUnexpectedFileAccesses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ReportedFileAccess.Deserialize(r2, allReportedProcesses, readPath: readPath)));
IReadOnlyList<ReportedProcess> processes = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => allReportedProcesses[r2.ReadInt32()]));
IReadOnlyList<ProcessDetouringStatusData> detouringStatuses = reader.ReadNullable(r => r.ReadReadOnlyList(r2 => ProcessDetouringStatusData.Deserialize(r2)));
string dumpFileDirectory = reader.ReadNullableString();
Expand Down
40 changes: 33 additions & 7 deletions Public/Src/Engine/Processes/SandboxedProcessUnix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,17 @@ private class PerfAggregator
/// </summary>
public string ExecutableAbsolutePath => Process.StartInfo.FileName;

/// <summary>
/// Gets file path for standard input.
/// </summary>
internal static string GetStdInFilePath(string workingDirectory, long pipSemiStableHash) =>
Path.Combine(workingDirectory, BuildXL.Pips.Operations.Pip.FormatSemiStableHash(pipSemiStableHash) + ".stdin");

/// <summary>
/// Shell executable that wraps the process to be executed.
/// </summary>
internal const string ShellExecutable = "/bin/sh";

/// <nodoc />
public SandboxedProcessUnix(SandboxedProcessInfo info, bool ignoreReportedAccesses = false, bool? overrideMeasureTime = null)
: base(info)
Expand Down Expand Up @@ -195,7 +206,7 @@ protected override System.Diagnostics.Process CreateProcess(SandboxedProcessInfo
{
var process = base.CreateProcess(info);

process.StartInfo.FileName = "/bin/sh";
process.StartInfo.FileName = ShellExecutable;
process.StartInfo.Arguments = string.Empty;
process.StartInfo.RedirectStandardInput = true;

Expand All @@ -220,10 +231,16 @@ private async Task OnProcessStartedAsync(SandboxedProcessInfo info)
ReportProcessCreated();

// Allow read access for /bin/sh
info.FileAccessManifest.AddPath(
AbsolutePath.Create(PathTable, Process.StartInfo.FileName),
mask: FileAccessPolicy.MaskNothing,
values: FileAccessPolicy.AllowReadAlways);
// When executed using external tool, the manifest tree has been sealed, and cannot be modified.
// We take care of adding this path in the manifest in SandboxedProcessPipExecutor.cs;
// see AddUnixSpecificSandcboxedProcessFileAccessPolicies
if (!info.FileAccessManifest.IsManifestTreeBlockSealed)
{
info.FileAccessManifest.AddPath(
AbsolutePath.Create(PathTable, Process.StartInfo.FileName),
mask: FileAccessPolicy.MaskNothing,
values: FileAccessPolicy.AllowReadAlways);
}

if (MeasureCpuTime)
{
Expand Down Expand Up @@ -709,7 +726,7 @@ private async Task<string> FlushStandardInputToFileIfNeededAsync(SandboxedProces
return null;
}

string stdinFileName = Path.Combine(Process.StartInfo.WorkingDirectory, info.PipSemiStableHash + ".stdin");
string stdinFileName = GetStdInFilePath(Process.StartInfo.WorkingDirectory, info.PipSemiStableHash);
string stdinText = await info.StandardInputReader.ReadToEndAsync();
Encoding encoding = info.StandardInputEncoding ?? Console.InputEncoding;
byte[] stdinBytes = encoding.GetBytes(stdinText);
Expand All @@ -720,7 +737,16 @@ private async Task<string> FlushStandardInputToFileIfNeededAsync(SandboxedProces
}

// Allow read from the created stdin file
info.FileAccessManifest.AddPath(AbsolutePath.Create(PathTable, stdinFileName), mask: FileAccessPolicy.MaskNothing, values: FileAccessPolicy.AllowAll);
// When executed using external tool, the manifest tree has been sealed, and cannot be modified.
// We take care of adding this path in the manifest in SandboxedProcessPipExecutor.cs;
// see AddUnixSpecificSandcboxedProcessFileAccessPolicies
if (!info.FileAccessManifest.IsManifestTreeBlockSealed)
{
info.FileAccessManifest.AddPath(
AbsolutePath.Create(PathTable, stdinFileName),
mask: FileAccessPolicy.MaskNothing,
values: FileAccessPolicy.AllowAll);
}

return stdinFileName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

namespace ExternalToolTest.BuildXL.Scheduler
{
[TestClassIfSupported(requiresWindowsBasedOperatingSystem: true)]
public class ExternalToolExecutionTests : SchedulerIntegrationTestBase
{
public ExternalToolExecutionTests(ITestOutputHelper output) : base(output)
Expand Down Expand Up @@ -232,6 +231,12 @@ public void ExecutionRespectTimeout()
RunScheduler().AssertFailure();
AssertErrorEventLogged(ProcessesLogEventId.PipProcessTookTooLongError, count: 1);
AssertErrorEventLogged(ProcessesLogEventId.PipProcessError, count: 1);

if (OperatingSystemHelper.IsUnixOS)
{
// Creating dump is not supported on non-Windows.
AssertWarningEventLogged(ProcessesLogEventId.PipFailedToCreateDumpFile, count: 1);
}
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.IO;
using BuildXL.Pips.Builders;
using BuildXL.Pips.Operations;
using BuildXL.Scheduler;
using BuildXL.Utilities;
using Test.BuildXL.Executables.TestProcess;
using Test.BuildXL.TestUtilities.Xunit;
Expand Down
Loading

0 comments on commit 0f59d16

Please sign in to comment.