commit e8f5c4352f1e0ef6159d4100a6ef691b68b0e992 parent a91559d09e1fd146ee28443b147a81a14c9d8ffa Author: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 28 Jul 2022 14:10:23 -0700 Validate SignatureSha256 for MSIX packages during the manifest validation (#2384) Diffstat:
24 files changed, 528 insertions(+), 162 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/HashCommand.cpp b/src/AppInstallerCLICore/Commands/HashCommand.cpp @@ -51,8 +51,7 @@ namespace AppInstaller::CLI try { Msix::MsixInfo msixInfo{ inputFile }; - auto signature = msixInfo.GetSignature(); - auto signatureHash = Utility::SHA256::ComputeHash(signature.data(), static_cast<uint32_t>(signature.size())); + auto signatureHash = msixInfo.GetSignatureHash(); context.Reporter.Info() << "SignatureSha256: "_liv << Utility::LocIndString{ Utility::SHA256::ConvertToString(signatureHash) } << std::endl; } diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp @@ -293,9 +293,7 @@ namespace AppInstaller::CLI::Workflow const auto& installer = context.Get<Execution::Data::Installer>().value(); Msix::MsixInfo msixInfo(installer.Url); - auto signature = msixInfo.GetSignature(); - - auto signatureHash = SHA256::ComputeHash(signature.data(), static_cast<uint32_t>(signature.size())); + auto signatureHash = msixInfo.GetSignatureHash(); context.Add<Execution::Data::HashPair>(std::make_pair(installer.SignatureSha256, signatureHash)); } diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -297,7 +297,7 @@ </CopyFileToFolders> <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MultipleNonPortableNestedInstallers.yaml"> <DeploymentContent>true</DeploymentContent> - </CopyFileToFolders> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\ImportFile-Bad-Invalid.json"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> @@ -624,7 +624,7 @@ </CopyFileToFolders> <CopyFileToFolders Include="TestData\UpdateFlowTest_ZipWithExe.yaml"> <DeploymentContent>true</DeploymentContent> - </CopyFileToFolders> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\InputNames.txt"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> @@ -730,6 +730,30 @@ <CopyFileToFolders Include="TestData\Installer-Good.msixbundle"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Installer-Signed-Good.msix"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Installer-Signed-Good.msixbundle"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixInstallerFields.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Good-SignedMsixInstaller.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Good-MsixBundleInstaller.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentMsixBundleInstallerFields.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Good-SignedMsixBundleInstaller.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> </ItemGroup> <ItemGroup> <ProjectReference Include="..\AppInstallerCLICore\AppInstallerCLICore.vcxproj"> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -218,10 +218,10 @@ <ClCompile Include="Archive.cpp"> <Filter>Source Files\Common</Filter> </ClCompile> - <ClCompile Include="FolderFileWatcher.cpp"> - <Filter>Source Files\Common</Filter> - </ClCompile> <ClCompile Include="Filesystem.cpp"> + <Filter>Source Files</Filter> + </ClCompile> + <ClCompile Include="FolderFileWatcher.cpp"> <Filter>Source Files\Common</Filter> </ClCompile> </ItemGroup> @@ -699,12 +699,38 @@ <CopyFileToFolders Include="TestData\Manifest-Bad-NoSupportedPlatforms.yaml"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Installer-Bad-NoSupportedPlatforms.msix"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\Installer-Good.msix"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Installer-Good.msixbundle"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> - <CopyFileToFolders Include="TestData\Installer-Bad-NoSupportedPlatforms.msix" /> + <CopyFileToFolders Include="TestData\Installer-Signed-Good.msix"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Installer-Signed-Good.msixbundle"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixInstallerFields.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Good-SignedMsixInstaller.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Good-MsixBundleInstaller.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentMsixBundleInstallerFields.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Good-SignedMsixBundleInstaller.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> </ItemGroup> </Project> \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/Installer-Signed-Good.msix b/src/AppInstallerCLITests/TestData/Installer-Signed-Good.msix Binary files differ. diff --git a/src/AppInstallerCLITests/TestData/Installer-Signed-Good.msixbundle b/src/AppInstallerCLITests/TestData/Installer-Signed-Good.msixbundle Binary files differ. diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentMsixBundleInstallerFields.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentMsixBundleInstallerFields.yaml @@ -0,0 +1,15 @@ +# Bad manifest: Inconsistent field values in unsigned MSIX bundle +PackageIdentifier: AppInstallerCliTest.InconsistentMsixBundleInstallerFields +PackageVersion: 1.2.3.4 # Mismatching value with msix installer +PackageLocale: es-MX +PackageName: es-MX package name +Publisher: es-MX publisher +PackageFamilyName: FakeInstallerForTesting_Bad # Mismatching value with msix installer +MinimumOSVersion: 5.6.7.8 # Mismatching value with msix installer +InstallerType: msix +Installers: + - Architecture: x64 + InstallerUrl: Installer-Good.msixbundle + SignatureSha256: 0000000000000000000000000000000000000000000000000000000000000000 # Unexpected value for unsigned msix installer +ManifestType: merged +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentMsixInstallerFields.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentMsixInstallerFields.yaml @@ -1,3 +1,4 @@ +# Bad manifest: Inconsistent field values in unsigned MSIX PackageIdentifier: AppInstallerCliTest.InconsistentMsixInstallerFields PackageVersion: 1.2.3.4 # Mismatching value with msix installer PackageLocale: es-MX @@ -9,5 +10,6 @@ InstallerType: msix Installers: - Architecture: x64 InstallerUrl: Installer-Good.msix + SignatureSha256: 0000000000000000000000000000000000000000000000000000000000000000 # Unexpected value for unsigned msix installer ManifestType: merged ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml @@ -0,0 +1,15 @@ +# Bad manifest: Inconsistent field values in signed MSIX bundle +PackageIdentifier: AppInstallerCliTest.InconsistentMsixBundleInstallerFields +PackageVersion: 1.2.3.4 # Mismatching value with msix installer +PackageLocale: es-MX +PackageName: es-MX package name +Publisher: es-MX publisher +PackageFamilyName: FakeInstallerForTesting_Bad # Mismatching value with msix installer +MinimumOSVersion: 5.6.7.8 # Mismatching value with msix installer +InstallerType: msix +Installers: + - Architecture: x64 + InstallerUrl: Installer-Signed-Good.msixbundle + SignatureSha256: 0000000000000000000000000000000000000000000000000000000000000000 # Inconsistent signature hash +ManifestType: merged +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentSignedMsixInstallerFields.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentSignedMsixInstallerFields.yaml @@ -0,0 +1,15 @@ +# Bad manifest: Inconsistent field values in signed MSIX +PackageIdentifier: AppInstallerCliTest.InconsistentMsixInstallerFields +PackageVersion: 1.2.3.4 # Mismatching value with msix installer +PackageLocale: es-MX +PackageName: es-MX package name +Publisher: es-MX publisher +PackageFamilyName: FakeInstallerForTesting_Bad # Mismatching value with msix installer +MinimumOSVersion: 5.6.7.8 # Mismatching value with msix installer +InstallerType: msix +Installers: + - Architecture: x64 + InstallerUrl: Installer-Signed-Good.msix + SignatureSha256: 0000000000000000000000000000000000000000000000000000000000000000 # Inconsistent signature hash +ManifestType: merged +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-MsixBundleInstaller.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-MsixBundleInstaller.yaml @@ -0,0 +1,13 @@ +PackageIdentifier: AppInstallerCliTest.GoodMsixBundleInstaller +PackageVersion: 43690.48059.52428.56797 +PackageLocale: es-MX +PackageName: es-MX package name +Publisher: es-MX publisher +PackageFamilyName: FakeInstallerForTesting_125rzkzqaqjwj +MinimumOSVersion: 10.0.16299.0 +InstallerType: msix +Installers: + - Architecture: x64 + InstallerUrl: Installer-Good.msixbundle +ManifestType: merged +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-SignedMsixBundleInstaller.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-SignedMsixBundleInstaller.yaml @@ -0,0 +1,14 @@ +PackageIdentifier: AppInstallerCliTest.GoodSignedMsixBundleInstaller +PackageVersion: 43690.48059.52428.56797 +PackageLocale: es-MX +PackageName: es-MX package name +Publisher: es-MX publisher +PackageFamilyName: FakeInstallerForTesting_125rzkzqaqjwj +MinimumOSVersion: 10.0.16299.0 +InstallerType: msix +Installers: + - Architecture: x64 + InstallerUrl: Installer-Signed-Good.msixbundle + SignatureSha256: d70bd623f87b6ce4ddba4506c6000cf43ef3af4ab1207f5579ec43400de1623f +ManifestType: merged +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-SignedMsixInstaller.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-SignedMsixInstaller.yaml @@ -0,0 +1,14 @@ +PackageIdentifier: AppInstallerCliTest.GoodSignedMsixInstaller +PackageVersion: 43690.48059.52428.56797 +PackageLocale: es-MX +PackageName: es-MX package name +Publisher: es-MX publisher +PackageFamilyName: FakeInstallerForTesting_125rzkzqaqjwj +MinimumOSVersion: 10.0.0.0 +InstallerType: msix +Installers: + - Architecture: x64 + InstallerUrl: Installer-Signed-Good.msix + SignatureSha256: 50562001202c8dad456474d3f20903138d0a15c44ee497c3d4f82e85edbf2f97 +ManifestType: merged +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp @@ -31,6 +31,24 @@ bool operator==(const MultiValue& a, const MultiValue& b) return true; } +void ValidateError( + const ValidationError& error, + ValidationError::Level level, + std::string message, + std::string field, + std::string value) +{ + REQUIRE(level == error.ErrorLevel); + REQUIRE(message == error.Message); + REQUIRE(field == error.Field); + REQUIRE(value == error.Value); +} + +void ValidateError(const ValidationError& error, ValidationError::Level level, std::string message) +{ + ValidateError(error, level, message, std::string(), std::string()); +} + TEST_CASE("ReadPreviewGoodManifestAndVerifyContents", "[ManifestValidation]") { auto manifestFile = TestDataFile("Manifest-Good.yaml"); @@ -951,25 +969,12 @@ TEST_CASE("ReadManifestAndValidateMsixInstallers_InconsistentFields", "[Manifest manifest.Installers[0].Url = msixFile.GetPath().u8string(); auto errors = ValidateManifestInstallers(manifest); - REQUIRE(3 == errors.size()); - - // Package family name - REQUIRE(ValidationError::Level::Error == errors[0].ErrorLevel); - REQUIRE(ManifestError::InstallerMsixInconsistencies == errors[0].Message); - REQUIRE("PackageFamilyName" == errors[0].Field); - REQUIRE("FakeInstallerForTesting_125rzkzqaqjwj" == errors[0].Value); - - // Package version - REQUIRE(ValidationError::Level::Error == errors[1].ErrorLevel); - REQUIRE(ManifestError::InstallerMsixInconsistencies == errors[1].Message); - REQUIRE("PackageVersion" == errors[1].Field); - REQUIRE("43690.48059.52428.56797" == errors[1].Value); - - // Min OS version - REQUIRE(ValidationError::Level::Error == errors[2].ErrorLevel); - REQUIRE(ManifestError::InstallerMsixInconsistencies == errors[2].Message); - REQUIRE("MinimumOSVersion" == errors[2].Field); - REQUIRE("10.0.0.0" == errors[2].Value); + REQUIRE(4 == errors.size()); + + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::MsixSignatureHashFailed); + ValidateError(errors[1], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[2], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797"); + ValidateError(errors[3], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", "10.0.0.0"); } TEST_CASE("ReadManifestAndValidateMsixInstallers_NoSupportedPlatforms", "[ManifestValidation]") @@ -986,10 +991,7 @@ TEST_CASE("ReadManifestAndValidateMsixInstallers_NoSupportedPlatforms", "[Manife auto errors = ValidateManifestInstallers(manifest); REQUIRE(1 == errors.size()); - REQUIRE(ValidationError::Level::Error == errors[0].ErrorLevel); - REQUIRE(ManifestError::NoSupportedPlatforms == errors[0].Message); - REQUIRE("InstallerUrl" == errors[0].Field); - REQUIRE(manifest.Installers.front().Url == errors[0].Value); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::NoSupportedPlatforms, "InstallerUrl", manifest.Installers.front().Url); } TEST_CASE("ReadManifestAndValidateMsixInstallers_MissingFields", "[ManifestValidation]") @@ -1008,20 +1010,124 @@ TEST_CASE("ReadManifestAndValidateMsixInstallers_MissingFields", "[ManifestValid auto expectedLevel = treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error; REQUIRE(2 == errors.size()); - // Package family name - REQUIRE(expectedLevel == errors[0].ErrorLevel); - REQUIRE(ManifestError::OptionalFieldMissing == errors[0].Message); - REQUIRE("PackageFamilyName" == errors[0].Field); - REQUIRE("FakeInstallerForTesting_125rzkzqaqjwj" == errors[0].Value); - - // Min OS version - REQUIRE(expectedLevel == errors[1].ErrorLevel); - REQUIRE(ManifestError::OptionalFieldMissing == errors[1].Message); - REQUIRE("MinimumOSVersion" == errors[1].Field); - REQUIRE("10.0.0.0" == errors[1].Value); + ValidateError(errors[0], expectedLevel, ManifestError::OptionalFieldMissing, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[1], expectedLevel, ManifestError::OptionalFieldMissing, "MinimumOSVersion", "10.0.0.0"); } } +TEST_CASE("ReadManifestAndValidateMsixInstallers_Signed_Success", "[ManifestValidation]") +{ + TestDataFile testFile("Manifest-Good-SignedMsixInstaller.yaml"); + Manifest manifest = YamlParser::CreateFromPath(testFile); + + // Update the installer path for testing + REQUIRE(1 == manifest.Installers.size()); + TestDataFile msixFile(manifest.Installers[0].Url.c_str()); + manifest.Installers[0].Url = msixFile.GetPath().u8string(); + + auto errors = ValidateManifestInstallers(manifest); + REQUIRE(0 == errors.size()); +} + +TEST_CASE("ReadManifestAndValidateMsixInstallers_Signed_InconsistentFields", "[ManifestValidation]") +{ + TestDataFile testFile("Manifest-Bad-InconsistentSignedMsixInstallerFields.yaml"); + Manifest manifest = YamlParser::CreateFromPath(testFile); + + // Update the installer path for testing + REQUIRE(1 == manifest.Installers.size()); + TestDataFile msixFile(manifest.Installers[0].Url.c_str()); + manifest.Installers[0].Url = msixFile.GetPath().u8string(); + + auto errors = ValidateManifestInstallers(manifest); + REQUIRE(4 == errors.size()); + + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "SignatureSha256", "50562001202c8dad456474d3f20903138d0a15c44ee497c3d4f82e85edbf2f97"); + ValidateError(errors[1], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[2], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797"); + ValidateError(errors[3], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", "10.0.0.0"); +} + +TEST_CASE("ReadManifestAndValidateMsixBundleInstallers_Success", "[ManifestValidation]") +{ + TestDataFile testFile("Manifest-Good-MsixBundleInstaller.yaml"); + Manifest manifest = YamlParser::CreateFromPath(testFile); + + // Update the installer path for testing + REQUIRE(1 == manifest.Installers.size()); + TestDataFile msixFile(manifest.Installers[0].Url.c_str()); + manifest.Installers[0].Url = msixFile.GetPath().u8string(); + + auto errors = ValidateManifestInstallers(manifest); + REQUIRE(0 == errors.size()); +} + +TEST_CASE("ReadManifestAndValidateMsixBundleInstallers_InconsistentFields", "[ManifestValidation]") +{ + TestDataFile testFile("Manifest-Bad-InconsistentMsixBundleInstallerFields.yaml"); + Manifest manifest = YamlParser::CreateFromPath(testFile); + + // Update the installer path for testing + REQUIRE(1 == manifest.Installers.size()); + TestDataFile msixFile(manifest.Installers[0].Url.c_str()); + manifest.Installers[0].Url = msixFile.GetPath().u8string(); + + auto errors = ValidateManifestInstallers(manifest); + REQUIRE(7 == errors.size()); + + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::MsixSignatureHashFailed); + + // Validate errors for the first msix package in the msix bundle + ValidateError(errors[1], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[2], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797"); + ValidateError(errors[3], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", "10.0.16299.0"); + + // Validate errors for the second msix package in the msix bundle + ValidateError(errors[4], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[5], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797"); + ValidateError(errors[6], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", "10.0.16299.0"); +} + +TEST_CASE("ReadManifestAndValidateMsixBundleInstallers_Signed_Success", "[ManifestValidation]") +{ + TestDataFile testFile("Manifest-Good-SignedMsixBundleInstaller.yaml"); + Manifest manifest = YamlParser::CreateFromPath(testFile); + + // Update the installer path for testing + REQUIRE(1 == manifest.Installers.size()); + TestDataFile msixFile(manifest.Installers[0].Url.c_str()); + manifest.Installers[0].Url = msixFile.GetPath().u8string(); + + auto errors = ValidateManifestInstallers(manifest); + REQUIRE(0 == errors.size()); +} + +TEST_CASE("ReadManifestAndValidateMsixBundleInstallers_Signed_InconsistentFields", "[ManifestValidation]") +{ + TestDataFile testFile("Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml"); + Manifest manifest = YamlParser::CreateFromPath(testFile); + + // Update the installer path for testing + REQUIRE(1 == manifest.Installers.size()); + TestDataFile msixFile(manifest.Installers[0].Url.c_str()); + manifest.Installers[0].Url = msixFile.GetPath().u8string(); + + auto errors = ValidateManifestInstallers(manifest); + REQUIRE(7 == errors.size()); + + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "SignatureSha256", "d70bd623f87b6ce4ddba4506c6000cf43ef3af4ab1207f5579ec43400de1623f"); + + // Validate errors for the first msix package in the msix bundle + ValidateError(errors[1], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[2], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797"); + ValidateError(errors[3], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", "10.0.16299.0"); + + // Validate errors for the second msix package in the msix bundle + ValidateError(errors[4], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", "FakeInstallerForTesting_125rzkzqaqjwj"); + ValidateError(errors[5], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797"); + ValidateError(errors[6], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", "10.0.16299.0"); +} + TEST_CASE("ManifestArpVersionRange", "[ManifestValidation]") { Manifest manifestNoArp = YamlParser::CreateFromPath(TestDataFile("Manifest-Good-NoArpVersionDeclared.yaml")); diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj @@ -306,7 +306,7 @@ <ClInclude Include="Public\AppInstallerLogging.h" /> <ClInclude Include="Public\AppInstallerArchitecture.h" /> <ClInclude Include="Public\AppInstallerVersions.h" /> - <ClInclude Include="Public\winget\Archive.h" /> + <ClInclude Include="Public\winget\Archive.h" /> <ClInclude Include="Public\winget\ExperimentalFeature.h" /> <ClInclude Include="Public\winget\ExtensionCatalog.h" /> <ClInclude Include="Public\winget\JsonSchemaValidation.h" /> @@ -322,6 +322,7 @@ <ClInclude Include="Public\winget\ManifestYamlParser.h" /> <ClInclude Include="Public\winget\ManifestYamlPopulator.h" /> <ClInclude Include="Public\winget\MsiExecArguments.h" /> + <ClInclude Include="Public\winget\MsixManifestValidation.h" /> <ClInclude Include="Public\winget\NameNormalization.h" /> <ClInclude Include="Public\winget\Filesystem.h" /> <ClInclude Include="Public\winget\Regex.h" /> @@ -381,6 +382,7 @@ <ClCompile Include="Manifest\ManifestValidation.cpp" /> <ClCompile Include="Manifest\ManifestSchemaValidation.cpp" /> <ClCompile Include="Manifest\ManifestYamlPopulator.cpp" /> + <ClCompile Include="Manifest\MsixManifestValidation.cpp" /> <ClCompile Include="Manifest\YamlParser.cpp" /> <ClCompile Include="MsiExecArguments.cpp" /> <ClCompile Include="MsixInfo.cpp"> @@ -396,7 +398,7 @@ <PrecompiledHeader>Create</PrecompiledHeader> </ClCompile> <ClCompile Include="AppInstallerTelemetry.cpp" /> - <ClCompile Include="Archive.cpp" /> + <ClCompile Include="Archive.cpp" /> <ClCompile Include="Settings.cpp" /> <ClCompile Include="SHA256.cpp" /> <ClCompile Include="PortableARPEntry.cpp" /> diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters @@ -210,6 +210,9 @@ <ClInclude Include="Public\winget\Archive.h"> <Filter>Public\winget</Filter> </ClInclude> + <ClInclude Include="Public\winget\MsixManifestValidation.h"> + <Filter>Header Files</Filter> + </ClInclude> </ItemGroup> <ItemGroup> <ClCompile Include="pch.cpp"> @@ -371,6 +374,9 @@ <ClCompile Include="Archive.cpp"> <Filter>Source Files</Filter> </ClCompile> + <ClCompile Include="Manifest\MsixManifestValidation.cpp"> + <Filter>Source Files</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -5,6 +5,7 @@ #include "AppInstallerMsixInfo.h" #include "winget/MsixManifest.h" #include "winget/ManifestValidation.h" +#include "winget/MsixManifestValidation.h" #include "winget/Locale.h" #include "winget/Filesystem.h" @@ -295,103 +296,17 @@ namespace AppInstaller::Manifest return resultErrors; } - // Validate msix and msixbundle installer manifest - std::vector<ValidationError> ValidateMsixManifest( - const Msix::PackageVersion& packageVersion, - const ManifestInstaller& installer, - Msix::MsixPackageManifestCache& msixManifestsCache, - bool treatErrorAsWarning) - { - std::vector<ValidationError> errors; - std::optional<Msix::OSVersion> installerMinOSVersion; - - try - { - if (!installer.MinOSVersion.empty()) - { - installerMinOSVersion = std::make_optional<Msix::OSVersion>(installer.MinOSVersion); - } - } - catch (const std::exception&) - { - errors.emplace_back(ManifestError::InvalidFieldValue, "MinimumOSVersion", installer.MinOSVersion); - } - - std::vector<Msix::MsixPackageManifest> msixManifests; - try - { - msixManifests = msixManifestsCache.GetAppPackageManifests(installer.Url); - } - catch (...) - { - errors.emplace_back(ManifestError::InstallerFailedToProcess, "InstallerUrl", installer.Url); - } - - for (auto msixManifest : msixManifests) - { - // Validate package family name - auto msixManifestIdentity = msixManifest.GetIdentity(); - auto msixPackageFamilyName = msixManifestIdentity.GetPackageFamilyName(); - if (!installer.PackageFamilyName.empty()) - { - if (installer.PackageFamilyName != msixPackageFamilyName) - { - errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", msixPackageFamilyName); - } - } - // Yaml manifest missing package family name - else - { - errors.emplace_back( - ManifestError::OptionalFieldMissing, - "PackageFamilyName", - msixPackageFamilyName, - treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error); - } - - // Validate package version - auto msixVersion = msixManifestIdentity.GetVersion(); - if (msixVersion != packageVersion) - { - errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "PackageVersion", msixVersion.ToString()); - } - - // Validate min OS version - auto targetMinOSVersion = msixManifest.GetMinimumOSVersionForSupportedPlatforms(); - if (!targetMinOSVersion.has_value()) - { - errors.emplace_back(ManifestError::NoSupportedPlatforms, "InstallerUrl", installer.Url); - } - else if (installerMinOSVersion.has_value()) - { - if (targetMinOSVersion.value() != installerMinOSVersion.value()) - { - errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", targetMinOSVersion.value().ToString()); - } - } - else - { - errors.emplace_back( - ManifestError::OptionalFieldMissing, - "MinimumOSVersion", - targetMinOSVersion.value().ToString(), - treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error); - } - } - - return errors; - } - std::vector<ValidationError> ValidateManifestInstallers(const Manifest& manifest, bool treatErrorAsWarning) { std::vector<ValidationError> errors; - Msix::MsixPackageManifestCache msixManifestsCache; + auto validationErrorLevel = treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error; + MsixManifestValidation msixManifestValidation(validationErrorLevel); for (const auto& installer : manifest.Installers) { // Installer msix or msixbundle if (installer.InstallerType == InstallerTypeEnum::Msix) { - auto installerErrors = ValidateMsixManifest(Msix::PackageVersion(manifest.Version), installer, msixManifestsCache, treatErrorAsWarning); + auto installerErrors = msixManifestValidation.Validate(manifest, installer); std::move(installerErrors.begin(), installerErrors.end(), std::inserter(errors, errors.end())); } } diff --git a/src/AppInstallerCommonCore/Manifest/MsixManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/MsixManifestValidation.cpp @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "AppInstallerLogging.h" +#include "winget/MsixManifestValidation.h" + +namespace AppInstaller::Manifest +{ + std::vector<ValidationError> MsixManifestValidation::Validate( + const Manifest &manifest, + const ManifestInstaller &installer) + { + std::vector<ValidationError> errors; + Msix::PackageVersion packageVersion(manifest.Version); + auto msixInfo = GetMsixInfo(installer.Url, errors); + if (msixInfo) + { + ValidateMsixManifestSignatureHash(msixInfo, installer.SignatureSha256, errors); + auto msixManifests = msixInfo->GetAppPackageManifests(); + auto installerMinOSVersion = GetManifestInstallerMinOSVersion(installer.MinOSVersion, errors); + for (const auto& msixManifest : msixManifests) + { + auto msixManifestIdentity = msixManifest.GetIdentity(); + ValidateMsixManifestPackageFamilyName(msixManifestIdentity.GetPackageFamilyName(), installer.PackageFamilyName, errors); + ValidateMsixManifestPackageVersion(msixManifestIdentity.GetVersion(), packageVersion, errors); + ValidateMsixManifestMinOSVersion(msixManifest.GetMinimumOSVersionForSupportedPlatforms(), installerMinOSVersion, installer.Url, errors); + } + } + + return errors; + } + + std::shared_ptr<Msix::MsixInfo> MsixManifestValidation::GetMsixInfo( + std::string installerUrl, + std::vector<ValidationError> &errors) + { + std::shared_ptr<Msix::MsixInfo> msixInfo; + try + { + // Cache Msix info for new installer url + auto findMsixInfo = m_msixInfoCache.find(installerUrl); + if (findMsixInfo == m_msixInfoCache.end()) + { + msixInfo = std::make_shared<Msix::MsixInfo>(installerUrl); + m_msixInfoCache.insert({ installerUrl, msixInfo }); + } + else + { + msixInfo = findMsixInfo->second; + } + + return msixInfo; + } + catch (...) + { + errors.emplace_back(ManifestError::InstallerFailedToProcess, "InstallerUrl", installerUrl); + } + + return nullptr; + } + + std::optional<Msix::OSVersion> MsixManifestValidation::GetManifestInstallerMinOSVersion( + std::string minOSVersion, + std::vector<ValidationError> &errors) + { + try + { + if (!minOSVersion.empty()) + { + return std::make_optional<Msix::OSVersion>(minOSVersion); + } + } + catch (const std::exception&) + { + errors.emplace_back(ManifestError::InvalidFieldValue, "MinimumOSVersion", minOSVersion); + } + return std::nullopt; + } + + void MsixManifestValidation::ValidateMsixManifestPackageFamilyName( + Utility::NormalizedString msixPackageFamilyName, + Utility::NormalizedString manifestPackageFamilyName, + std::vector<ValidationError> &errors) + { + if (!manifestPackageFamilyName.empty()) + { + if (manifestPackageFamilyName != msixPackageFamilyName) + { + errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "PackageFamilyName", msixPackageFamilyName); + } + } + else + { + errors.emplace_back(ManifestError::OptionalFieldMissing, "PackageFamilyName", msixPackageFamilyName, m_validationErrorLevel); + } + } + + void MsixManifestValidation::ValidateMsixManifestPackageVersion( + const Msix::PackageVersion &msixPackageVersion, + const Msix::PackageVersion &manifestPackageVersion, + std::vector<ValidationError> &errors) + { + if (msixPackageVersion != manifestPackageVersion) + { + errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "PackageVersion", msixPackageVersion.ToString()); + } + } + + void MsixManifestValidation::ValidateMsixManifestMinOSVersion( + const std::optional<Msix::OSVersion>& msixMinOSVersion, + const std::optional<Msix::OSVersion>& manifestMinOSVersion, + std::string installerUrl, + std::vector<ValidationError> &errors) + { + if (!msixMinOSVersion.has_value()) + { + errors.emplace_back(ManifestError::NoSupportedPlatforms, "InstallerUrl", installerUrl); + } + else if (manifestMinOSVersion.has_value()) + { + if (msixMinOSVersion.value() != manifestMinOSVersion.value()) + { + errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "MinimumOSVersion", msixMinOSVersion.value().ToString()); + } + } + else + { + errors.emplace_back( + ManifestError::OptionalFieldMissing, + "MinimumOSVersion", + msixMinOSVersion.value().ToString(), + m_validationErrorLevel); + } + } + + void MsixManifestValidation::ValidateMsixManifestSignatureHash( + const std::shared_ptr<Msix::MsixInfo> msixInfo, + const Utility::SHA256::HashBuffer& manifestSignatureHash, + std::vector<ValidationError>& errors) + { + try + { + if (!manifestSignatureHash.empty()) + { + auto msixSignatureHash = msixInfo->GetSignatureHash(); + if (msixSignatureHash != manifestSignatureHash) + { + auto msixSignatureHashString = Utility::SHA256::ConvertToString(msixSignatureHash); + errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "SignatureSha256", msixSignatureHashString); + } + } + } + catch (const wil::ResultException&) + { + errors.emplace_back(ManifestError::MsixSignatureHashFailed); + } + } +} diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp @@ -564,6 +564,12 @@ namespace AppInstaller::Msix return signatureContent; } + Utility::SHA256::HashBuffer MsixInfo::GetSignatureHash() + { + auto signature = GetSignature(); + return Utility::SHA256::ComputeHash(signature.data(), static_cast<uint32_t>(signature.size())); + } + std::wstring MsixInfo::GetPackageFullNameWide() { ComPtr<IAppxManifestPackageId> packageId; diff --git a/src/AppInstallerCommonCore/MsixManifest.cpp b/src/AppInstallerCommonCore/MsixManifest.cpp @@ -108,20 +108,4 @@ namespace AppInstaller::Msix return Other; } - - const std::vector<MsixPackageManifest>& MsixPackageManifestCache::GetAppPackageManifests(std::string url) - { - // If an installer url has already been processed, then use the cached result - auto installerIter = m_msixManifests.find(url); - if (installerIter != m_msixManifests.end()) - { - return installerIter->second; - } - - MsixInfo msixInfo(url); - - // Cache installer url result - m_msixManifests[url] = msixInfo.GetAppPackageManifests(); - return m_msixManifests[url]; - } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerMsixInfo.h b/src/AppInstallerCommonCore/Public/AppInstallerMsixInfo.h @@ -69,6 +69,9 @@ namespace AppInstaller::Msix // If skipP7xFileId is true, returns content of converted .p7s std::vector<byte> GetSignature(bool skipP7xFileId = false); + // Get the signature sha256 hash. + Utility::SHA256::HashBuffer GetSignatureHash(); + // Gets the package full name. std::wstring GetPackageFullNameWide(); std::string GetPackageFullName(); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -60,6 +60,7 @@ namespace AppInstaller::Manifest const char* const ArpVersionValidationInternalError = "Internal error while validating DisplayVersion against index."; const char* const ExceededNestedInstallerFilesLimit = "Only one entry for NestedInstallerFiles can be specified for non-portable InstallerTypes."; const char* const RelativeFilePathEscapesDirectory = "Relative file path must not point to a location outside of archive directory"; + const char* const MsixSignatureHashFailed = "Failed to calculate MSIX signature hash. Please verify that the input file is a valid, signed MSIX."; } struct ValidationError diff --git a/src/AppInstallerCommonCore/Public/winget/MsixManifest.h b/src/AppInstallerCommonCore/Public/winget/MsixManifest.h @@ -61,14 +61,4 @@ namespace AppInstaller::Msix private: Microsoft::WRL::ComPtr<IAppxManifestReader> m_manifestReader; }; - - // MSIX manifest cache - struct MsixPackageManifestCache - { - // Construct MSIX manifest or fetch it from cache - const std::vector<MsixPackageManifest>& GetAppPackageManifests(std::string url); - private: - // Map installer url to a vector of msix package manifests - std::map<std::string, std::vector<MsixPackageManifest>> m_msixManifests; - }; } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/winget/MsixManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/MsixManifestValidation.h @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "AppInstallerErrors.h" +#include "AppInstallerMsixInfo.h" +#include "winget/Manifest.h" +#include "winget/ManifestValidation.h" + +namespace AppInstaller::Manifest +{ + struct MsixManifestValidation + { + MsixManifestValidation(ValidationError::Level validationErrorLevel) : m_validationErrorLevel(validationErrorLevel) {} + + // Validate manifest for Msix packages and Msix bundles. + std::vector<ValidationError> Validate( + const Manifest &manifest, + const ManifestInstaller &installer); + private: + std::map<std::string, std::shared_ptr<Msix::MsixInfo>> m_msixInfoCache; + ValidationError::Level m_validationErrorLevel; + + // Get Msix info from installer url, or load it from cache. Return null + // pointer if failed to process installer url. + std::shared_ptr<Msix::MsixInfo> GetMsixInfo( + std::string installerUrl, + std::vector<ValidationError>& errors); + + // Get manifest installer minimum OS version or nullopt if failed to + // parse input. + std::optional<Msix::OSVersion> GetManifestInstallerMinOSVersion( + std::string minOSVersion, + std::vector<ValidationError>& errors); + + // Validate Msix package family name. + void ValidateMsixManifestPackageFamilyName( + Utility::NormalizedString msixPackageFamilyName, + Utility::NormalizedString manifestPackageFamilyName, + std::vector<ValidationError>& errors); + + // Validate Msix package version. + void ValidateMsixManifestPackageVersion( + const Msix::PackageVersion& msixPackageVersion, + const Msix::PackageVersion& manifestPackageVersion, + std::vector<ValidationError>& errors); + + // Validate Msix minimum OS version for supported platforms. + void ValidateMsixManifestMinOSVersion( + const std::optional<Msix::OSVersion>& msixMinOSVersion, + const std::optional<Msix::OSVersion>& manifestMinOSVersion, + std::string installerUrl, + std::vector<ValidationError>& errors); + + // Validate Msix signature hash. + void ValidateMsixManifestSignatureHash( + const std::shared_ptr<Msix::MsixInfo> msixInfo, + const Utility::SHA256::HashBuffer& manifestSignatureHash, + std::vector<ValidationError>& errors); + }; +}