Sign reference assemblies in legacy

The design of reference assemblies, no matter whether they are generated
using `/ref` or `/refout` is that they are signed if strong signing
would be done for an implementation assembly.

This was implemented correctly for builder based signing but not for COM
based. This PR fixes the signing to be consistent.

src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs

@@ -41,6 +41,9 @@ internal override Stream CreateInputStream()
                 throw ThrownException;
             }
 
+            internal override void SignFile(StrongNameKeys keys, string filePath) =>
+                _underlyingProvider.SignFile(keys, filePath);
+
             internal override StrongNameKeys CreateKeys(string keyFilePath, string keyContainerName, bool hasSignatureKey, CommonMessageProvider messageProvider) =>
                 _underlyingProvider.CreateKeys(keyFilePath, keyContainerName, hasSignatureKey, messageProvider);
 
@@ -140,26 +143,6 @@ public void ExceptionInReadKeysFromContainer()
                 // error CS7028: Error signing output with public key from container 'RoslynTestContainer' -- Crazy exception you could never have predicted!
                 Diagnostic(ErrorCode.ERR_PublicKeyContainerFailure).WithArguments("RoslynTestContainer", ex.Message).WithLocation(1, 1));
         }
-
-        [ConditionalFact(typeof(WindowsDesktopOnly), Reason = "https://github.com/dotnet/roslyn/issues/30152")]
-        public void BadInputStream()
-        {
-            string src = @"
-class C
-{
-    public static void Main(string[] args) { }
-}";
-            var testProvider = new StrongNameProviderWithBadInputStream(DefaultDesktopStrongNameProvider);
-            var options = TestOptions.DebugExe
-                .WithStrongNameProvider(testProvider)
-                .WithCryptoKeyContainer("RoslynTestContainer");
-
-            var comp = CreateCompilation(src, options: options);
-
-            comp.Emit(new MemoryStream()).Diagnostics.Verify(
-                // error CS8104: An error occurred while writing the Portable Executable file.
-                Diagnostic(ErrorCode.ERR_PeWritingFailure).WithArguments(testProvider.ThrownException.ToString()).WithLocation(1, 1));
-        }
     }
 }
 #endif

src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs

@@ -124,10 +124,10 @@ public void PubKeyFromKeyFileAttribute_AssemblyKeyFileResolver(CSharpParseOption
             string keyFileName = Path.GetFileName(s_keyPairFile);
 
             string s = string.Format("{0}{1}{2}", @"[assembly: System.Reflection.AssemblyKeyFile(@""", keyFileName, @""")] public class C {}");
-            var syntaxTree = Parse(s, @"IVTAndStrongNameTests\AnotherTempDir\temp.cs");
+            var syntaxTree = Parse(s, @"IVTAndStrongNameTests\AnotherTempDir\temp.cs", parseOptions);
 
             // verify failure with default assembly key file resolver
-            var comp = CreateCompilation(syntaxTree, options: TestOptions.ReleaseDll, parseOptions: parseOptions);
+            var comp = CreateCompilation(syntaxTree, options: TestOptions.ReleaseDll);
             comp.VerifyDiagnostics(
                 Diagnostic(ErrorCode.ERR_PublicKeyFileFailure).WithArguments(keyFileName, "Assembly signing not supported."));
 
@@ -153,10 +153,10 @@ public void PubKeyFromKeyFileAttribute_AssemblyKeyFileResolver_RelativeToCurrent
             string keyFileName = Path.GetFileName(s_keyPairFile);
 
             string s = String.Format("{0}{1}{2}", @"[assembly: System.Reflection.AssemblyKeyFile(@""..\", keyFileName, @""")] public class C {}");
-            var syntaxTree = Parse(s, @"IVTAndStrongNameTests\AnotherTempDir\temp.cs");
+            var syntaxTree = Parse(s, @"IVTAndStrongNameTests\AnotherTempDir\temp.cs", parseOptions);
 
             // verify failure with default assembly key file resolver
-            var comp = CreateCompilation(syntaxTree, options: TestOptions.ReleaseDll, parseOptions: parseOptions);
+            var comp = CreateCompilation(syntaxTree, options: TestOptions.ReleaseDll);
             comp.VerifyDiagnostics(
                 // error CS7027: Error extracting public key from file '..\KeyPairFile.snk' -- File not found.
                 Diagnostic(ErrorCode.ERR_PublicKeyFileFailure).WithArguments(@"..\" + keyFileName, "Assembly signing not supported."));
@@ -174,9 +174,8 @@ public void PubKeyFromKeyFileAttribute_AssemblyKeyFileResolver_RelativeToCurrent
             Assert.True(ByteSequenceComparer.Equals(s_publicKey, comp.Assembly.Identity.PublicKey));
         }
 
-        [Theory]
-        [MemberData(nameof(AllProviderParseOptions))]
-        public void SigningNotAvailable001(CSharpParseOptions parseOptions)
+        [Fact]
+        public void SigningNotAvailable001()
         {
             string keyFileDir = Path.GetDirectoryName(s_keyPairFile);
             string keyFileName = Path.GetFileName(s_keyPairFile);
@@ -191,9 +190,7 @@ public void SigningNotAvailable001(CSharpParseOptions parseOptions)
             var comp = CreateCompilation(
                 assemblyName: GetUniqueName(),
                 source: new[] { syntaxTree },
-                references: new[] { MscorlibRef },
-                options: options,
-                parseOptions: parseOptions);
+                options: options);
 
             var provider = (DesktopStrongNameProvider)comp.Options.StrongNameProvider;
 
@@ -1558,7 +1555,7 @@ public void SignModuleKeyFileCmdLine(CSharpParseOptions parseOptions)
             string s = "public class C {}";
 
             var options = TestOptions.SigningReleaseModule.WithCryptoKeyFile(s_keyPairFile);
-            var other = CreateCompilation(s, options: options);
+            var other = CreateCompilation(s, options: options, parseOptions: parseOptions);
 
             var outStrm = new MemoryStream();
             var success = other.Emit(outStrm);
@@ -1618,7 +1615,7 @@ public void SignModuleKeyFileCmdLine_1(CSharpParseOptions parseOptions)
             string s = String.Format("{0}{1}{2}", @"[assembly: System.Reflection.AssemblyKeyFile(@""", x, @""")] public class C {}");
 
             var options = TestOptions.SigningReleaseModule.WithCryptoKeyFile(s_keyPairFile);
-            var other = CreateCompilation(s, options: options);
+            var other = CreateCompilation(s, options: options, parseOptions: parseOptions);
 
             var outStrm = new MemoryStream();
             var success = other.Emit(outStrm);

src/Compilers/Core/Portable/Compilation.EmitStream.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.Diagnostics;
+using System.IO;
+using Roslyn.Utilities;
+
+namespace Microsoft.CodeAnalysis
+{
+    public abstract partial class Compilation
+    {
+        /// <summary>
+        /// Describes the kind of real signing that is being done during Emit. In the case of public signing
+        /// this value will be <see cref="None"/>.
+        /// </summary>
+        internal enum EmitStreamSignKind
+        {
+            None,
+            SignedWithBulider,
+            SignedWithFile,
+        }
+
+        internal sealed class EmitStream
+        {
+            private readonly EmitStreamProvider _emitStreamProvider;
+            private readonly EmitStreamSignKind _emitStreamSignKind;
+            private readonly StrongNameProvider _strongNameProvider;
+            private (Stream tempStream, string tempFilePath)? _tempInfo;
+
+            /// <summary>
+            /// The <see cref="Stream"/> that is being emitted into. This value should _never_ be 
+            /// Dispose. It is either returned from the <see cref="EmitStreamProvider"/> instance in
+            /// which case it is owned by that. Or it is just an alias for the value that is stored 
+            /// in <see cref="_tempInfo"/> in which case it will be disposed from there.
+            /// </summary>
+            private Stream _stream;
+
+            internal EmitStream(
+                EmitStreamProvider emitStreamProvider,
+                EmitStreamSignKind emitStreamSignKind,
+                StrongNameProvider strongNameProvider)
+            {
+                Debug.Assert(emitStreamProvider != null);
+                Debug.Assert(strongNameProvider != null || emitStreamSignKind == EmitStreamSignKind.None);
+                _emitStreamProvider = emitStreamProvider;
+                _emitStreamSignKind = emitStreamSignKind;
+                _strongNameProvider = strongNameProvider;
+            }
+
+            internal Func<Stream> GetCreateStreamFunc(DiagnosticBag diagnostics)
+            {
+                return () => CreateStream(diagnostics);
+            }
+
+            internal void Dispose()
+            {
+                _tempInfo?.tempStream.Dispose();
+                _tempInfo = null;
+
+                // The _stream value is deliberately excluded from being disposed here. That value is not 
+                // owned by this type.
+                _stream = null;
+            }
+
+            /// <summary>
+            /// Create the stream which should be used for Emit. This should only be called one time.
+            /// </summary>
+            private Stream CreateStream(DiagnosticBag diagnostics)
+            {
+                Debug.Assert(_stream == null);
+                Debug.Assert(diagnostics != null);
+
+                if (diagnostics.HasAnyErrors())
+                {
+                    return null;
+                }
+
+                _stream = _emitStreamProvider.GetOrCreateStream(diagnostics);
+                if (_stream == null)
+                {
+                    Debug.Assert(diagnostics.HasAnyErrors());
+                    return null;
+                }
+
+                // If the current strong name provider is the Desktop version, signing can only be done to on-disk files.
+                // If this binary is configured to be signed, create a temp file, output to that
+                // then stream that to the stream that this method was called with. Otherwise output to the
+                // stream that this method was called with.
+                if (_emitStreamSignKind == EmitStreamSignKind.SignedWithFile)
+                {
+                    Debug.Assert(_strongNameProvider != null);
+
+                    try
+                    {
+                        Func<string, Stream> streamConstructor = path => new FileStream(path, FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite);
+
+                        var tempDir = _strongNameProvider.FileSystem.GetTempPath();
+                        var tempFilePath = Path.Combine(tempDir, Guid.NewGuid().ToString("N"));
+                        var tempStream = FileUtilities.CreateFileStreamChecked(streamConstructor, tempFilePath);
+
+                        _tempInfo = (tempStream, tempFilePath);
+                        return tempStream;
+                    }
+                    catch (IOException e)
+                    {
+                        throw new Cci.PeWritingException(e);
+                    }
+                }
+                else
+                {
+                    return _stream;
+                }
+            }
+
+            internal bool Complete(StrongNameKeys strongNameKeys, CommonMessageProvider messageProvider, DiagnosticBag diagnostics)
+            {
+                Debug.Assert(_stream != null);
+
+                if (_tempInfo.HasValue)
+                {
+                    Debug.Assert(_emitStreamSignKind == EmitStreamSignKind.SignedWithFile);
+                    var (tempStream, tempFilePath) = _tempInfo.Value;
+
+                    try
+                    {
+                        // Dispose the temp stream to ensure all of the contents are written to 
+                        // disk.
+                        tempStream.Dispose();
+
+                        _strongNameProvider.SignFile(strongNameKeys, tempFilePath);
+
+                        using (var tempFileStream = new FileStream(tempFilePath, FileMode.Open))
+                        {
+                            tempFileStream.CopyTo(_stream);
+                        }
+                    }
+                    catch (DesktopStrongNameProvider.ClrStrongNameMissingException)
+                    {
+                        diagnostics.Add(StrongNameKeys.GetError(strongNameKeys.KeyFilePath, strongNameKeys.KeyContainer,
+                            new CodeAnalysisResourcesLocalizableErrorArgument(nameof(CodeAnalysisResources.AssemblySigningNotSupported)), messageProvider));
+                        return false;
+                    }
+                    catch (IOException ex)
+                    {
+                        diagnostics.Add(StrongNameKeys.GetError(strongNameKeys.KeyFilePath, strongNameKeys.KeyContainer, ex.Message, messageProvider));
+                        return false;
+                    }
+                    finally
+                    {
+                        try
+                        {
+                            File.Delete(tempFilePath);
+                        }
+                        catch
+                        {
+                            // Not much to do if we can't delete from the temp directory
+                        }
+
+                        tempStream.Dispose();
+                        _tempInfo = null;
+                        _stream = null;
+                    }
+                }
+
+                _stream = null;
+                return true;
+            }
+        }
+    }
+}

src/Compilers/Core/Portable/Compilation/Compilation.cs

@@ -2624,10 +2624,8 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
             cancellationToken.ThrowIfCancellationRequested();
 
             Cci.PdbWriter nativePdbWriter = null;
-            Stream signingInputStream = null;
             DiagnosticBag metadataDiagnostics = null;
             DiagnosticBag pdbBag = null;
-            Stream peStream = null;
 
             bool deterministic = IsEmitDeterministic;
 
@@ -2648,8 +2646,17 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
                 pePdbFilePath = PathUtilities.GetFileName(pePdbFilePath);
             }
 
+            EmitStream emitPeStream = null;
+            EmitStream emitMetadataStream = null;
             try
             {
+                var signKind = IsRealSigned
+                    ? (SignUsingBuilder ? EmitStreamSignKind.SignedWithBulider : EmitStreamSignKind.SignedWithFile)
+                    : EmitStreamSignKind.None;
+                emitPeStream = new EmitStream(peStreamProvider, signKind, Options.StrongNameProvider);
+                emitMetadataStream = metadataPEStreamProvider == null
+                    ? null
+                    : new EmitStream(metadataPEStreamProvider, signKind, Options.StrongNameProvider);
                 metadataDiagnostics = DiagnosticBag.GetInstance();
 
                 if (moduleBeingBuilt.DebugInformationFormat == DebugInformationFormat.Pdb && pdbStreamProvider != null)
@@ -2663,18 +2670,6 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
                     nativePdbWriter = new Cci.PdbWriter(pePdbFilePath, testSymWriterFactory, deterministic ? moduleBeingBuilt.PdbChecksumAlgorithm : default);
                 }
 
-                Func<Stream> getPeStream = () =>
-                {
-                    Stream ret;
-                    (peStream, signingInputStream, ret) = GetPeStream(metadataDiagnostics, peStreamProvider, metadataOnly);
-                    return ret;
-                };
-
-                Func<Stream> getRefPeStream =
-                    metadataPEStreamProvider == null
-                    ? null
-                    : (Func<Stream>)(() => ConditionalGetOrCreateStream(metadataPEStreamProvider, metadataDiagnostics));
-
                 Func<Stream> getPortablePdbStream =
                     moduleBeingBuilt.DebugInformationFormat != DebugInformationFormat.PortablePdb || pdbStreamProvider == null
                     ? null
@@ -2686,8 +2681,8 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
                         moduleBeingBuilt,
                         metadataDiagnostics,
                         MessageProvider,
-                        getPeStream,
-                        getRefPeStream,
+                        emitPeStream.GetCreateStreamFunc(metadataDiagnostics),
+                        emitMetadataStream?.GetCreateStreamFunc(metadataDiagnostics),
                         getPortablePdbStream,
                         nativePdbWriter,
                         pePdbFilePath,
@@ -2737,31 +2732,17 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
                     return false;
                 }
 
-                if (signingInputStream != null && peStream != null)
+                if (!emitPeStream.Complete(StrongNameKeys, MessageProvider, diagnostics) ||
+                    emitMetadataStream?.Complete(StrongNameKeys, MessageProvider, diagnostics) == false)
                 {
-                    Debug.Assert(Options.StrongNameProvider != null);
-
-                    try
-                    {
-                        Options.StrongNameProvider.SignStream(StrongNameKeys, signingInputStream, peStream);
-                    }
-                    catch (DesktopStrongNameProvider.ClrStrongNameMissingException)
-                    {
-                        diagnostics.Add(StrongNameKeys.GetError(StrongNameKeys.KeyFilePath, StrongNameKeys.KeyContainer,
-                            new CodeAnalysisResourcesLocalizableErrorArgument(nameof(CodeAnalysisResources.AssemblySigningNotSupported)), MessageProvider));
-                        return false;
-                    }
-                    catch (IOException ex)
-                    {
-                        diagnostics.Add(StrongNameKeys.GetError(StrongNameKeys.KeyFilePath, StrongNameKeys.KeyContainer, ex.Message, MessageProvider));
-                        return false;
-                    }
+                    return false;
                 }
             }
             finally
             {
                 nativePdbWriter?.Dispose();
-                signingInputStream?.Dispose();
+                emitPeStream?.Dispose();
+                emitMetadataStream?.Dispose();
                 pdbBag?.Free();
                 metadataDiagnostics?.Free();
             }
@@ -2781,58 +2762,6 @@ private static Stream ConditionalGetOrCreateStream(EmitStreamProvider metadataPE
             return auxStream;
         }
 
-        /// <summary>
-        /// Returns a tuple of streams where
-        /// * <c>peStream</c> is a stream which will carry the output PE bits
-        /// * <c>signingStream</c> is the stream which will be signed by the legacy strong name signer, or null if we aren't using the legacy signer
-        /// * <c>selectedStream</c> is an alias of either peStream or signingStream, and is the stream that will be written to by the emitter.
-        /// </summary>
-        private (Stream peStream, Stream signingStream, Stream selectedStream) GetPeStream(DiagnosticBag metadataDiagnostics, EmitStreamProvider peStreamProvider, bool metadataOnly)
-        {
-            Stream peStream = null;
-            Stream signingStream = null;
-            Stream selectedStream = null;
-
-            if (metadataDiagnostics.HasAnyErrors())
-            {
-                return (peStream, signingStream, selectedStream);
-            }
-
-            peStream = peStreamProvider.GetOrCreateStream(metadataDiagnostics);
-            if (peStream == null)
-            {
-                Debug.Assert(metadataDiagnostics.HasAnyErrors());
-                return (peStream, signingStream, selectedStream);
-            }
-
-            // If the current strong name provider is the Desktop version, signing can only be done to on-disk files.
-            // If this binary is configured to be signed, create a temp file, output to that
-            // then stream that to the stream that this method was called with. Otherwise output to the
-            // stream that this method was called with.
-            if (!metadataOnly && IsRealSigned && !SignUsingBuilder)
-            {
-                Debug.Assert(Options.StrongNameProvider != null);
-
-                try
-                {
-                    signingStream = Options.StrongNameProvider.CreateInputStream();
-                }
-                catch (IOException e)
-                {
-                    throw new Cci.PeWritingException(e);
-                }
-
-                selectedStream = signingStream;
-            }
-            else
-            {
-                signingStream = null;
-                selectedStream = peStream;
-            }
-
-            return (peStream, signingStream, selectedStream);
-        }
-
         internal static bool SerializePeToStream(
             CommonPEModuleBuilder moduleBeingBuilt,
             DiagnosticBag metadataDiagnostics,

src/Compilers/Core/Portable/StrongName/DesktopStrongNameProvider.cs

@@ -19,6 +19,7 @@ namespace Microsoft.CodeAnalysis
     /// </summary>
     public class DesktopStrongNameProvider : StrongNameProvider
     {
+        // TODO: delete this
         internal sealed class TempFileStream : Stream
         {
             private readonly string _path;
@@ -259,22 +260,26 @@ internal virtual void ReadKeysFromContainer(string keyContainer, out ImmutableAr
             }
         }
 
-        internal override void SignStream(StrongNameKeys keys, Stream inputStream, Stream outputStream)
+        internal override void SignFile(StrongNameKeys keys, string filePath)
         {
-            Debug.Assert(inputStream is TempFileStream);
-
-            var tempStream = (TempFileStream)inputStream;
-            string assemblyFilePath = tempStream.Path;
-            tempStream.DisposeUnderlyingStream();
-
             if (keys.KeyContainer != null)
             {
-                Sign(assemblyFilePath, keys.KeyContainer);
+                Sign(filePath, keys.KeyContainer);
             }
             else
             {
-                Sign(assemblyFilePath, keys.KeyPair);
+                Sign(filePath, keys.KeyPair);
             }
+        }
+
+        internal override void SignStream(StrongNameKeys keys, Stream inputStream, Stream outputStream)
+        {
+            Debug.Assert(inputStream is TempFileStream);
+
+            var tempStream = (TempFileStream)inputStream;
+            string assemblyFilePath = tempStream.Path;
+            tempStream.DisposeUnderlyingStream();
+            SignFile(keys, assemblyFilePath);
 
             using (var fileToSign = new FileStream(assemblyFilePath, FileMode.Open))
             {

src/Compilers/Core/Portable/StrongName/StrongNameProvider.cs

@@ -29,6 +29,7 @@ protected StrongNameProvider()
         /// <summary>
         /// Create a <see cref="Stream"/> for use in when not signing with a builder (<see cref="Compilation.SignUsingBuilder"/>).
         /// </summary>
+        // TODO: delete this entirely as the stream management is now done via EmitStream
         // TOOD: Create and expose a SigningStream. This avoids unnecessary casting in the SignStream method
         internal virtual Stream CreateInputStream() => throw new NotSupportedException();
 
@@ -36,11 +37,18 @@ protected StrongNameProvider()
         /// Signs the <paramref name="inputStream"/> value using <paramref name="keys"/> and copies the final result
         /// to <paramref name="outputStream"/>
         /// </summary>
+        // TODO: delete this entirely as the stream management is now done via EmitStream
         internal virtual void SignStream(StrongNameKeys keys, Stream inputStream, Stream outputStream) => throw new NotSupportedException();
 
+        /// <summary>
+        /// Signs the <paramref name="filePath"/> value using <paramref name="keys"/>.
+        /// </summary>
+        internal virtual void SignFile(StrongNameKeys keys, string filePath) => throw new NotSupportedException();
+
         /// <summary>
         /// Signs the contents of <paramref name="peBuilder"/> using <paramref name="privateKey"/>.
         /// </summary>
+        // TODO: rename to SignBuilder
         internal virtual void SignPeBuilder(ExtendedPEBuilder peBuilder, BlobBuilder peBlob, RSAParameters privateKey) => throw new NotSupportedException();
 
         /// <summary>