diff --git a/build/dependencies.props b/build/dependencies.props index fe712915df..49aa91d1bf 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -19,7 +19,7 @@ 2.1.0-preview2-30478 2.1.0-preview2-30478 2.0.0 - 2.1.0-preview2-26326-03 + 2.1.0-preview2-26403-06 15.6.1 15.0.26606 15.6.161-preview diff --git a/src/Microsoft.AspNetCore.Razor.Design/build/netstandard2.0/Microsoft.AspNetCore.Razor.Design.CodeGeneration.targets b/src/Microsoft.AspNetCore.Razor.Design/build/netstandard2.0/Microsoft.AspNetCore.Razor.Design.CodeGeneration.targets index 1eb7d2baad..c390749c45 100644 --- a/src/Microsoft.AspNetCore.Razor.Design/build/netstandard2.0/Microsoft.AspNetCore.Razor.Design.CodeGeneration.targets +++ b/src/Microsoft.AspNetCore.Razor.Design/build/netstandard2.0/Microsoft.AspNetCore.Razor.Design.CodeGeneration.targets @@ -72,6 +72,7 @@ ToolAssembly="$(_RazorToolAssembly)" UseServer="$(UseRazorBuildServer)" ForceServer="$(_RazorForceBuildServer)" + SuppressCurrentUserOnlyPipeOptions="$(_RazorSuppressCurrentUserOnlyPipeOptions)" PipeName="$(_RazorBuildServerPipeName)" Version="$(RazorLangVersion)" Configuration="@(ResolvedRazorConfiguration)" @@ -123,6 +124,7 @@ ToolAssembly="$(_RazorToolAssembly)" UseServer="$(UseRazorBuildServer)" ForceServer="$(_RazorForceBuildServer)" + SuppressCurrentUserOnlyPipeOptions="$(_RazorSuppressCurrentUserOnlyPipeOptions)" PipeName="$(_RazorBuildServerPipeName)" Version="$(RazorLangVersion)" Configuration="@(ResolvedRazorConfiguration)" diff --git a/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs b/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs index a4ee3ca53e..95d5f93efd 100644 --- a/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs +++ b/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.IO.Pipes; using System.Threading; using Microsoft.AspNetCore.Razor.Tools; using Microsoft.Build.Framework; @@ -15,6 +16,10 @@ namespace Microsoft.AspNetCore.Razor.Tasks { public abstract class DotNetToolTask : ToolTask { + // From https://github.com/dotnet/corefx/blob/29cd6a0b0ac2993cee23ebaf36ca3d4bce6dd75f/src/System.IO.Pipes/ref/System.IO.Pipes.cs#L93. + // Using the enum value directly as this option is not available in netstandard. + private const PipeOptions PipeOptionCurrentUserOnly = (PipeOptions)536870912; + private CancellationTokenSource _razorServerCts; public bool Debug { get; set; } @@ -29,6 +34,10 @@ namespace Microsoft.AspNetCore.Razor.Tasks // Specifies whether we should fallback to in-process execution if server execution fails. public bool ForceServer { get; set; } + // Specifies whether server execution is allowed when PipeOptions.CurrentUserOnly is not available. + // For testing purposes only. + public bool SuppressCurrentUserOnlyPipeOptions { get; set; } + public string PipeName { get; set; } protected override string ToolName => "dotnet"; @@ -115,6 +124,15 @@ namespace Microsoft.AspNetCore.Razor.Tasks string commandLineCommands, out int result) { + if (!SuppressCurrentUserOnlyPipeOptions && !Enum.IsDefined(typeof(PipeOptions), PipeOptionCurrentUserOnly)) + { + // For security reasons, we don't want to spin up a server that doesn't + // restrict requests only to the current user. + result = -1; + + return false; + } + Log.LogMessage(StandardOutputLoggingImportance, "Server execution started."); using (_razorServerCts = new CancellationTokenSource()) { diff --git a/src/Microsoft.AspNetCore.Razor.Tools/Client.cs b/src/Microsoft.AspNetCore.Razor.Tools/Client.cs index 4f7903dec5..f371ebe487 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/Client.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/Client.cs @@ -13,6 +13,12 @@ namespace Microsoft.AspNetCore.Razor.Tools { private static int counter; + // From https://github.com/dotnet/corefx/blob/29cd6a0b0ac2993cee23ebaf36ca3d4bce6dd75f/src/System.IO.Pipes/ref/System.IO.Pipes.cs#L93. + // Using the enum value directly as this option is not available in netstandard. + private const PipeOptions PipeOptionCurrentUserOnly = (PipeOptions)536870912; + + private static readonly PipeOptions _pipeOptions = GetPipeOptions(); + public abstract Stream Stream { get; } public abstract string Identifier { get; } @@ -40,7 +46,7 @@ namespace Microsoft.AspNetCore.Razor.Tools // The NamedPipeClientStream class handles the "\\.\pipe\" part for us. ServerLogger.Log("Attempt to open named pipe '{0}'", pipeName); - var stream = new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous); + var stream = new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, _pipeOptions); cancellationToken.ThrowIfCancellationRequested(); ServerLogger.Log("Attempt to connect named pipe '{0}'", pipeName); @@ -74,6 +80,18 @@ namespace Microsoft.AspNetCore.Razor.Tools } } + private static PipeOptions GetPipeOptions() + { + var options = PipeOptions.Asynchronous; + + if (Enum.IsDefined(typeof(PipeOptions), PipeOptionCurrentUserOnly)) + { + return options | PipeOptionCurrentUserOnly; + } + + return options; + } + private static string GetNextIdentifier() { var id = Interlocked.Increment(ref counter); diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ConnectionHost.cs b/src/Microsoft.AspNetCore.Razor.Tools/ConnectionHost.cs index 6bda4338a8..dfa661f77d 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ConnectionHost.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ConnectionHost.cs @@ -13,9 +13,6 @@ namespace Microsoft.AspNetCore.Razor.Tools // https://github.com/dotnet/roslyn/blob/14aed138a01c448143b9acf0fe77a662e3dfe2f4/src/Compilers/Server/VBCSCompiler/NamedPipeClientConnection.cs#L17 internal abstract class ConnectionHost { - // Size of the buffers to use: 64K - private const int PipeBufferSize = 0x10000; - private static int counter; public abstract Task WaitForConnectionAsync(CancellationToken cancellationToken); @@ -33,6 +30,15 @@ namespace Microsoft.AspNetCore.Razor.Tools private class NamedPipeConnectionHost : ConnectionHost { + // Size of the buffers to use: 64K + private const int PipeBufferSize = 0x10000; + + // From https://github.com/dotnet/corefx/blob/29cd6a0b0ac2993cee23ebaf36ca3d4bce6dd75f/src/System.IO.Pipes/ref/System.IO.Pipes.cs#L93. + // Using the enum value directly as this option is not available in netstandard. + private const PipeOptions PipeOptionCurrentUserOnly = (PipeOptions)536870912; + + private static readonly PipeOptions _pipeOptions = GetPipeOptions(); + public NamedPipeConnectionHost(string pipeName) { PipeName = pipeName; @@ -45,15 +51,12 @@ namespace Microsoft.AspNetCore.Razor.Tools // Create the pipe and begin waiting for a connection. This doesn't block, but could fail // in certain circumstances, such as the OS refusing to create the pipe for some reason // or the pipe was disconnected before we starting listening. - // - // Also, note that we're waiting on CoreFx to implement some security features for us. - // https://github.com/dotnet/corefx/issues/24040 var pipeStream = new NamedPipeServerStream( PipeName, PipeDirection.InOut, NamedPipeServerStream.MaxAllowedServerInstances, // Maximum connections. PipeTransmissionMode.Byte, - PipeOptions.Asynchronous | PipeOptions.WriteThrough, + _pipeOptions, PipeBufferSize, // Default input buffer PipeBufferSize);// Default output buffer @@ -70,6 +73,18 @@ namespace Microsoft.AspNetCore.Razor.Tools pipeStream.Close(); throw new Exception("Insufficient resources to process new connection."); } + + private static PipeOptions GetPipeOptions() + { + var options = PipeOptions.Asynchronous | PipeOptions.WriteThrough; + + if (Enum.IsDefined(typeof(PipeOptions), PipeOptionCurrentUserOnly)) + { + return options | PipeOptionCurrentUserOnly; + } + + return options; + } } private class NamedPipeConnection : Connection diff --git a/src/Microsoft.AspNetCore.Razor.Tools/PipeName.cs b/src/Microsoft.AspNetCore.Razor.Tools/PipeName.cs index b35275816d..23fc72f90c 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/PipeName.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/PipeName.cs @@ -3,7 +3,6 @@ using System; using System.IO; -using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Text; @@ -13,14 +12,10 @@ namespace Microsoft.AspNetCore.Razor.Tools { // We want each pipe to unique and predictable based on the inputs of: // - user (security) - // - elevation status (security) // - path of tool on disk (version) // // This allows us to meet the security and version compat requirements just by selecting a pipe name. // - // https://github.com/dotnet/corefx/issues/25427 will actually enforce the security, but we still - // want these guarantees when we try to connect so we can expect it to succeed. - // // This is similar to (and based on) the code used by Roslyn in VBCSCompiler: // https://github.com/dotnet/roslyn/blob/c273b6a9f19570a344c274ae89185b3a2b64d93d/src/Compilers/Shared/BuildServerConnection.cs#L528 public static string ComputeDefault(string toolDirectory = null) @@ -36,24 +31,14 @@ namespace Microsoft.AspNetCore.Razor.Tools // That would be a pretty wacky bug to try and unravel. var baseName = ComputeBaseName("Razor:" + toolDirectory); - // Prefix with username and elevation - var isAdmin = false; - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { -#if WINDOWS_HACK_LOL - var currentIdentity = WindowsIdentity.GetCurrent(); - var principal = new WindowsPrincipal(currentIdentity); - isAdmin = principal.IsInRole(WindowsBuiltInRole.Administrator); -#endif - } - + // Prefix with username var userName = Environment.UserName; if (userName == null) { return null; } - return $"{userName}.{(isAdmin ? 'T' : 'F')}.{baseName}"; + return $"{userName}.{baseName}"; } private static string ComputeBaseName(string baseDirectory) diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs index a0abcb93df..aaedd15cec 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs @@ -72,6 +72,11 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests if (!suppressBuildServer) { buildArgumentList.Add($"/p:_RazorBuildServerPipeName={buildServerPipeName ?? BuildServer.PipeName}"); + + // The build server will not be used in netcoreapp2.0 because PipeOptions.CurrentUserOnly is not available. + // But we still want to make sure to run the tests on the server. So suppress that check. + // This can be removed once https://github.com/aspnet/Razor/issues/2237 is done. + buildArgumentList.Add($"/p:_RazorSuppressCurrentUserOnlyPipeOptions=true"); } buildArgumentList.Add($"/t:{target} /p:Configuration={Configuration} {args}");