From c6868765235e09642a04c66b84d8fc16b48e61fd Mon Sep 17 00:00:00 2001 From: PaulHigin Date: Wed, 13 Jul 2022 09:25:23 -0700 Subject: [PATCH 1/2] Some minor code clean up --- src/code/InstallHelper.cs | 6 +++++- src/code/Utils.cs | 41 +++++++++++++++++++++------------------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 5f2a7b06f..35fea684c 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -501,7 +501,11 @@ private List InstallPackage( : _pathsToInstallPkg.Find(path => path.EndsWith("Scripts", StringComparison.InvariantCultureIgnoreCase)); } - if (_authenticodeCheck && !AuthenticodeSignature.CheckAuthenticodeSignature(pkg.Name, tempDirNameVersion, _versionRange, _pathsToSearch, installPath, _cmdletPassedIn, out ErrorRecord errorRecord)) + if (_authenticodeCheck && !AuthenticodeSignature.CheckAuthenticodeSignature( + pkg.Name, + tempDirNameVersion, + _cmdletPassedIn, + out ErrorRecord errorRecord)) { ThrowTerminatingError(errorRecord); } diff --git a/src/code/Utils.cs b/src/code/Utils.cs index ae72d6b21..7e11a0db1 100644 --- a/src/code/Utils.cs +++ b/src/code/Utils.cs @@ -819,7 +819,7 @@ private static bool TryReadPSDataFile( } public static void ValidateModuleManifest(string moduleManifestPath, out string[] errorMsgs) - { + { List errorMsgsList = new List(); using (System.Management.Automation.PowerShell pwsh = System.Management.Automation.PowerShell.Create()) { @@ -866,11 +866,11 @@ public static void ValidateModuleManifest(string moduleManifestPath, out string[ { // This will handle version errors message = $"{pwsh.Streams.Error[0].ToString()} Run 'Test-ModuleManifest' to validate the module manifest."; - } - + } + errorMsgsList.Add(message); } - } + } errorMsgs = errorMsgsList.ToArray(); } @@ -1236,7 +1236,11 @@ internal static class AuthenticodeSignature { #region Methods - internal static bool CheckAuthenticodeSignature(string pkgName, string tempDirNameVersion, VersionRange versionRange, List pathsToSearch, string installPath, PSCmdlet cmdletPassedIn, out ErrorRecord errorRecord) + internal static bool CheckAuthenticodeSignature( + string pkgName, + string tempDirNameVersion, + PSCmdlet cmdletPassedIn, + out ErrorRecord errorRecord) { errorRecord = null; @@ -1251,7 +1255,7 @@ internal static bool CheckAuthenticodeSignature(string pkgName, string tempDirNa if (File.Exists(catalogFilePath)) { // Run catalog validation - Collection TestFileCatalogResult = new Collection(); + Collection TestFileCatalogResult; string moduleBasePath = tempDirNameVersion; try { @@ -1283,7 +1287,7 @@ internal static bool CheckAuthenticodeSignature(string pkgName, string tempDirNa return false; } - bool catalogValidation = (TestFileCatalogResult[0] != null) ? (bool)TestFileCatalogResult[0].BaseObject : false; + bool catalogValidation = TestFileCatalogResult.Count > 0 ? (bool)TestFileCatalogResult[0].BaseObject : false; if (!catalogValidation) { var exMessage = String.Format("The catalog file '{0}' is invalid.", pkgName + ".cat"); @@ -1292,13 +1296,15 @@ internal static bool CheckAuthenticodeSignature(string pkgName, string tempDirNa errorRecord = new ErrorRecord(ex, "TestFileCatalogError", ErrorCategory.InvalidResult, cmdletPassedIn); return false; } + + return true; } - Collection authenticodeSignature = new Collection(); + Collection authenticodeSignatures; try { string[] listOfExtensions = { "*.ps1", "*.psd1", "*.psm1", "*.mof", "*.cat", "*.ps1xml" }; - authenticodeSignature = cmdletPassedIn.InvokeCommand.InvokeScript( + authenticodeSignatures = cmdletPassedIn.InvokeCommand.InvokeScript( script: @"param ( [string] $tempDirNameVersion, [string[]] $listOfExtensions @@ -1316,19 +1322,16 @@ internal static bool CheckAuthenticodeSignature(string pkgName, string tempDirNa } // If the authenticode signature is not valid, return false - if (authenticodeSignature.Any() && authenticodeSignature[0] != null) + foreach (var signatureObject in authenticodeSignatures) { - foreach (var sign in authenticodeSignature) + Signature signature = (Signature)signatureObject.BaseObject; + if (!signature.Status.Equals(SignatureStatus.Valid)) { - Signature signature = (Signature)sign.BaseObject; - if (!signature.Status.Equals(SignatureStatus.Valid)) - { - var exMessage = String.Format("The signature for '{0}' is '{1}.", pkgName, signature.Status.ToString()); - var ex = new ArgumentException(exMessage); - errorRecord = new ErrorRecord(ex, "GetAuthenticodeSignatureError", ErrorCategory.InvalidResult, cmdletPassedIn); + var exMessage = String.Format("The signature for '{0}' is '{1}.", pkgName, signature.Status.ToString()); + var ex = new ArgumentException(exMessage); + errorRecord = new ErrorRecord(ex, "GetAuthenticodeSignatureError", ErrorCategory.InvalidResult, cmdletPassedIn); - return false; - } + return false; } } From bd286bff48d536e2f21f5cb4264d11747719eab6 Mon Sep 17 00:00:00 2001 From: PaulHigin Date: Wed, 13 Jul 2022 09:35:04 -0700 Subject: [PATCH 2/2] Update comments --- src/code/Utils.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/code/Utils.cs b/src/code/Utils.cs index 7e11a0db1..f19861847 100644 --- a/src/code/Utils.cs +++ b/src/code/Utils.cs @@ -1250,16 +1250,16 @@ internal static bool CheckAuthenticodeSignature( return true; } - // Check that the catalog file is signed properly + // First check if the files are catalog signed. string catalogFilePath = Path.Combine(tempDirNameVersion, pkgName + ".cat"); if (File.Exists(catalogFilePath)) { - // Run catalog validation + // Run catalog validation. Collection TestFileCatalogResult; string moduleBasePath = tempDirNameVersion; try { - // By default "Test-FileCatalog will look through all files in the provided directory, -FilesToSkip allows us to ignore specific files + // By default "Test-FileCatalog will look through all files in the provided directory, -FilesToSkip allows us to ignore specific files. TestFileCatalogResult = cmdletPassedIn.InvokeCommand.InvokeScript( script: @"param ( [string] $moduleBasePath, @@ -1300,6 +1300,7 @@ internal static bool CheckAuthenticodeSignature( return true; } + // Otherwise check for signatures on individual files. Collection authenticodeSignatures; try { @@ -1321,7 +1322,7 @@ internal static bool CheckAuthenticodeSignature( return false; } - // If the authenticode signature is not valid, return false + // If any file authenticode signatures are not valid, return false. foreach (var signatureObject in authenticodeSignatures) { Signature signature = (Signature)signatureObject.BaseObject;