From 697ce60a23ba2bc8b9983ba7d69116b20626a91e Mon Sep 17 00:00:00 2001 From: Chris Crowther Date: Wed, 13 Apr 2022 12:54:37 +0100 Subject: [PATCH 1/3] Added option to hold off between signing attempts. Largely added because even with "--mdop 1" I still periodically get errors from timestamp servers. --- .../AuthenticodeKeyVaultSigner.cs | 64 +++++++++++++++---- src/AzureSign.Core/Interop/HRESULT.cs | 10 +++ src/AzureSignTool/SignCommand.cs | 21 +++++- 3 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 src/AzureSign.Core/Interop/HRESULT.cs diff --git a/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs b/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs index d36df3f..e96d232 100644 --- a/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs +++ b/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs @@ -5,6 +5,7 @@ using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Threading; namespace AzureSign.Core { @@ -19,24 +20,27 @@ public class AuthenticodeKeyVaultSigner : IDisposable private readonly TimeStampConfiguration _timeStampConfiguration; private readonly MemoryCertificateStore _certificateStore; private readonly X509Chain _chain; - private readonly SignCallback _signCallback; + private readonly SignCallback _signCallback; + private readonly int? _signingThrottle; + private static DateTime _lastSigningOperation; /// /// Creates a new instance of . /// /// - /// An instance of an asymmetric algorithm that will be used to sign. It must support signing with - /// a private key. + /// An instance of an asymmetric algorithm that will be used to sign. It must support signing with + /// a private key. /// /// The X509 public certificate for the . /// The digest algorithm to sign the file. /// The timestamp configuration for timestamping the file. To omit timestamping, - /// use . + /// use . /// Any additional certificates to assist in building a certificate chain. + /// An optional hold-off period in seconds between signing attempts. public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Certificate2 signingCertificate, HashAlgorithmName fileDigestAlgorithm, TimeStampConfiguration timeStampConfiguration, - X509Certificate2Collection? additionalCertificates = null) + X509Certificate2Collection? additionalCertificates = null, int? signingThrottle = null) { _fileDigestAlgorithm = fileDigestAlgorithm; _signingCertificate = signingCertificate ?? throw new ArgumentNullException(nameof(signingCertificate)); @@ -44,6 +48,7 @@ public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Cert _signingAlgorithm = signingAlgorithm ?? throw new ArgumentNullException(nameof(signingAlgorithm)); _certificateStore = MemoryCertificateStore.Create(); _chain = new X509Chain(); + _signingThrottle = signingThrottle; if (additionalCertificates is not null) { @@ -63,20 +68,54 @@ public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Cert _certificateStore.Add(_chain.ChainElements[i].Certificate); } _signCallback = SignCallback; + + _lastSigningOperation = DateTime.MinValue; } /// Authenticode signs a file. - /// True if the signing process should try to include page hashing, otherwise false. - /// Use null to use the operating system default. Note that page hashing still may be disabled if the - /// Subject Interface Package does not support page hashing. - /// A URL describing the signature or the signer. - /// The description to apply to the signature. /// The path to the file to signed. + /// The description to apply to the signature. + /// A URL describing the signature or the signer. + /// True if the signing process should try to include page hashing, otherwise false. + /// Use null to use the operating system default. Note that page hashing still may be disabled if the + /// Subject Interface Package does not support page hashing. /// An optional logger to capture signing operations. + /// + ///A cancellation source that can be used to abort the operation. + /// /// A HRESULT indicating the result of the signing operation. S_OK, or zero, is returned if the signing /// operation completed successfully. - public unsafe int SignFile(ReadOnlySpan path, ReadOnlySpan description, ReadOnlySpan descriptionUrl, bool? pageHashing, ILogger? logger = null) + public unsafe int SignFile(ReadOnlySpan path, ReadOnlySpan description, + ReadOnlySpan descriptionUrl, bool? pageHashing, ILogger? logger = null, + CancellationTokenSource? cancellationTokenSource = null) { + if (_signingThrottle is > 1) + { + if (_lastSigningOperation == DateTime.MinValue) + { + _lastSigningOperation = DateTime.Now.AddSeconds(_signingThrottle.Value * -1); + } + + DateTime goTime = _lastSigningOperation.AddSeconds(_signingThrottle.Value); + + if (goTime >= DateTime.Now) + { + logger?.LogTrace($"Holding off between signing requests until {goTime}."); + + while (goTime >= DateTime.Now && cancellationTokenSource is {IsCancellationRequested: false}) + { + Thread.Sleep(500); + } + + logger?.LogTrace("Continuing signing operation."); + } + + if (cancellationTokenSource is {IsCancellationRequested: true}) + { + return HRESULT.E_ABORT; + } + } + static char[] NullTerminate(ReadOnlySpan str) { char[] result = new char[str.Length + 1]; @@ -197,6 +236,9 @@ static char[] NullTerminate(ReadOnlySpan str) Marshal.Release(state); } } + + _lastSigningOperation = DateTime.Now; + return result; } } diff --git a/src/AzureSign.Core/Interop/HRESULT.cs b/src/AzureSign.Core/Interop/HRESULT.cs new file mode 100644 index 0000000..9c79601 --- /dev/null +++ b/src/AzureSign.Core/Interop/HRESULT.cs @@ -0,0 +1,10 @@ +namespace AzureSign.Core.Interop +{ + internal static class HRESULT + { + /// + /// Operation aborted. + /// + public const int E_ABORT = unchecked((int)0x80004004); + } +} diff --git a/src/AzureSignTool/SignCommand.cs b/src/AzureSignTool/SignCommand.cs index dd7b5e1..7695c8f 100644 --- a/src/AzureSignTool/SignCommand.cs +++ b/src/AzureSignTool/SignCommand.cs @@ -86,6 +86,9 @@ internal sealed class SignCommand [Option("-mdop | --max-degree-of-parallelism", "The maximum number of concurrent signing operations.", CommandOptionType.SingleValue), Range(-1, int.MaxValue)] public int? MaxDegreeOfParallelism { get; set; } + [Option("-st | --signing-throttle", "Controls the rate of signing operations. If this value is specified it indicates the number of seconds to hold off between each signing operation. Only valid when parallelism is disabled.", CommandOptionType.SingleValue), Range(-1, int.MaxValue)] + public int? SigningThrottle { get; set; } + [Option("--colors", "Enable color output on the command line.", CommandOptionType.NoValue)] public bool Colors { get; set; } = false; @@ -152,6 +155,11 @@ private ValidationResult OnValidate(ValidationContext context, CommandLineContex return new ValidationResult("Cannot use '--timestamp-rfc3161' and '--timestamp-authenticode' options together.", new[] { nameof(Rfc3161Timestamp), nameof(AuthenticodeTimestamp) }); } + if (SigningThrottle is > 0 && MaxDegreeOfParallelism > 1) + { + return new ValidationResult("Cannot use '--signing-throttle' and '--max-degree-of-parallelism' options together."); + } + if (KeyVaultClientId.Present && !KeyVaultClientSecret.Present) { return new ValidationResult("Must supply '--azure-key-vault-client-secret' when using '--azure-key-vault-client-id'.", new[] { nameof(KeyVaultClientSecret) }); @@ -169,7 +177,7 @@ private ValidationResult OnValidate(ValidationContext context, CommandLineContex { return new ValidationResult("At least one file must be specified to sign."); } - foreach(var file in AllFiles) + foreach (var file in AllFiles) { if (!File.Exists(file)) { @@ -254,6 +262,12 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso { performPageHashing = false; } + if (SigningThrottle is > 0) + { + logger?.LogTrace($"Forcing MaxDegreeOfParallelism to 1 because signing throttling is requested."); + MaxDegreeOfParallelism = 1; + } + var configurationDiscoverer = new KeyVaultConfigurationDiscoverer(logger); var materializedResult = await configurationDiscoverer.Materialize(configuration); AzureKeyVaultMaterializedConfiguration materialized; @@ -279,10 +293,11 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso { options.MaxDegreeOfParallelism = MaxDegreeOfParallelism.Value; } + logger.LogTrace("Creating context"); using (var keyVault = RSAFactory.Create(materialized.TokenCredential, materialized.KeyId, materialized.PublicCertificate)) - using (var signer = new AuthenticodeKeyVaultSigner(keyVault, materialized.PublicCertificate, FileDigestAlgorithm, timeStampConfiguration, certificates)) + using (var signer = new AuthenticodeKeyVaultSigner(keyVault, materialized.PublicCertificate, FileDigestAlgorithm, timeStampConfiguration, certificates, SigningThrottle)) { Parallel.ForEach(AllFiles, options, () => (succeeded: 0, failed: 0), (filePath, pls, state) => { @@ -304,7 +319,7 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso return (state.succeeded + 1, state.failed); } - var result = signer.SignFile(filePath, Description, DescriptionUri, performPageHashing, logger); + var result = signer.SignFile(filePath, Description, DescriptionUri, performPageHashing, logger, cancellationSource); switch (result) { case COR_E_BADIMAGEFORMAT: From 4a1a80300bacd7636be5d9d3acb298685a2f9a78 Mon Sep 17 00:00:00 2001 From: Chris Crowther Date: Wed, 13 Apr 2022 13:10:14 +0100 Subject: [PATCH 2/3] Updated README.md file with new option. --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 938187a..8fad58c 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,9 @@ The `--help` or `sign --help` option provides more detail about each parameter. the system will use the default based on the number of available processor threads. Setting this value to "1" disable concurrent signing. +* `--signing-throttle` [short: `-st`, required: no]: When specified instructs the tool to throttle the rate of + signing operations. Cannot be combined with -mdop (other than with a value of 1). The value is the minimum time between requests in seconds. + In most circumances, using the defaults for page hashing is recommended, which can be done by simply omitting both of the parameters. ## Supported Formats From f34478129c7e7faf21b3f38f1feccbf19b34787d Mon Sep 17 00:00:00 2001 From: Chris Crowther Date: Tue, 19 Apr 2022 10:17:10 +0100 Subject: [PATCH 3/3] Relocate the hold off code to the SignCommand. --- .../AuthenticodeKeyVaultSigner.cs | 44 +------------------ src/AzureSign.Core/Interop/HRESULT.cs | 10 ----- src/AzureSignTool/SignCommand.cs | 36 ++++++++++++++- 3 files changed, 36 insertions(+), 54 deletions(-) delete mode 100644 src/AzureSign.Core/Interop/HRESULT.cs diff --git a/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs b/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs index e96d232..4c73824 100644 --- a/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs +++ b/src/AzureSign.Core/AuthenticodeKeyVaultSigner.cs @@ -21,9 +21,6 @@ public class AuthenticodeKeyVaultSigner : IDisposable private readonly MemoryCertificateStore _certificateStore; private readonly X509Chain _chain; private readonly SignCallback _signCallback; - private readonly int? _signingThrottle; - - private static DateTime _lastSigningOperation; /// /// Creates a new instance of . @@ -37,10 +34,9 @@ public class AuthenticodeKeyVaultSigner : IDisposable /// The timestamp configuration for timestamping the file. To omit timestamping, /// use . /// Any additional certificates to assist in building a certificate chain. - /// An optional hold-off period in seconds between signing attempts. public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Certificate2 signingCertificate, HashAlgorithmName fileDigestAlgorithm, TimeStampConfiguration timeStampConfiguration, - X509Certificate2Collection? additionalCertificates = null, int? signingThrottle = null) + X509Certificate2Collection? additionalCertificates = null) { _fileDigestAlgorithm = fileDigestAlgorithm; _signingCertificate = signingCertificate ?? throw new ArgumentNullException(nameof(signingCertificate)); @@ -48,7 +44,6 @@ public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Cert _signingAlgorithm = signingAlgorithm ?? throw new ArgumentNullException(nameof(signingAlgorithm)); _certificateStore = MemoryCertificateStore.Create(); _chain = new X509Chain(); - _signingThrottle = signingThrottle; if (additionalCertificates is not null) { @@ -68,8 +63,6 @@ public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Cert _certificateStore.Add(_chain.ChainElements[i].Certificate); } _signCallback = SignCallback; - - _lastSigningOperation = DateTime.MinValue; } /// Authenticode signs a file. @@ -80,42 +73,11 @@ public AuthenticodeKeyVaultSigner(AsymmetricAlgorithm signingAlgorithm, X509Cert /// Use null to use the operating system default. Note that page hashing still may be disabled if the /// Subject Interface Package does not support page hashing. /// An optional logger to capture signing operations. - /// - ///A cancellation source that can be used to abort the operation. - /// /// A HRESULT indicating the result of the signing operation. S_OK, or zero, is returned if the signing /// operation completed successfully. public unsafe int SignFile(ReadOnlySpan path, ReadOnlySpan description, - ReadOnlySpan descriptionUrl, bool? pageHashing, ILogger? logger = null, - CancellationTokenSource? cancellationTokenSource = null) + ReadOnlySpan descriptionUrl, bool? pageHashing, ILogger? logger = null) { - if (_signingThrottle is > 1) - { - if (_lastSigningOperation == DateTime.MinValue) - { - _lastSigningOperation = DateTime.Now.AddSeconds(_signingThrottle.Value * -1); - } - - DateTime goTime = _lastSigningOperation.AddSeconds(_signingThrottle.Value); - - if (goTime >= DateTime.Now) - { - logger?.LogTrace($"Holding off between signing requests until {goTime}."); - - while (goTime >= DateTime.Now && cancellationTokenSource is {IsCancellationRequested: false}) - { - Thread.Sleep(500); - } - - logger?.LogTrace("Continuing signing operation."); - } - - if (cancellationTokenSource is {IsCancellationRequested: true}) - { - return HRESULT.E_ABORT; - } - } - static char[] NullTerminate(ReadOnlySpan str) { char[] result = new char[str.Length + 1]; @@ -237,8 +199,6 @@ static char[] NullTerminate(ReadOnlySpan str) } } - _lastSigningOperation = DateTime.Now; - return result; } } diff --git a/src/AzureSign.Core/Interop/HRESULT.cs b/src/AzureSign.Core/Interop/HRESULT.cs deleted file mode 100644 index 9c79601..0000000 --- a/src/AzureSign.Core/Interop/HRESULT.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace AzureSign.Core.Interop -{ - internal static class HRESULT - { - /// - /// Operation aborted. - /// - public const int E_ABORT = unchecked((int)0x80004004); - } -} diff --git a/src/AzureSignTool/SignCommand.cs b/src/AzureSignTool/SignCommand.cs index 7695c8f..0270688 100644 --- a/src/AzureSignTool/SignCommand.cs +++ b/src/AzureSignTool/SignCommand.cs @@ -99,7 +99,9 @@ internal sealed class SignCommand [Argument(0, "file", "The path to the file.")] public string[] Files { get; set; } = Array.Empty(); + private static DateTime _lastSigningOperation; private HashSet _allFiles; + public HashSet AllFiles { get @@ -266,6 +268,7 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso { logger?.LogTrace($"Forcing MaxDegreeOfParallelism to 1 because signing throttling is requested."); MaxDegreeOfParallelism = 1; + _lastSigningOperation = DateTime.Now.AddSeconds(SigningThrottle.Value * -1); } var configurationDiscoverer = new KeyVaultConfigurationDiscoverer(logger); @@ -297,7 +300,7 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso logger.LogTrace("Creating context"); using (var keyVault = RSAFactory.Create(materialized.TokenCredential, materialized.KeyId, materialized.PublicCertificate)) - using (var signer = new AuthenticodeKeyVaultSigner(keyVault, materialized.PublicCertificate, FileDigestAlgorithm, timeStampConfiguration, certificates, SigningThrottle)) + using (var signer = new AuthenticodeKeyVaultSigner(keyVault, materialized.PublicCertificate, FileDigestAlgorithm, timeStampConfiguration, certificates)) { Parallel.ForEach(AllFiles, options, () => (succeeded: 0, failed: 0), (filePath, pls, state) => { @@ -309,6 +312,33 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso { return state; } + + if (SigningThrottle is > 0) + { + DateTime goTime = _lastSigningOperation.AddSeconds(SigningThrottle.Value); + + if (goTime >= DateTime.Now) + { + logger.LogTrace("Holding off between signing requests until {time}.", goTime); + + while (goTime >= DateTime.Now && !cancellationSource.IsCancellationRequested) + { + Thread.Sleep(500); + } + + logger.LogTrace("Continuing signing operation."); + } + + if (cancellationSource.IsCancellationRequested) + { + pls.Stop(); + } + if (pls.IsStopped) + { + return state; + } + } + using (logger.BeginScope("File: {Id}", filePath)) { logger.LogInformation("Signing file."); @@ -319,7 +349,7 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso return (state.succeeded + 1, state.failed); } - var result = signer.SignFile(filePath, Description, DescriptionUri, performPageHashing, logger, cancellationSource); + var result = signer.SignFile(filePath, Description, DescriptionUri, performPageHashing, logger); switch (result) { case COR_E_BADIMAGEFORMAT: @@ -330,6 +360,8 @@ public async Task OnExecuteAsync(CommandLineApplication app, IConsole conso break; } + _lastSigningOperation = DateTime.Now; + if (result == S_OK) { logger.LogInformation("Signing completed successfully.");