Merge pull request #13647 from pkerling/addon-ssl

Verify add-on zip authenticity

14 files changed, 365 insertions, 123 deletions

diff --git a/addons/repository.xbmc.org/addon.xml b/addons/repository.xbmc.org/addon.xml
index fc1ad73f5e..4d6552c757 100644
--- a/addons/repository.xbmc.org/addon.xml
+++ b/addons/repository.xbmc.org/addon.xml
@@ -8,9 +8,10 @@
   </requires>
 	<extension point="xbmc.addon.repository">
 		<info>http://mirrors.kodi.tv/addons/leia/addons.xml.gz</info>
-		<checksum>http://mirrors.kodi.tv/addons/leia/addons.xml.gz.md5</checksum>
-		<datadir>http://mirrors.kodi.tv/addons/leia</datadir>
-		<hashes>true</hashes>
+		<checksum verify="sha256">http://mirrors.kodi.tv/addons/leia/addons.xml.gz?sha256</checksum>
+		<datadir>https://mirrors.kodi.tv/addons/leia</datadir>
+		<artdir>http://mirrors.kodi.tv/addons/leia</artdir>
+		<hashes>sha256</hashes>
 	</extension>
 	<extension point="xbmc.addon.metadata">
 		<summary lang="af_ZA">Installeer Byvoegsels vanaf Kodi.tv</summary>
diff --git a/xbmc/addons/AddonInstaller.cpp b/xbmc/addons/AddonInstaller.cpp
index c851ce7818..e8dfefd0f9 100644
--- a/xbmc/addons/AddonInstaller.cpp
+++ b/xbmc/addons/AddonInstaller.cpp
@@ -57,6 +57,7 @@ using namespace ADDON;
 using namespace KODI::MESSAGING;
 
 using KODI::MESSAGING::HELPERS::DialogResponse;
+using KODI::UTILITY::TypedDigest;
 
 struct find_map : public std::binary_function<CAddonInstaller::JobMap::value_type, unsigned int, bool>
 {
@@ -199,11 +200,10 @@ bool CAddonInstaller::InstallOrUpdate(const std::string &addonID, bool backgroun
 {
   AddonPtr addon;
   RepositoryPtr repo;
-  std::string hash;
-  if (!CAddonInstallJob::GetAddonWithHash(addonID, repo, addon, hash))
+  if (!CAddonInstallJob::GetAddon(addonID, repo, addon))
     return false;
 
-  return DoInstall(addon, repo, hash, background, modal);
+  return DoInstall(addon, repo, background, modal);
 }
 
 void CAddonInstaller::Install(const std::string& addonId, const AddonVersion& version, const std::string& repoId)
@@ -221,22 +221,17 @@ void CAddonInstaller::Install(const std::string& addonId, const AddonVersion& ve
   if (!CServiceBroker::GetAddonMgr().GetAddon(repoId, repo, ADDON_REPOSITORY))
     return;
 
-  std::string hash;
-  if (!std::static_pointer_cast<CRepository>(repo)->GetAddonHash(addon, hash))
-    return;
-  DoInstall(addon, std::static_pointer_cast<CRepository>(repo), hash, true, false);
+  DoInstall(addon, std::static_pointer_cast<CRepository>(repo), true, false);
 }
 
-bool CAddonInstaller::DoInstall(const AddonPtr &addon, const RepositoryPtr& repo,
-    const std::string &hash /* = "" */, bool background /* = true */, bool modal /* = false */,
-    bool autoUpdate /* = false*/)
+bool CAddonInstaller::DoInstall(const AddonPtr &addon, const RepositoryPtr& repo, bool background /* = true */, bool modal /* = false */, bool autoUpdate /* = false*/)
 {
   // check whether we already have the addon installing
   CSingleLock lock(m_critSection);
   if (m_downloadJobs.find(addon->ID()) != m_downloadJobs.end())
     return false;
 
-  CAddonInstallJob* installJob = new CAddonInstallJob(addon, repo, hash, autoUpdate);
+  CAddonInstallJob* installJob = new CAddonInstallJob(addon, repo, autoUpdate);
   if (background)
   {
     // Workaround: because CAddonInstallJob is blocking waiting for other jobs, it needs to be run
@@ -434,9 +429,8 @@ void CAddonInstaller::InstallUpdates()
     {
       AddonPtr toInstall;
       RepositoryPtr repo;
-      std::string hash;
-      if (CAddonInstallJob::GetAddonWithHash(addon->ID(), repo, toInstall, hash))
-        DoInstall(toInstall, repo, hash, true, false, true);
+      if (CAddonInstallJob::GetAddon(addon->ID(), repo, toInstall))
+        DoInstall(toInstall, repo, true, false, true);
     }
   }
 }
@@ -474,19 +468,17 @@ int64_t CAddonInstaller::EnumeratePackageFolder(std::map<std::string,CFileItemLi
   return size;
 }
 
-CAddonInstallJob::CAddonInstallJob(const AddonPtr &addon, const AddonPtr &repo,
-    const std::string &hash, bool isAutoUpdate)
+CAddonInstallJob::CAddonInstallJob(const AddonPtr &addon, const RepositoryPtr &repo, bool isAutoUpdate)
   : m_addon(addon),
     m_repo(repo),
-    m_hash(hash),
     m_isAutoUpdate(isAutoUpdate)
 {
   AddonPtr dummy;
   m_isUpdate = CServiceBroker::GetAddonMgr().GetAddon(addon->ID(), dummy, ADDON_UNKNOWN, false);
 }
 
-bool CAddonInstallJob::GetAddonWithHash(const std::string& addonID, RepositoryPtr& repo,
-    ADDON::AddonPtr& addon, std::string& hash)
+bool CAddonInstallJob::GetAddon(const std::string& addonID, RepositoryPtr& repo,
+    ADDON::AddonPtr& addon)
 {
   if (!CServiceBroker::GetAddonMgr().FindInstallableById(addonID, addon))
     return false;
@@ -496,7 +488,8 @@ bool CAddonInstallJob::GetAddonWithHash(const std::string& addonID, RepositoryPt
     return false;
 
   repo = std::static_pointer_cast<CRepository>(tmp);
-  return repo->GetAddonHash(addon, hash);
+
+  return true;
 }
 
 bool CAddonInstallJob::DoWork()
@@ -524,12 +517,27 @@ bool CAddonInstallJob::DoWork()
     std::string dest = "special://home/addons/packages/";
     std::string package = URIUtils::AddFileToFolder("special://home/addons/packages/",
                                                     URIUtils::GetFileName(m_addon->Path()));
-    if (URIUtils::HasSlashAtEnd(m_addon->Path()))
+    if (!m_repo && URIUtils::HasSlashAtEnd(m_addon->Path()))
     { // passed in a folder - all we need do is copy it across
       installFrom = m_addon->Path();
     }
     else
     {
+      std::string path{m_addon->Path()};
+      TypedDigest hash;
+      if (m_repo)
+      {
+        CRepository::ResolveResult resolvedAddon = m_repo->ResolvePathAndHash(m_addon);
+        path = resolvedAddon.location;
+        hash = resolvedAddon.digest;
+        if (path.empty())
+        {
+          CLog::Log(LOGERROR, "CAddonInstallJob[%s]: failed to resolve addon install source path", m_addon->ID().c_str());
+          ReportInstallError(m_addon->ID(), m_addon->ID());
+          return false;
+        }
+      }
+
       CAddonDatabase db;
       if (!db.Open())
       {
@@ -538,13 +546,16 @@ bool CAddonInstallJob::DoWork()
         return false;
       }
 
-      std::string md5;
       // check that we don't already have a valid copy
-      if (!m_hash.empty() && CFile::Exists(package))
+      if (!hash.Empty())
       {
-        if (db.GetPackageHash(m_addon->ID(), package, md5) && m_hash != md5)
+        std::string hashExisting;
+        if (db.GetPackageHash(m_addon->ID(), package, hashExisting) && hash.value != hashExisting)
         {
           db.RemovePackage(package);
+        }
+        if (CFile::Exists(package))
+        {
           CFile::Delete(package);
         }
       }
@@ -552,7 +563,6 @@ bool CAddonInstallJob::DoWork()
       // zip passed in - download + extract
       if (!CFile::Exists(package))
       {
-        std::string path(m_addon->Path());
         if (!DownloadPackage(path, dest))
         {
           CFile::Delete(package);
@@ -565,20 +575,20 @@ bool CAddonInstallJob::DoWork()
 
       // at this point we have the package - check that it is valid
       SetText(g_localizeStrings.Get(24077));
-      if (!m_hash.empty())
+      if (!hash.Empty())
       {
-        md5 = CUtil::GetFileDigest(package, KODI::UTILITY::CDigest::Type::MD5);
-        if (!StringUtils::EqualsNoCase(md5, m_hash))
+        TypedDigest actualHash{hash.type, CUtil::GetFileDigest(package, hash.type)};
+        if (hash != actualHash)
         {
           CFile::Delete(package);
 
-          CLog::Log(LOGERROR, "CAddonInstallJob[%s]: MD5 mismatch after download. Expected %s, was %s",
-              m_addon->ID().c_str(), m_hash.c_str(), md5.c_str());
+          CLog::Log(LOGERROR, "CAddonInstallJob[{}]: Hash mismatch after download. Expected {}, was {}",
+              m_addon->ID(), hash.value, actualHash.value);
           ReportInstallError(m_addon->ID(), URIUtils::GetFileName(package));
           return false;
         }
 
-        db.AddPackage(m_addon->ID(), package, md5);
+        db.AddPackage(m_addon->ID(), package, hash.value);
       }
 
       // check if the archive is valid
@@ -702,7 +712,7 @@ bool CAddonInstallJob::DoFileOperation(FileAction action, CFileItemList &items,
   return result;
 }
 
-bool CAddonInstallJob::Install(const std::string &installFrom, const AddonPtr& repo)
+bool CAddonInstallJob::Install(const std::string &installFrom, const RepositoryPtr& repo)
 {
   SetText(g_localizeStrings.Get(24079));
   auto deps = m_addon->GetDependencies();
@@ -745,15 +755,14 @@ bool CAddonInstallJob::Install(const std::string &installFrom, const AddonPtr& r
         {
           RepositoryPtr repoForDep;
           AddonPtr addon;
-          std::string hash;
-          if (!CAddonInstallJob::GetAddonWithHash(addonID, repoForDep, addon, hash))
+          if (!CAddonInstallJob::GetAddon(addonID, repoForDep, addon))
           {
             CLog::Log(LOGERROR, "CAddonInstallJob[%s]: failed to find dependency %s", m_addon->ID().c_str(), addonID.c_str());
             ReportInstallError(m_addon->ID(), m_addon->ID(), g_localizeStrings.Get(24085));
             return false;
           }
 
-          CAddonInstallJob dependencyJob(addon, repoForDep, hash, false);
+          CAddonInstallJob dependencyJob(addon, repoForDep, false);
 
           // pass our progress indicators to the temporary job and don't allow it to
           // show progress or information updates (no progress, title or text changes)
diff --git a/xbmc/addons/AddonInstaller.h b/xbmc/addons/AddonInstaller.h
index e98237e0b2..157f006aab 100644
--- a/xbmc/addons/AddonInstaller.h
+++ b/xbmc/addons/AddonInstaller.h
@@ -125,12 +125,11 @@ private:
   /*! \brief Install an addon from a repository or zip
    \param addon the AddonPtr describing the addon
    \param repo the repository to install addon from
-   \param hash the hash to verify the install. Defaults to "".
    \param background whether to install in the background or not. Defaults to true.
    \return true on successful install, false on failure.
    */
   bool DoInstall(const ADDON::AddonPtr &addon, const ADDON::RepositoryPtr &repo,
-      const std::string &hash = "", bool background = true, bool modal = false, bool autoUpdate = false);
+      bool background = true, bool modal = false, bool autoUpdate = false);
 
   /*! \brief Check whether dependencies of an addon exist or are installable.
    Iterates through the addon's dependencies, checking they're installed or installable.
@@ -154,8 +153,7 @@ private:
 class CAddonInstallJob : public CFileOperationJob
 {
 public:
-  CAddonInstallJob(const ADDON::AddonPtr& addon, const ADDON::AddonPtr& repo,
-      const std::string& hash, bool isAutoUpdate);
+  CAddonInstallJob(const ADDON::AddonPtr& addon, const ADDON::RepositoryPtr& repo, bool isAutoUpdate);
 
   bool DoWork() override;
 
@@ -166,13 +164,12 @@ public:
    *  \param hash Hash of the add-on
    *  \return True if the add-on and its hash were found, false otherwise.
    */
-  static bool GetAddonWithHash(const std::string& addonID, ADDON::RepositoryPtr& repo,
-      ADDON::AddonPtr& addon, std::string& hash);
+  static bool GetAddon(const std::string& addonID, ADDON::RepositoryPtr& repo, ADDON::AddonPtr& addon);
 
 private:
   void OnPreInstall();
   void OnPostInstall();
-  bool Install(const std::string &installFrom, const ADDON::AddonPtr& repo = ADDON::AddonPtr());
+  bool Install(const std::string &installFrom, const ADDON::RepositoryPtr& repo = ADDON::RepositoryPtr());
   bool DownloadPackage(const std::string &path, const std::string &dest);
 
   bool DoFileOperation(FileAction action, CFileItemList &items, const std::string &file, bool useSameJob = true);
@@ -185,8 +182,7 @@ private:
   void ReportInstallError(const std::string& addonID, const std::string& fileName, const std::string& message = "");
 
   ADDON::AddonPtr m_addon;
-  ADDON::AddonPtr m_repo;
-  std::string m_hash;
+  ADDON::RepositoryPtr m_repo;
   bool m_isUpdate;
   bool m_isAutoUpdate;
 };
diff --git a/xbmc/addons/AddonManager.cpp b/xbmc/addons/AddonManager.cpp
index 796a9239df..3b7ae1989a 100644
--- a/xbmc/addons/AddonManager.cpp
+++ b/xbmc/addons/AddonManager.cpp
@@ -42,6 +42,11 @@ namespace ADDON
 void cp_fatalErrorHandler(const char *msg);
 void cp_logger(cp_log_severity_t level, const char *msg, const char *apid, void *user_data);
 
+namespace {
+// Note that all of these characters are url-safe
+const std::string VALID_ADDON_IDENTIFIER_CHARACTERS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_@!$";
+}
+
 /**********************************************************
  * CAddonMgr
  *
@@ -74,11 +79,20 @@ AddonPtr CAddonMgr::Factory(const cp_plugin_info_t* plugin, TYPE type)
   return nullptr;
 }
 
-bool CAddonMgr::Factory(const cp_plugin_info_t* plugin, TYPE type, CAddonBuilder& builder, bool ignoreExtensions/* = false*/)
+bool CAddonMgr::Factory(const cp_plugin_info_t* plugin, TYPE type, CAddonBuilder& builder, bool ignoreExtensions/* = false*/, std::string assetBasePath)
 {
   if (!plugin || !plugin->identifier)
     return false;
 
+  // Check addon identifier for forbidden characters
+  // The identifier is used e.g. in URLs so we shouldn't allow just
+  // any character to go through.
+  if (std::string{plugin->identifier}.find_first_not_of(VALID_ADDON_IDENTIFIER_CHARACTERS) != std::string::npos)
+  {
+    CLog::Log(LOGERROR, "Plugin identifier {} is invalid", plugin->identifier);
+    return false;
+  }
+
   if (!PlatformSupportsAddon(plugin))
     return false;
 
@@ -101,11 +115,17 @@ bool CAddonMgr::Factory(const cp_plugin_info_t* plugin, TYPE type, CAddonBuilder
     }
   }
 
-  FillCpluffMetadata(plugin, builder);
+  if (assetBasePath.empty() && plugin->plugin_path)
+  {
+    // Default for add-on information not loaded from repository
+    assetBasePath = plugin->plugin_path;
+  }
+
+  FillCpluffMetadata(plugin, builder, assetBasePath);
   return true;
 }
 
-void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder& builder)
+void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder& builder, std::string const& assetBasePath)
 {
   builder.SetId(plugin->identifier);
 
@@ -142,15 +162,15 @@ void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder
   if (!metadata)
     metadata = CServiceBroker::GetAddonMgr().GetExtension(plugin, "kodi.addon.metadata");
 
-  if (plugin->plugin_path && strcmp(plugin->plugin_path, "") != 0)
+  if (!assetBasePath.empty())
   {
     //backwards compatibility
     std::string icon = metadata && CServiceBroker::GetAddonMgr().GetExtValue(metadata->configuration, "noicon") == "true" ? "" : "icon.png";
     std::string fanart = metadata && CServiceBroker::GetAddonMgr().GetExtValue(metadata->configuration, "nofanart") == "true" ? "" : "fanart.jpg";
     if (!icon.empty())
-      builder.SetIcon(URIUtils::AddFileToFolder(plugin->plugin_path, icon));
+      builder.SetIcon(URIUtils::AddFileToFolder(assetBasePath, icon));
     if (!fanart.empty())
-      builder.SetArt("fanart", URIUtils::AddFileToFolder(plugin->plugin_path, fanart));
+      builder.SetArt("fanart", URIUtils::AddFileToFolder(assetBasePath, fanart));
   }
 
   if (metadata)
@@ -172,7 +192,7 @@ void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder
 
     builder.SetBroken(CServiceBroker::GetAddonMgr().GetExtValue(metadata->configuration, "broken"));
 
-    if (plugin->plugin_path && strcmp(plugin->plugin_path, "") != 0)
+    if (!assetBasePath.empty())
     {
       auto assets = CServiceBroker::GetAddonMgr().GetExtElement(metadata->configuration, "assets");
       if (assets)
@@ -181,7 +201,7 @@ void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder
         builder.SetArt("fanart", "");
         std::string icon = CServiceBroker::GetAddonMgr().GetExtValue(assets, "icon");
         if (!icon.empty())
-          icon = URIUtils::AddFileToFolder(plugin->plugin_path, icon);
+          icon = URIUtils::AddFileToFolder(assetBasePath, icon);
         builder.SetIcon(icon);
 
         std::map<std::string, std::string> art;
@@ -191,7 +211,7 @@ void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder
           auto value = CServiceBroker::GetAddonMgr().GetExtValue(assets, type.c_str());
           if (!value.empty())
           {
-            value = URIUtils::AddFileToFolder(plugin->plugin_path, value);
+            value = URIUtils::AddFileToFolder(assetBasePath, value);
             builder.SetArt(type, value);
           }
         }
@@ -203,7 +223,7 @@ void CAddonMgr::FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder
           for (const auto& elem : elements)
           {
             if (elem->value && strcmp(elem->value, "") != 0)
-              screenshots.emplace_back(URIUtils::AddFileToFolder(plugin->plugin_path, elem->value));
+              screenshots.emplace_back(URIUtils::AddFileToFolder(assetBasePath, elem->value));
           }
         }
         builder.SetScreenshots(std::move(screenshots));
@@ -1175,15 +1195,15 @@ bool CAddonMgr::AddonsFromRepoXML(const CRepository::DirInfo& repo, const std::s
     if (info)
     {
       CAddonBuilder builder;
-      auto basePath = URIUtils::AddFileToFolder(repo.datadir, std::string(info->identifier));
+      auto basePath = URIUtils::AddFileToFolder(repo.datadir, info->identifier);
       info->plugin_path = static_cast<char*>(malloc(basePath.length() + 1));
       strncpy(info->plugin_path, basePath.c_str(), basePath.length());
       info->plugin_path[basePath.length()] = '\0';
 
-      if (Factory(info, ADDON_UNKNOWN, builder))
+      if (Factory(info, ADDON_UNKNOWN, builder, false, URIUtils::AddFileToFolder(repo.artdir, info->identifier)))
       {
-        builder.SetPath(URIUtils::AddFileToFolder(repo.datadir, StringUtils::Format("%s/%s-%s.zip",
-            info->identifier, info->identifier, builder.GetVersion().asString().c_str())));
+        builder.SetPath(URIUtils::AddFileToFolder(repo.datadir, info->identifier,
+          StringUtils::Format("{}-{}.zip", info->identifier, builder.GetVersion().asString())));
         auto addon = builder.Build();
         if (addon)
           addons.push_back(std::move(addon));
diff --git a/xbmc/addons/AddonManager.h b/xbmc/addons/AddonManager.h
index a581f0ee87..d092f194e3 100644
--- a/xbmc/addons/AddonManager.h
+++ b/xbmc/addons/AddonManager.h
@@ -287,8 +287,8 @@ namespace ADDON
     std::vector<DependencyInfo> GetDepsRecursive(const std::string& id);
 
     static AddonPtr Factory(const cp_plugin_info_t* plugin, TYPE type);
-    static bool Factory(const cp_plugin_info_t* plugin, TYPE type, CAddonBuilder& builder, bool ignoreExtensions = false);
-    static void FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder& builder);
+    static bool Factory(const cp_plugin_info_t* plugin, TYPE type, CAddonBuilder& builder, bool ignoreExtensions = false, std::string assetBasePath = "");
+    static void FillCpluffMetadata(const cp_plugin_info_t* plugin, CAddonBuilder& builder, std::string const& assetBasePath);
 
   private:
     CAddonMgr& operator=(CAddonMgr const&) = delete;
diff --git a/xbmc/addons/AddonVersion.cpp b/xbmc/addons/AddonVersion.cpp
index 50f4a0ed0f..38408f6213 100644
--- a/xbmc/addons/AddonVersion.cpp
+++ b/xbmc/addons/AddonVersion.cpp
@@ -24,8 +24,17 @@
 #include <ctype.h>
 
 #include "AddonVersion.h"
+#include "utils/log.h"
 #include "utils/StringUtils.h"
 
+namespace {
+// Add-on versions are used e.g. in file names and should
+// not have too much freedom in their accepted characters
+// Things that should be allowed: e.g. 0.1.0~beta3+git010cab3
+// Note that all of these characters are url-safe
+const std::string VALID_ADDON_VERSION_CHARACTERS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.+_@~";
+}
+
 namespace ADDON
 {
   AddonVersion::AddonVersion(const std::string& version)
@@ -42,8 +51,19 @@ namespace ADDON
     if (pos != std::string::npos)
     {
       mRevision = mUpstream.substr(pos+1);
+      if (mRevision.find_first_not_of(VALID_ADDON_VERSION_CHARACTERS) != std::string::npos)
+      {
+        CLog::Log(LOGERROR, "AddonVersion: {} is not a valid revision number", mRevision);
+        mRevision = "";
+      }
       mUpstream.erase(pos);
     }
+
+    if (mUpstream.find_first_not_of(VALID_ADDON_VERSION_CHARACTERS) != std::string::npos)
+    {
+      CLog::Log(LOGERROR, "AddonVersion: {} is not a valid version", mUpstream);
+      mUpstream = "0.0.0";
+    }
   }
 
   /**Compare two components of a Debian-style version.  Return -1, 0, or 1
diff --git a/xbmc/addons/Repository.cpp b/xbmc/addons/Repository.cpp
index 91499be17a..1107097511 100644
--- a/xbmc/addons/Repository.cpp
+++ b/xbmc/addons/Repository.cpp
@@ -21,6 +21,7 @@
 #include "Repository.h"
 
 #include <iterator>
+#include <tuple>
 #include <utility>
 
 #include "ServiceBroker.h"
@@ -41,6 +42,8 @@
 #include "settings/Settings.h"
 #include "TextureDatabase.h"
 #include "URL.h"
+#include "utils/Base64.h"
+#include "utils/Digest.h"
 #include "utils/JobManager.h"
 #include "utils/log.h"
 #include "utils/Mime.h"
@@ -52,9 +55,107 @@
 using namespace XFILE;
 using namespace ADDON;
 using namespace KODI::MESSAGING;
+using KODI::UTILITY::CDigest;
+using KODI::UTILITY::TypedDigest;
 
 using KODI::MESSAGING::HELPERS::DialogResponse;
 
+CRepository::ResolveResult CRepository::ResolvePathAndHash(const AddonPtr& addon) const
+{
+  std::string const& path = addon->Path();
+
+  auto dirIt = std::find_if(m_dirs.begin(), m_dirs.end(), [&path](DirInfo const& dir)
+  {
+    return URIUtils::PathHasParent(path, dir.datadir, true);
+  });
+  if (dirIt == m_dirs.end())
+  {
+    CLog::Log(LOGERROR, "Requested path {} not found in known repository directories", path);
+    return {};
+  }
+
+  if (dirIt->hashType == CDigest::Type::INVALID)
+  {
+    // We have a path, but need no hash
+    return {path, {}};
+  }
+
+  // Do not follow mirror redirect, we want the headers of the redirect response
+  CURL url{path};
+  url.SetProtocolOption("redirect-limit", "0");
+  CCurlFile file;
+  if (!file.Open(url))
+  {
+    CLog::Log(LOGERROR, "Could not fetch addon location and hash from {}", path);
+    return {};
+  }
+
+  std::string hashTypeStr = CDigest::TypeToString(dirIt->hashType);
+
+  // Return the location from the header so we don't have to look it up again
+  // (saves one request per addon install)
+  std::string location = file.GetRedirectURL();
+  // content-* headers are base64, convert to base16
+  TypedDigest hash{dirIt->hashType, StringUtils::ToHexadecimal(Base64::Decode(file.GetHttpHeader().GetValue(std::string("content-") + hashTypeStr)))};
+
+  if (hash.Empty())
+  {
+    // Expected hash, but none found -> fall back to old method
+    if (!FetchChecksum(path + "." + hashTypeStr, hash.value) || hash.Empty())
+    {
+      CLog::Log(LOGERROR, "Failed to find hash for {} from HTTP header and in separate file", path);
+      return {};
+    }
+  }
+  if (location.empty())
+  {
+    // Fall back to original URL if we do not get a redirect
+    location = path;
+  }
+
+  CLog::Log(LOGDEBUG, "Resolved addon path {} to {} hash {}", path, location, hash.value);
+
+  return {location, hash};
+}
+
+CRepository::DirInfo CRepository::ParseDirConfiguration(cp_cfg_element_t* configuration)
+{
+  auto const& mgr = CServiceBroker::GetAddonMgr();
+  DirInfo dir;
+  dir.checksum = mgr.GetExtValue(configuration, "checksum");
+  std::string checksumStr = mgr.GetExtValue(configuration, "checksum@verify");
+  if (!checksumStr.empty())
+  {
+    dir.checksumType = CDigest::TypeFromString(checksumStr);
+  }
+  dir.info = mgr.GetExtValue(configuration, "info");
+  dir.datadir = mgr.GetExtValue(configuration, "datadir");
+  dir.artdir = mgr.GetExtValue(configuration, "artdir");
+  if (dir.artdir.empty())
+  {
+    dir.artdir = dir.datadir;
+  }
+
+  std::string hashStr = mgr.GetExtValue(configuration, "hashes");
+  StringUtils::ToLower(hashStr);
+  if (hashStr == "true")
+  {
+    // Deprecated alias
+    hashStr = "md5";
+  }
+  if (!hashStr.empty() && hashStr != "false")
+  {
+    dir.hashType = CDigest::TypeFromString(hashStr);
+    if (dir.hashType == CDigest::Type::MD5)
+    {
+      CLog::Log(LOGWARNING, "Repository has MD5 hashes enabled - this hash function is broken and will only guard against unintentional data corruption");
+    }
+  }
+
+  dir.version = AddonVersion{mgr.GetExtValue(configuration, "@minversion")};
+  return dir;
+}
+
 std::unique_ptr<CRepository> CRepository::FromExtension(CAddonInfo addonInfo, const cp_extension_t* ext)
 {
   DirList dirs;
@@ -64,30 +165,19 @@ std::unique_ptr<CRepository> CRepository::FromExtension(CAddonInfo addonInfo, co
     version = addonver->Version();
   for (size_t i = 0; i < ext->configuration->num_children; ++i)
   {
-    if(ext->configuration->children[i].name &&
-       strcmp(ext->configuration->children[i].name, "dir") == 0)
+    cp_cfg_element_t* element = &ext->configuration->children[i];
+    if(element->name && strcmp(element->name, "dir") == 0)
     {
-      AddonVersion min_version(CServiceBroker::GetAddonMgr().GetExtValue(&ext->configuration->children[i], "@minversion"));
-      if (min_version <= version)
+      DirInfo dir = ParseDirConfiguration(element);
+      if (dir.version <= version)
       {
-        DirInfo dir;
-        dir.version = min_version;
-        dir.checksum = CServiceBroker::GetAddonMgr().GetExtValue(&ext->configuration->children[i], "checksum");
-        dir.info = CServiceBroker::GetAddonMgr().GetExtValue(&ext->configuration->children[i], "info");
-        dir.datadir = CServiceBroker::GetAddonMgr().GetExtValue(&ext->configuration->children[i], "datadir");
-        dir.hashes = CServiceBroker::GetAddonMgr().GetExtValue(&ext->configuration->children[i], "hashes") == "true";
         dirs.push_back(std::move(dir));
       }
     }
   }
   if (!CServiceBroker::GetAddonMgr().GetExtValue(ext->configuration, "info").empty())
   {
-    DirInfo info;
-    info.checksum = CServiceBroker::GetAddonMgr().GetExtValue(ext->configuration, "checksum");
-    info.info = CServiceBroker::GetAddonMgr().GetExtValue(ext->configuration, "info");
-    info.datadir = CServiceBroker::GetAddonMgr().GetExtValue(ext->configuration, "datadir");
-    info.hashes = CServiceBroker::GetAddonMgr().GetExtValue(ext->configuration, "hashes") == "true";
-    dirs.push_back(std::move(info));
+    dirs.push_back(ParseDirConfiguration(ext->configuration));
   }
   return std::unique_ptr<CRepository>(new CRepository(std::move(addonInfo), std::move(dirs)));
 }
@@ -95,34 +185,13 @@ std::unique_ptr<CRepository> CRepository::FromExtension(CAddonInfo addonInfo, co
 CRepository::CRepository(CAddonInfo addonInfo, DirList dirs)
     : CAddon(std::move(addonInfo)), m_dirs(std::move(dirs))
 {
-}
-
-
-bool CRepository::GetAddonHash(const AddonPtr& addon, std::string& checksum) const
-{
-  DirList::const_iterator it;
-  for (it = m_dirs.begin();it != m_dirs.end(); ++it)
-    if (URIUtils::PathHasParent(addon->Path(), it->datadir, true))
-      break;
-
-  if (it != m_dirs.end())
+  for (auto const& dir : m_dirs)
   {
-    if (!it->hashes)
+    if (CURL{dir.datadir}.IsProtocol("http"))
     {
-      checksum = "";
-      return true;
-    }
-    if (FetchChecksum(addon->Path() + ".md5", checksum))
-    {
-      size_t pos = checksum.find_first_of(" \n");
-      if (pos != std::string::npos)
-      {
-        checksum = checksum.substr(0, pos);
-        return true;
-      }
+      CLog::Log(LOGWARNING, "Repository {} uses plain HTTP for add-on downloads - this is insecure and will make your Kodi installation vulnerable to attacks if enabled!", Name());
     }
   }
-  return false;
 }
 
 bool CRepository::FetchChecksum(const std::string& url, std::string& checksum) noexcept
@@ -141,10 +210,15 @@ bool CRepository::FetchChecksum(const std::string& url, std::string& checksum) n
   if (read <= -1)
     return false;
   checksum = ss.str();
+  std::size_t pos = checksum.find_first_of(" \n");
+  if (pos != std::string::npos)
+  {
+    checksum = checksum.substr(0, pos);
+  }
   return true;
 }
 
-bool CRepository::FetchIndex(const DirInfo& repo, VECADDONS& addons) noexcept
+bool CRepository::FetchIndex(const DirInfo& repo, std::string const& digest, VECADDONS& addons) noexcept
 {
   XFILE::CCurlFile http;
   http.SetAcceptEncoding("gzip");
@@ -156,6 +230,16 @@ bool CRepository::FetchIndex(const DirInfo& repo, VECADDONS& addons) noexcept
     return false;
   }
 
+  if (repo.checksumType != CDigest::Type::INVALID)
+  {
+    std::string actualDigest = CDigest::Calculate(repo.checksumType, response);
+    if (!StringUtils::EqualsNoCase(digest, actualDigest))
+    {
+      CLog::Log(LOGERROR, "CRepository: {} index has wrong digest {}, expected: {}", repo.info, actualDigest, digest);
+      return false;
+    }
+  }
+
   if (URIUtils::HasExtension(repo.info, ".gz")
       || CMime::GetFileTypeFromMime(http.GetProperty(XFILE::FILE_PROPERTY_MIME_TYPE)) == CMime::EFileType::FileTypeGZip)
   {
@@ -176,6 +260,7 @@ CRepository::FetchStatus CRepository::FetchIfChanged(const std::string& oldCheck
     std::string& checksum, VECADDONS& addons) const
 {
   checksum = "";
+  std::vector<std::tuple<DirInfo const&, std::string>> dirChecksums;
   for (const auto& dir : m_dirs)
   {
     if (!dir.checksum.empty())
@@ -186,6 +271,7 @@ CRepository::FetchStatus CRepository::FetchIfChanged(const std::string& oldCheck
         CLog::Log(LOGERROR, "CRepository: failed read '%s'", dir.checksum.c_str());
         return STATUS_ERROR;
       }
+      dirChecksums.emplace_back(dir, part);
       checksum += part;
     }
   }
@@ -193,10 +279,10 @@ CRepository::FetchStatus CRepository::FetchIfChanged(const std::string& oldCheck
   if (oldChecksum == checksum && !oldChecksum.empty())
     return STATUS_NOT_MODIFIED;
 
-  for (const auto& dir : m_dirs)
+  for (const auto& dirTuple : dirChecksums)
   {
     VECADDONS tmp;
-    if (!FetchIndex(dir, tmp))
+    if (!FetchIndex(std::get<0>(dirTuple), std::get<1>(dirTuple), tmp))
       return STATUS_ERROR;
     addons.insert(addons.end(), tmp.begin(), tmp.end());
   }
diff --git a/xbmc/addons/Repository.h b/xbmc/addons/Repository.h
index 78a2624810..67bd38002c 100644
--- a/xbmc/addons/Repository.h
+++ b/xbmc/addons/Repository.h
@@ -24,9 +24,12 @@
 #include <vector>
 
 #include "Addon.h"
+#include "utils/Digest.h"
 #include "utils/Job.h"
 #include "utils/ProgressJob.h"
 
+struct cp_cfg_element_t;
+
 namespace ADDON
 {
   class CRepository : public CAddon
@@ -34,12 +37,13 @@ namespace ADDON
   public:
     struct DirInfo
     {
-      DirInfo() : version("0.0.0"), hashes(false) {}
-      AddonVersion version;
+      AddonVersion version{""};
       std::string info;
       std::string checksum;
+      KODI::UTILITY::CDigest::Type checksumType{KODI::UTILITY::CDigest::Type::INVALID};
       std::string datadir;
-      bool hashes;
+      std::string artdir;
+      KODI::UTILITY::CDigest::Type hashType{KODI::UTILITY::CDigest::Type::INVALID};
     };
 
     typedef std::vector<DirInfo> DirList;
@@ -63,9 +67,18 @@ namespace ADDON
 
     FetchStatus FetchIfChanged(const std::string& oldChecksum, std::string& checksum, VECADDONS& addons) const;
 
+    struct ResolveResult
+    {
+      std::string location;
+      KODI::UTILITY::TypedDigest digest;
+    };
+    ResolveResult ResolvePathAndHash(AddonPtr const& addon) const;
+
   private:
     static bool FetchChecksum(const std::string& url, std::string& checksum) noexcept;
-    static bool FetchIndex(const DirInfo& repo, VECADDONS& addons) noexcept;
+    static bool FetchIndex(const DirInfo& repo, std::string const& digest, VECADDONS& addons) noexcept;
+
+    static DirInfo ParseDirConfiguration(cp_cfg_element_t* configuration);
 
     DirList m_dirs;
   };
diff --git a/xbmc/filesystem/CurlFile.cpp b/xbmc/filesystem/CurlFile.cpp
index 9a07efc058..d5a58190b5 100644
--- a/xbmc/filesystem/CurlFile.cpp
+++ b/xbmc/filesystem/CurlFile.cpp
@@ -535,10 +535,6 @@ void CCurlFile::SetCommonOptions(CReadState* state)
     state->m_curlAliasList = g_curlInterface.slist_append(state->m_curlAliasList, "ICY 200 OK");
   g_curlInterface.easy_setopt(h, CURLOPT_HTTP200ALIASES, state->m_curlAliasList);
 
-  // never verify peer, we don't have any certificates to do this
-  g_curlInterface.easy_setopt(h, CURLOPT_SSL_VERIFYPEER, 0);
-  g_curlInterface.easy_setopt(h, CURLOPT_SSL_VERIFYHOST, 0);
-
   g_curlInterface.easy_setopt(m_state->m_easyHandle, CURLOPT_URL, m_url.c_str());
   g_curlInterface.easy_setopt(m_state->m_easyHandle, CURLOPT_TRANSFERTEXT, CURL_OFF);
 
@@ -1065,8 +1061,8 @@ bool CCurlFile::Open(const CURL& url)
     }
   }
 
-  char* efurl;
-  if (CURLE_OK == g_curlInterface.easy_getinfo(m_state->m_easyHandle, CURLINFO_EFFECTIVE_URL,&efurl) && efurl)
+  std::string efurl = GetInfoString(CURLINFO_EFFECTIVE_URL);
+  if (!efurl.empty())
   {
     if (m_url != efurl)
     {
@@ -1102,8 +1098,8 @@ bool CCurlFile::OpenForWrite(const CURL& url, bool bOverWrite)
   SetCommonOptions(m_state);
   SetRequestHeaders(m_state);
 
-  char* efurl;
-  if (CURLE_OK == g_curlInterface.easy_getinfo(m_state->m_easyHandle, CURLINFO_EFFECTIVE_URL,&efurl) && efurl)
+  std::string efurl = GetInfoString(CURLINFO_EFFECTIVE_URL);
+  if (!efurl.empty())
     m_url = efurl;
 
   m_opened = true;
@@ -1773,6 +1769,23 @@ std::string CCurlFile::GetURL(void)
   return m_url;
 }
 
+std::string CCurlFile::GetRedirectURL()
+{
+  return GetInfoString(CURLINFO_REDIRECT_URL);
+}
+
+std::string CCurlFile::GetInfoString(int infoType)
+{
+  char* info{};
+  CURLcode result = g_curlInterface.easy_getinfo(m_state->m_easyHandle, static_cast<XCURL::CURLINFO> (infoType), &info);
+  if (result != CURLE_OK)
+  {
+    CLog::Log(LOGERROR, "Info string request for type {} failed with result code {}", infoType, result);
+    return "";
+  }
+  return (info ? info : "");
+}
+
 /* STATIC FUNCTIONS */
 bool CCurlFile::GetHttpHeader(const CURL &url, CHttpHeader &headers)
 {
diff --git a/xbmc/filesystem/CurlFile.h b/xbmc/filesystem/CurlFile.h
index 62215b15eb..463cde64bb 100644
--- a/xbmc/filesystem/CurlFile.h
+++ b/xbmc/filesystem/CurlFile.h
@@ -94,6 +94,7 @@ namespace XFILE
 
       const CHttpHeader& GetHttpHeader() const { return m_state->m_httpheader; }
       std::string GetURL(void);
+      std::string GetRedirectURL();
 
       /* static function that will get content type of a file */
       static bool GetHttpHeader(const CURL &url, CHttpHeader &headers);
@@ -156,6 +157,7 @@ namespace XFILE
       void SetRequestHeaders(CReadState* state);
       void SetCorrectHeaders(CReadState* state);
       bool Service(const std::string& strURL, std::string& strHTML);
+      std::string GetInfoString(int infoType);
 
     protected:
       CReadState* m_state;
diff --git a/xbmc/filesystem/File.h b/xbmc/filesystem/File.h
index 2d06efec0d..466530d13e 100644
--- a/xbmc/filesystem/File.h
+++ b/xbmc/filesystem/File.h
@@ -24,9 +24,6 @@
 //
 //////////////////////////////////////////////////////////////////////
 
-#if !defined(AFX_FILE_H__A7ED6320_C362_49CB_8925_6C6C8CAE7B78__INCLUDED_)
-#define AFX_FILE_H__A7ED6320_C362_49CB_8925_6C6C8CAE7B78__INCLUDED_
-
 #pragma once
 
 #include <iostream>
@@ -234,4 +231,3 @@ private:
 };
 
 }
-#endif // !defined(AFX_FILE_H__A7ED6320_C362_49CB_8925_6C6C8CAE7B78__INCLUDED_)
diff --git a/xbmc/utils/Digest.cpp b/xbmc/utils/Digest.cpp
index 04ef0e5c57..4bd6cb1544 100644
--- a/xbmc/utils/Digest.cpp
+++ b/xbmc/utils/Digest.cpp
@@ -53,6 +53,11 @@ EVP_MD const * TypeToEVPMD(CDigest::Type type)
 
 }
 
+std::ostream& operator<<(std::ostream& os, TypedDigest const& digest)
+{
+  return os << "{" << CDigest::TypeToString(digest.type) << "}" << digest.value;
+}
+
 std::string CDigest::TypeToString(Type type)
 {
   switch (type)
@@ -65,6 +70,8 @@ std::string CDigest::TypeToString(Type type)
       return "sha256";
     case Type::SHA512:
       return "sha512";
+    case Type::INVALID:
+      return "invalid";
     default:
       throw std::invalid_argument("Unknown digest type");
   }
diff --git a/xbmc/utils/Digest.h b/xbmc/utils/Digest.h
index a3d1c869b2..ab6903035a 100644
--- a/xbmc/utils/Digest.h
+++ b/xbmc/utils/Digest.h
@@ -22,9 +22,13 @@
 
 #include <openssl/evp.h>
 
+#include <iostream>
 #include <memory>
+#include <stdexcept>
 #include <string>
 
+#include "StringUtils.h"
+
 namespace KODI
 {
 namespace UTILITY
@@ -41,7 +45,8 @@ public:
     MD5,
     SHA1,
     SHA256,
-    SHA512
+    SHA512,
+    INVALID
   };
 
   /**
@@ -108,5 +113,39 @@ private:
   EVP_MD const* m_md;
 };
 
+struct TypedDigest
+{
+  CDigest::Type type{CDigest::Type::INVALID};
+  std::string value;
+
+  TypedDigest()
+  {}
+
+  TypedDigest(CDigest::Type type, std::string const& value)
+  : type(type), value(value)
+  {}
+
+  bool Empty() const
+  {
+    return (type == CDigest::Type::INVALID || value.empty());
+  }
+};
+
+inline bool operator==(TypedDigest const& left, TypedDigest const& right)
+{
+  if (left.type != right.type)
+  {
+    throw std::logic_error("Cannot compare digests of different type");
+  }
+  return StringUtils::EqualsNoCase(left.value, right.value);
+}
+
+inline bool operator!=(TypedDigest const& left, TypedDigest const& right)
+{
+  return !(left == right);
+}
+
+std::ostream& operator<<(std::ostream& os, TypedDigest const& digest);
+
 }
 }
diff --git a/xbmc/utils/test/TestDigest.cpp b/xbmc/utils/test/TestDigest.cpp
index 5a052aef49..d1b5e67dd0 100644
--- a/xbmc/utils/test/TestDigest.cpp
+++ b/xbmc/utils/test/TestDigest.cpp
@@ -23,6 +23,7 @@
 #include "gtest/gtest.h"
 
 using KODI::UTILITY::CDigest;
+using KODI::UTILITY::TypedDigest;
 
 TEST(TestDigest, Digest_Empty)
 {
@@ -69,3 +70,42 @@ TEST(TestDigest, Digest_SHA512)
   EXPECT_STREQ(CDigest::Calculate(CDigest::Type::SHA512, "").c_str(), "cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e");
   EXPECT_STREQ(CDigest::Calculate(CDigest::Type::SHA512, "asdf").c_str(), "401b09eab3c013d4ca54922bb802bec8fd5318192b0a75f201d8b3727429080fb337591abd3e44453b954555b7a0812e1081c39b740293f765eae731f5a65ed1");
 }
+
+TEST(TestDigest, TypedDigest_Empty)
+{
+  TypedDigest t1, t2;
+  EXPECT_EQ(t1, t2);
+  EXPECT_EQ(t1.type, CDigest::Type::INVALID);
+  EXPECT_EQ(t1.value, "");
+  EXPECT_TRUE(t1.Empty());
+  t1.type = CDigest::Type::SHA1;
+  EXPECT_TRUE(t1.Empty());
+}
+
+TEST(TestDigest, TypedDigest_SameType)
+{
+  TypedDigest t1{CDigest::Type::SHA1, "da39a3ee5e6b4b0d3255bfef95601890afd80709"};
+  TypedDigest t2{CDigest::Type::SHA1, "da39a3ee5e6b4b0d3255bfef95601890afd80708"};
+  EXPECT_NE(t1, t2);
+  EXPECT_FALSE(t1.Empty());
+}
+
+TEST(TestDigest, TypedDigest_CompareCase)
+{
+  TypedDigest t1{CDigest::Type::SHA1, "da39a3ee5e6b4b0d3255bfef95601890afd80708"};
+  TypedDigest t2{CDigest::Type::SHA1, "da39A3EE5e6b4b0d3255bfef95601890afd80708"};
+  EXPECT_EQ(t1, t2);
+}
+
+TEST(TestDigest, TypedDigest_DifferingType)
+{
+  TypedDigest t1{CDigest::Type::SHA1, "da39a3ee5e6b4b0d3255bfef95601890afd80709"};
+  TypedDigest t2{CDigest::Type::SHA256, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"};
+  // Silence "unused expression" warning
+  bool a;
+  EXPECT_THROW(a = (t1 == t2), std::logic_error);
+  // Silence "unused variable" warning
+  (void)a;
+  EXPECT_THROW(a = (t1 != t2), std::logic_error);
+  (void)a;
+}