chocolatey
(#1144) Add validation for package hash
Description Of Changes
Add usePackageHashValidation feature
This feature is used to control if the checksum of the downloaded .nupkg is checked against the checksum provided by the package source. It is disabled by default, as it unknown if it works correctly with all major NuGet v2 implementations.
Add HashConverter class
This class provides a method to convert base64 hashes into hex form, returning the input if it is already in hex. It also returns the type of the hash. Both are based on the length of the input hash. Sha1, Sha256 and Sha512 are supported.
Implement usePackageHashValidation feature
This adds a check to both install and upgrade to validate that the downloaded .nupkg file has the same hash as the source metadata. The check is conducted before the package is installed. If the source does not provide a sha512 checksum or if usePackageHashValidation is disabled, then the check is skipped.
Only sha512 is supported because it is the only provided package hash type after download. The provided hash is used because it accounts for package signing correctly.
Motivation and Context
See #1144
Testing
WIP
Operating Systems Testing
- Windows 10 22H2
Change Types Made
- [ ] Bug fix (non-breaking change).
- [x] Feature / Enhancement (non-breaking change).
- [ ] Breaking change (fix or feature that could cause existing functionality to change).
- [ ] Documentation changes.
- [ ] PowerShell code changes.
Change Checklist
- [ ] Requires a change to the documentation.
- [ ] Documentation has been updated.
- [ ] Tests to cover my changes, have been added.
- [ ] All new and existing tests passed?
- [ ] PowerShell code changes: PowerShell v2 compatibility checked?
Related Issue
Fixes #1144
@TheCakeIsNaOH are you in a position to complete the template for this PR?
I have a couple questions about the implementation...
- Should there be a feature that can be toggled on/off for verifying the checksum of the package?
- Related to the above, should it be possible to enable/disable checksum validation on a per source basis?
@gep13 I agree that a way to disable it would be appropriate. I think either a global switch, or a per source switch would be ideal, not both.
The implementation in code is not complete, so I can't completely fill in the template yet.
- Non-sha512 should be explicitly handled via checking the hash type in the metadata and either ignoring and logging, or manually getting the checksum of the downloaded files in that other algorithm.
- A way to disable the check, as mentioned above.
- Handling for when a normal string, instead of base64, is provided by the server (Nexus provides a normal string instead of base64 I think).
@TheCakeIsNaOH said... I think either a global switch, or a per source switch would be ideal, not both.
Let's go with a feature called usePackageHashValidation, which is off by default, and folks can opt into it.
Let's go with a feature called usePackageHashValidation, which is off by default, and folks can opt into it.
Sounds good, I'll get that added.
@TheCakeIsNaOH I really like the idea of getting this added, and I have made some refactoring to the changes that you have here (created a ValidatePackageHash method, rather than duplicating the code, as well as some wording changes based on a conversation with @pauby).
Are you in a position to provide some testing steps in the PR template?
I have tried to take this for a spin, doing a normal choco install windirstat but the hash isn't being validated correctly. I haven't dug into this yet, as it is coming to the end of my day here. If you are in a position to take a look at it, that would be great, otherwise I will pick this up next week.
It looks like wires got crossed somewhere, as the local package hash was not being normalized to hex, only the remote hash. It probably crept in during one of my rebases. I've fixed that up, and it should be working now. I've also added testing steps.