diff --git a/build/buildpipeline/windows.groovy b/build/buildpipeline/windows.groovy index 8d26f313d4..9c0b86f089 100644 --- a/build/buildpipeline/windows.groovy +++ b/build/buildpipeline/windows.groovy @@ -2,7 +2,7 @@ // 'node' indicates to Jenkins that the enclosed block runs on a node that matches // the label 'windows-with-vs' -simpleNode('Windows_NT','latest') { +simpleNode('Windows.10.Enterprise.RS3.ASPNET') { stage ('Checking out source') { checkout scm } diff --git a/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs b/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs index dc7953565a..d0ec48f2b2 100644 --- a/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs +++ b/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs @@ -124,14 +124,16 @@ namespace Microsoft.AspNetCore.Razor.Tasks string commandLineCommands, out int result) { +#if !NET46 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; + return ForceServer; } +#endif 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 f371ebe487..9378906d45 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/Client.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/Client.cs @@ -4,6 +4,10 @@ using System; using System.IO; using System.IO.Pipes; +#if NET46 +using System.Security.AccessControl; +using System.Security.Principal; +#endif using System.Threading; using System.Threading.Tasks; @@ -67,9 +71,14 @@ namespace Microsoft.AspNetCore.Razor.Tools ServerLogger.Log("Named pipe '{0}' connected", pipeName); cancellationToken.ThrowIfCancellationRequested(); - // The original code in Roslyn checks that the server pipe is owned by the same user for security. - // We plan to rely on the BCL for this but it's not yet implemented: - // See https://github.com/dotnet/corefx/issues/25427 +#if NET46 + // Verify that we own the pipe. + if (!CheckPipeConnectionOwnership(stream)) + { + ServerLogger.Log("Owner of named pipe is incorrect"); + return null; + } +#endif return new NamedPipeClient(stream, GetNextIdentifier()); } @@ -80,6 +89,44 @@ namespace Microsoft.AspNetCore.Razor.Tools } } +#if NET46 + /// + /// Check to ensure that the named pipe server we connected to is owned by the same + /// user. + /// + private static bool CheckPipeConnectionOwnership(NamedPipeClientStream pipeStream) + { + try + { + if (PlatformInformation.IsWindows) + { + using (var currentIdentity = WindowsIdentity.GetCurrent()) + { + var currentOwner = currentIdentity.Owner; + var remotePipeSecurity = GetPipeSecurity(pipeStream); + var remoteOwner = remotePipeSecurity.GetOwner(typeof(SecurityIdentifier)); + + return currentOwner.Equals(remoteOwner); + } + } + + // We don't need to verify on non-windows as that will be taken care of by the + // PipeOptions.CurrentUserOnly flag. + return false; + } + catch (Exception ex) + { + ServerLogger.LogException(ex, "Checking pipe connection"); + return false; + } + } + + private static ObjectSecurity GetPipeSecurity(PipeStream pipeStream) + { + return pipeStream.GetAccessControl(); + } +#endif + private static PipeOptions GetPipeOptions() { var options = PipeOptions.Asynchronous; diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildIntegrationTest.cs index 9ac5e9ab13..541cac3523 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildIntegrationTest.cs @@ -25,9 +25,8 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests public Task Build_SimpleMvc_UsingDotnetMSBuildAndWithoutBuildServer_CanBuildSuccessfully() => Build_SimpleMvc_WithoutBuildServer_CanBuildSuccessfully(MSBuildProcessKind.Dotnet); - [ConditionalFact(Skip = "https://github.com/aspnet/Razor/issues/2208")] - [OSSkipCondition(OperatingSystems.Linux)] - [OSSkipCondition(OperatingSystems.MacOSX)] + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX)] [InitializeTestProject("SimpleMvc")] public Task Build_SimpleMvc_UsingDesktopMSBuildAndWithoutBuildServer_CanBuildSuccessfully() => Build_SimpleMvc_WithoutBuildServer_CanBuildSuccessfully(MSBuildProcessKind.Desktop); diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs index 478ee4fee6..60076a7a67 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests public Task Build_SimpleMvc_WithServer_UsingDotnetMSBuild_CanBuildSuccessfully() => Build_SimpleMvc_CanBuildSuccessfully(MSBuildProcessKind.Dotnet); - [ConditionalFact(Skip = "https://github.com/aspnet/Razor/issues/2208")] + [ConditionalFact] [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX)] [InitializeTestProject("SimpleMvc")] public Task Build_SimpleMvc_WithServer_UsingDesktopMSBuild_CanBuildSuccessfully()