From 9bb15cd34800973aee0eaa876e837477bd14a80e Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Tue, 30 Jul 2024 14:25:07 -0500 Subject: [PATCH 1/3] Stop checking multiple hashes --- Core/Net/NetModuleCache.cs | 43 +++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/Core/Net/NetModuleCache.cs b/Core/Net/NetModuleCache.cs index 2ed1827727..a69ce5dc84 100644 --- a/Core/Net/NetModuleCache.cs +++ b/Core/Net/NetModuleCache.cs @@ -155,10 +155,8 @@ public string GetFileHashSha256(string filePath, IProgress progress, Cance /// public string Store(CkanModule module, string path, IProgress progress, string description = null, bool move = false, CancellationToken cancelToken = default) { - // ZipValid takes a lot longer than the hash steps, so scale them 60:20:20 - const int zipValidPercent = 60; - const int hashSha1Percent = 20; - const int hashSha256Percent = 20; + // ZipValid takes a lot longer than the hash check, so scale them 70:30 if hashes are present + int zipValidPercent = module.download_hash == null ? 100 : 70; progress?.Report(0); // Check file exists @@ -192,24 +190,31 @@ public string Store(CkanModule module, string path, IProgress progress, st // Some older metadata doesn't have hashes if (module.download_hash != null) { - // Check SHA1 match - string sha1 = GetFileHashSha1(path, new Progress(percent => - progress?.Report(zipValidPercent + (percent * hashSha1Percent / 100))), cancelToken); - if (sha1 != module.download_hash.sha1) + int hashPercent = 100 - zipValidPercent; + // Only check one hash, sha256 if it's set, sha1 otherwise + if (!string.IsNullOrEmpty(module.download_hash.sha256)) { - throw new InvalidModuleFileKraken(module, path, string.Format( - Properties.Resources.NetModuleCacheMismatchSHA1, - module, path, sha1, module.download_hash.sha1)); + // Check SHA256 match + string sha256 = GetFileHashSha256(path, new Progress(percent => + progress?.Report(zipValidPercent + (percent * hashPercent / 100))), cancelToken); + if (sha256 != module.download_hash.sha256) + { + throw new InvalidModuleFileKraken(module, path, string.Format( + Properties.Resources.NetModuleCacheMismatchSHA256, + module, path, sha256, module.download_hash.sha256)); + } } - - // Check SHA256 match - string sha256 = GetFileHashSha256(path, new Progress(percent => - progress?.Report(zipValidPercent + hashSha1Percent + (percent * hashSha256Percent / 100))), cancelToken); - if (sha256 != module.download_hash.sha256) + else if (!string.IsNullOrEmpty(module.download_hash.sha1)) { - throw new InvalidModuleFileKraken(module, path, string.Format( - Properties.Resources.NetModuleCacheMismatchSHA256, - module, path, sha256, module.download_hash.sha256)); + // Check SHA1 match + string sha1 = GetFileHashSha1(path, new Progress(percent => + progress?.Report(zipValidPercent + (percent * hashPercent / 100))), cancelToken); + if (sha1 != module.download_hash.sha1) + { + throw new InvalidModuleFileKraken(module, path, string.Format( + Properties.Resources.NetModuleCacheMismatchSHA1, + module, path, sha1, module.download_hash.sha1)); + } } } From 9476a44a4319d01afd73f3ce4e996c5606d3eccd Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Tue, 30 Jul 2024 14:41:53 -0500 Subject: [PATCH 2/3] Omit sha1 hash for spec versions newer than v1.34 --- Netkan/Transformers/DownloadAttributeTransformer.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Netkan/Transformers/DownloadAttributeTransformer.cs b/Netkan/Transformers/DownloadAttributeTransformer.cs index 6428d8df9a..c3443f8d41 100644 --- a/Netkan/Transformers/DownloadAttributeTransformer.cs +++ b/Netkan/Transformers/DownloadAttributeTransformer.cs @@ -1,7 +1,10 @@ using System; using System.Collections.Generic; + using log4net; using Newtonsoft.Json.Linq; + +using CKAN.Versioning; using CKAN.NetKAN.Extensions; using CKAN.NetKAN.Model; using CKAN.NetKAN.Services; @@ -45,8 +48,12 @@ public IEnumerable Transform(Metadata metadata, TransformOptions opts) json["download_hash"] = new JObject(); var download_hashJson = (JObject)json["download_hash"]; - Log.Debug("Calculating download SHA1..."); - download_hashJson.SafeAdd("sha1", _fileService.GetFileHashSha1(file)); + // Older clients will complain if download_hash is set without sha1 + if (metadata.SpecVersion <= v1p34) + { + Log.Debug("Calculating download SHA1..."); + download_hashJson.SafeAdd("sha1", _fileService.GetFileHashSha1(file)); + } Log.Debug("Calculating download SHA256..."); download_hashJson.SafeAdd("sha256", _fileService.GetFileHashSha256(file)); @@ -63,5 +70,6 @@ public IEnumerable Transform(Metadata metadata, TransformOptions opts) yield return metadata; } } + private static readonly ModuleVersion v1p34 = new ModuleVersion("v1.34"); } } From 68fa8ebd55813f2a97e5c6e0094ed6a42797c32e Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Tue, 30 Jul 2024 15:49:06 -0500 Subject: [PATCH 3/3] Mark download_hash.sha1 optional in spec --- Spec.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Spec.md b/Spec.md index 4024e0d069..887d6a209e 100644 --- a/Spec.md +++ b/Spec.md @@ -451,14 +451,14 @@ and not filled in by hand. ##### download_hash -If supplied, `download_hash` is an object of hash digests. Currently -SHA1 and SHA256 calculated hashes of the resulting file downloaded. +If supplied, `download_hash` is an object of hash digests of the downloaded file. +Clients up to **v1.34** require both `sha1` and `sha256` to be populated, but +later clients allow either or both to be omitted. It is recommended that this field is only generated by automated tools (where it is encouraged), and not filled in by hand. ```json "download_hash": { - "sha1": "1F4B3F21A77D4A302E3417A7C7A24A0B63740FC5", "sha256": "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855" } ```