Enforce hash requirements

doc/release-notes/1.31/require-rest-installer-hash.md

@@ -0,0 +1,5 @@
+## Bug Fixes
+
+### Minor Bug Fixes
+
+* Ignore REST installers missing an installer hash, except for Microsoft Store installers, and reject missing or incomplete hashes during installer verification.

src/AppInstallerCLICore/Workflows/DownloadFlow.cpp

@@ -543,10 +543,11 @@ namespace AppInstaller::CLI::Workflow
     {
         const auto& [expectedHash, downloadResult] = context.Get<Execution::Data::DownloadHashInfo>();
 
-        if (!std::equal(
+        if (downloadResult.Sha256Hash.empty() || !std::equal(
+            downloadResult.Sha256Hash.begin(),
+            downloadResult.Sha256Hash.end(),
             expectedHash.begin(),
-            expectedHash.end(),
-            downloadResult.Sha256Hash.begin()))
+            expectedHash.end()))
         {
             bool overrideHashMismatch = context.Args.Contains(Execution::Args::Type::HashOverride);
 

src/AppInstallerCLITests/InstallFlow.cpp

@@ -39,7 +39,8 @@ void OverrideForDirectMsi(TestContext& context)
 
     context.Override({ DownloadInstallerFile, [](TestContext& context)
     {
-        context.Add<Data::DownloadHashInfo>({ {}, {} });
+        const auto& fileHash = GetTestExeInstallerHash();
+        context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
         // We don't have an msi installer for tests, but we won't execute it anyway
         context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
     } });
@@ -97,7 +98,8 @@ TEST_CASE("InstallFlow_RenameFromEncodedUrl", "[InstallFlow][workflow]")
     OverrideForCheckExistingInstaller(context);
     context.Override({ DownloadInstallerFile, [](TestContext& context)
     {
-        context.Add<Data::DownloadHashInfo>({ {}, {} });
+        const auto& fileHash = GetTestExeInstallerHash();
+        context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
         auto installerPath = std::filesystem::temp_directory_path();
         installerPath /= "EncodedUrlTest.exe";
         std::filesystem::copy(TestDataFile("AppInstallerTestExeInstaller.exe"), installerPath, std::filesystem::copy_options::overwrite_existing);
@@ -129,7 +131,8 @@ TEST_CASE("InstallFlow_RenameFromInvalidFileCharacterUrl", "[InstallFlow][workfl
     OverrideForCheckExistingInstaller(context);
     context.Override({ DownloadInstallerFile, [](TestContext& context)
     {
-        context.Add<Data::DownloadHashInfo>({ {}, {} });
+        const auto& fileHash = GetTestExeInstallerHash();
+        context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
         auto installerPath = std::filesystem::temp_directory_path();
         installerPath /= "InvalidFileCharacterUrlTest.exe";
         std::filesystem::copy(TestDataFile("AppInstallerTestExeInstaller.exe"), installerPath, std::filesystem::copy_options::overwrite_existing);

src/AppInstallerCLITests/RestInterface_1_0.cpp

@@ -6,6 +6,7 @@
 #include <Rest/RestSource.h>
 #include <Rest/Schema/1_0/Interface.h>
 #include <Rest/Schema/1_4/Interface.h>
+#include <Rest/Schema/1_28/Json/ManifestDeserializer.h>
 #include <Rest/Schema/IRestClient.h>
 #include <AppInstallerVersions.h>
 #include <AppInstallerErrors.h>
@@ -2270,6 +2271,68 @@ TEST_CASE("GetManifests_BadResponse_SuccessCode", "[RestSource][Interface_1_0]")
     REQUIRE_THROWS_HR(v1.GetManifests("Foo.Bar"), APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_DATA);
 }
 
+TEST_CASE("GetManifests_BadResponse_InstallerHash", "[RestSource][Interface_1_0]")
+{
+    auto response = web::json::value::parse(GetGoodManifest_RequiredFields());
+    auto& installers = response[L"Data"][L"Versions"][0][L"Installers"];
+    auto& installer = installers[0];
+
+    SECTION("Missing hash")
+    {
+        installer.as_object().erase(L"InstallerSha256");
+    }
+    SECTION("Empty hash")
+    {
+        installer[L"InstallerSha256"] = web::json::value::string(L"");
+    }
+    SECTION("Null hash")
+    {
+        installer[L"InstallerSha256"] = web::json::value::null();
+    }
+    SECTION("Non-string hash")
+    {
+        installer[L"InstallerSha256"] = web::json::value::number(42);
+    }
+    HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, response.serialize()) };
+    Interface v1{ TestRestUriString, std::move(helper) };
+    REQUIRE_THROWS_HR(v1.GetManifests("Foo.Bar"), APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_DATA);
+    REQUIRE_THROWS_HR(V1_28::Json::ManifestDeserializer{}.Deserialize(response), APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_DATA);
+}
+
+TEST_CASE("GetManifests_SkipsInstallerWithoutHash", "[RestSource][Interface_1_0][Interface_1_28]")
+{
+    auto response = web::json::value::parse(GetGoodManifest_RequiredFields());
+    auto& installers = response[L"Data"][L"Versions"][0][L"Installers"];
+    auto validInstaller = installers[0];
+    installers[0].as_object().erase(L"InstallerSha256");
+    installers[1] = std::move(validInstaller);
+
+    HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, response.serialize()) };
+    Interface v1{ TestRestUriString, std::move(helper) };
+    for (const auto& manifests : { v1.GetManifests("Foo.Bar"), V1_28::Json::ManifestDeserializer{}.Deserialize(response) })
+    {
+        REQUIRE(manifests.size() == 1);
+        REQUIRE(manifests[0].Installers.size() == 1);
+        REQUIRE(manifests[0].Installers[0].Sha256 == SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df84b6"));
+    }
+}
+
+TEST_CASE("ManifestDeserializer_MSStoreWithoutInstallerHash", "[RestSource][Interface_1_28]")
+{
+    auto response = web::json::value::parse(GetGoodManifest_RequiredFields());
+    auto& installer = response[L"Data"][L"Versions"][0][L"Installers"][0];
+    installer[L"InstallerType"] = web::json::value::string(L"msstore");
+    installer[L"MSStoreProductIdentifier"] = web::json::value::string(L"9nblggh4nns1");
+    installer.as_object().erase(L"InstallerUrl");
+    installer.as_object().erase(L"InstallerSha256");
+
+    auto manifests = V1_28::Json::ManifestDeserializer{}.Deserialize(response);
+    REQUIRE(manifests.size() == 1);
+    REQUIRE(manifests[0].Installers.size() == 1);
+    REQUIRE(manifests[0].Installers[0].BaseInstallerType == InstallerTypeEnum::MSStore);
+    REQUIRE(manifests[0].Installers[0].Sha256.empty());
+}
+
 TEST_CASE("GetManifests_NotFoundCode", "[RestSource][Interface_1_0]")
 {
     HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::NotFound) };
@@ -2296,6 +2359,7 @@ TEST_CASE("GetManifests_GoodResponse_UnknownInstaller", "[RestSource][Interface_
                     "Installers": [
                         {
                             "Architecture": "x64",
+                            "InstallerSha256": "011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df84b6",
                             "InstallerType": "msstore",
                             "MSStoreProductIdentifier": "9nblggh4nns1"
                         }

src/AppInstallerCLITests/WorkFlow.cpp

@@ -45,7 +45,8 @@ TEST_CASE("VerifyInstallerTrustLevelAndUpdateInstallerFileMotw", "[DownloadInsta
     std::ostringstream updateMotwOutput;
     TestContext context{ updateMotwOutput, std::cin };
     auto previousThreadGlobals = context.SetForCurrentThread();
-    context.Add<Data::DownloadHashInfo>({ {}, {} });
+    auto installerHash = SHA256::ComputeHashFromFile(testInstallerPath);
+    context.Add<Data::DownloadHashInfo>({ installerHash, DownloadResult{ installerHash } });
     context.Add<Data::InstallerPath>(testInstallerPath);
     auto packageVersion = std::make_shared<TestPackageVersion>(Manifest{});
     auto testSource = std::make_shared<TestSource>();
@@ -69,6 +70,55 @@ TEST_CASE("VerifyInstallerTrustLevelAndUpdateInstallerFileMotw", "[DownloadInsta
     INFO(updateMotwOutput.str());
 }
 
+TEST_CASE("VerifyInstallerHash_RejectsMissingOrMismatchedHash", "[DownloadInstaller][workflow]")
+{
+    auto expectedHash = SHA256::ComputeHash("test"sv);
+    DownloadResult downloadResult{ expectedHash };
+    downloadResult.SizeInBytes = 4;
+
+    SECTION("Missing expected hash")
+    {
+        expectedHash.clear();
+    }
+    SECTION("Truncated expected hash")
+    {
+        expectedHash.pop_back();
+    }
+    SECTION("Overlong expected hash")
+    {
+        expectedHash.push_back(0);
+    }
+    SECTION("Mismatched hash")
+    {
+        expectedHash[0] ^= 0xFF;
+    }
+    SECTION("Missing downloaded hash")
+    {
+        downloadResult.Sha256Hash.clear();
+    }
+    SECTION("Truncated downloaded hash")
+    {
+        downloadResult.Sha256Hash.pop_back();
+    }
+    SECTION("Both hashes missing")
+    {
+        expectedHash.clear();
+        downloadResult.Sha256Hash.clear();
+    }
+
+    std::ostringstream output;
+    TestContext context{ output, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    context.Add<Data::Manifest>(Manifest{});
+    context.Add<Data::DownloadHashInfo>({ expectedHash, downloadResult });
+
+    context << VerifyInstallerHash;
+
+    REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_INSTALLER_HASH_MISMATCH);
+    REQUIRE_FALSE(WI_IsFlagSet(context.GetFlags(), ContextFlag::InstallerHashMatched));
+    REQUIRE_FALSE(WI_IsFlagSet(context.GetFlags(), ContextFlag::InstallerTrusted));
+}
+
 TEST_CASE("ValidateCommand_Dependencies", "[workflow][dependencies]")
 {
     std::ostringstream validateOutput;

src/AppInstallerCLITests/WorkflowCommon.cpp

@@ -552,13 +552,20 @@ namespace TestCommon
         } });
     }
 
+    const SHA256::HashBuffer& GetTestExeInstallerHash()
+    {
+        static const auto hash = SHA256::ComputeHashFromFile(TestDataFile("AppInstallerTestExeInstaller.exe"));
+        return hash;
+    }
+
     void OverrideForShellExecute(TestContext& context, int expectedUseCount)
     {
         OverrideForCheckExistingInstaller(context);
 
         context.Override({ DownloadInstallerFile, [](TestContext& context)
         {
-            context.Add<Data::DownloadHashInfo>({ {}, {} });
+            const auto& fileHash = GetTestExeInstallerHash();
+            context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
             context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
         }, expectedUseCount });
 
@@ -573,7 +580,8 @@ namespace TestCommon
     {
         context.Override({ DownloadInstallerFile, [&installationLog](TestContext& context)
         {
-            context.Add<Data::DownloadHashInfo>({ {}, {} });
+            const auto& fileHash = GetTestExeInstallerHash();
+            context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
             context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
 
             auto dependency = Dependency(DependencyType::Package, context.Get<Execution::Data::Manifest>().Id, context.Get<Execution::Data::Manifest>().Version);
@@ -602,7 +610,8 @@ namespace TestCommon
     {
         context.Override({ DownloadInstallerFile, [](TestContext& context)
         {
-            context.Add<Data::DownloadHashInfo>({ {}, {} });
+            const auto& fileHash = GetTestExeInstallerHash();
+            context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
             context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
         } });
 
@@ -624,8 +633,7 @@ namespace TestCommon
             std::filesystem::copy(installerPath, tempInstallerPath, std::filesystem::copy_options::overwrite_existing);
             context.Add<Data::InstallerPath>(tempInstallerPath);
 
-            std::ifstream inStream{ tempInstallerPath, std::ifstream::binary };
-            SHA256::HashBuffer fileHash = SHA256::ComputeHash(inStream);
+            const auto& fileHash = GetTestExeInstallerHash();
             context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
         } });
 
@@ -732,7 +740,8 @@ namespace TestCommon
             std::ofstream file(installerPath, std::ofstream::out | std::ofstream::trunc);
             file << installer.Url;
             file.close();
-            context.Add<Data::DownloadHashInfo>({ {}, {} });
+            auto fileHash = SHA256::ComputeHashFromFile(installerPath);
+            context.Add<Data::DownloadHashInfo>({ fileHash, DownloadResult{ fileHash } });
         } });
     }
 }

src/AppInstallerCLITests/WorkflowCommon.h

@@ -3,6 +3,7 @@
 #include "pch.h"
 #include "TestCommon.h"
 #include "TestSource.h"
+#include <AppInstallerSHA256.h>
 #include <winget/ManifestCommon.h>
 #include <ExecutionContext.h>
 #include <Workflows/WorkflowBase.h>
@@ -116,6 +117,8 @@ namespace TestCommon
 
     void OverrideForCheckExistingInstaller(TestContext& context);
 
+    const AppInstaller::Utility::SHA256::HashBuffer& GetTestExeInstallerHash();
+
     void OverrideForShellExecute(TestContext& context, int expectedUseCount = -1);
 
     void OverrideForShellExecute(TestContext& context, std::vector<AppInstaller::Manifest::Dependency>& installationLog);

src/AppInstallerCommonCore/AppInstallerTelemetry.cpp

@@ -599,9 +599,9 @@ namespace AppInstaller::Logging
 
         AICLI_LOG(CLI, Error,
             << "Package hash verification failed. SHA256 in manifest ["
-            << Utility::SHA256::ConvertToString(expected)
+            << Utility::ConvertToHexString(expected)
             << "] does not match download ["
-            << Utility::SHA256::ConvertToString(actual)
+            << Utility::ConvertToHexString(actual)
             << "] with file size [" << downloadSizeInBytes << "] and content type [" << actualContentType << "]");
     }
 

src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp

@@ -964,6 +964,11 @@ namespace AppInstaller::Manifest
             installerType == InstallerTypeEnum::Exe;
     }
 
+    bool DoesInstallerTypeRequireHash(InstallerTypeEnum installerType)
+    {
+        return installerType != InstallerTypeEnum::MSStore;
+    }
+
     bool DoesInstallerTypeUseMsiProperties(InstallerTypeEnum installerType)
     {
         return

src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp

@@ -360,7 +360,7 @@ namespace AppInstaller::Manifest
                 {
                     resultErrors.emplace_back(ManifestError::RequiredFieldMissing, "InstallerUrl");
                 }
-                if (installer.Sha256.empty())
+                if (DoesInstallerTypeRequireHash(installer.EffectiveInstallerType()) && installer.Sha256.empty())
                 {
                     resultErrors.emplace_back(ManifestError::RequiredFieldMissing, "InstallerSha256");
                 }

src/AppInstallerCommonCore/Public/winget/ManifestCommon.h

@@ -503,6 +503,9 @@ namespace AppInstaller::Manifest
     // Gets a value indicating whether the given installer requires RepairBehavior for repair.
     bool DoesInstallerTypeRequireRepairBehaviorForRepair(InstallerTypeEnum installerType);
 
+    // Gets a value indicating whether the given installer type requires an installer hash.
+    bool DoesInstallerTypeRequireHash(InstallerTypeEnum installerType);
+
     // Gets a value indicating whether the given installer type uses MSI properties in its command line.
     bool DoesInstallerTypeUseMsiProperties(InstallerTypeEnum installerType);
 

src/AppInstallerRepositoryCore/Rest/Schema/1_0/Json/ManifestDeserializer_1_0.cpp

@@ -324,12 +324,6 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0::Json
 
         installer.Url = JSON::GetRawStringValueFromJsonNode(installerJsonObject, JSON::GetUtilityString(InstallerUrl)).value_or("");
 
-        std::optional<std::string> sha256 = JSON::GetRawStringValueFromJsonNode(installerJsonObject, JSON::GetUtilityString(InstallerSha256));
-        if (JSON::IsValidNonEmptyStringValue(sha256))
-        {
-            installer.Sha256 = Utility::SHA256::ConvertToBytes(sha256.value());
-        }
-
         std::optional<std::string> arch = JSON::GetRawStringValueFromJsonNode(installerJsonObject, JSON::GetUtilityString(Architecture));
         if (!JSON::IsValidNonEmptyStringValue(arch))
         {
@@ -345,6 +339,18 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0::Json
             return {};
         }
         installer.BaseInstallerType = ConvertToInstallerType(installerType.value());
+
+        std::optional<std::string> sha256 = JSON::GetRawStringValueFromJsonNode(installerJsonObject, JSON::GetUtilityString(InstallerSha256));
+        if (JSON::IsValidNonEmptyStringValue(sha256))
+        {
+            installer.Sha256 = Utility::SHA256::ConvertToBytes(sha256.value());
+        }
+        else if (DoesInstallerTypeRequireHash(installer.BaseInstallerType))
+        {
+            AICLI_LOG(Repo, Error, << "Missing installer SHA256.");
+            return {};
+        }
+
         installer.Locale = JSON::GetRawStringValueFromJsonNode(installerJsonObject, JSON::GetUtilityString(InstallerLocale)).value_or("");
 
         // platform