Generalize integrity checking and implement it for git origins #210
Conversation
The verification of downloaded info is generalized for all origins, and also we allow having more than a single hash.
doc/catalog-format-spec.rst
Outdated
| * ``archive-hash``: mandatory string for source archives. A "kind:digest" field | ||
| that specifies a hash kind and its value. The only accepted kind is SHA512 at | ||
| this time. | ||
| * ``origin-hashes``: mandatory string for source archives, optional for git |
There was a problem hiding this comment.
I don't think it should be optional for git.
There was a problem hiding this comment.
In accordance with the comment in #210. OK.
There was a problem hiding this comment.
I have made it a warning for now, with the idea of turning it into an error once the current index is completely updated.
doc/catalog-format-spec.rst
Outdated
| this time. | ||
| * ``origin-hashes``: mandatory string for source archives, optional for git | ||
| origins. An array of "kind:digest" fields that specify a hash kind and its | ||
| value. Kinds accepted are sha256, sha384, sha512. |
There was a problem hiding this comment.
Why accept multiple kinds? Let's take the most secure.
src/alire/alire-hashes-compute.adb
Outdated
| Exit_Code : Integer; | ||
| begin | ||
| OS_Lib.Subprocess.Raw_Spawn | ||
| (Program => "sh", |
There was a problem hiding this comment.
It would be better to use the hash functions provided in libGNAT. There would be less dependency on external tools.
There was a problem hiding this comment.
I thought we wanted this for simplicity. Will revert the change.
| Output : Utils.String_Vector; | ||
| Exit_Code : Integer; | ||
| begin | ||
| Raw_Spawn (Program => "sh", |
There was a problem hiding this comment.
Why use sh here? Is it possible to spawn git directly?
There was a problem hiding this comment.
I don't know a way to pipe stdout of a subprocess to stdin with GNAT.Os_Lib, but I might have missed it. Do you know if GNAT has portable support for some kind of pipes?
Alternatively, we can use a temporary file.
There was a problem hiding this comment.
Done with temporary file (commit 7f10d7c).
| .Empty_Vector | ||
| .Append ("-c") | ||
| .Append ("git -c core.autocrlf=false archive HEAD | " | ||
| & Hasher & " | cut -f1 -d\ "), |
There was a problem hiding this comment.
Like above, we should use libGNAT to produce the hashes.
| Archive_Name : constant String := This.Base.Archive_Name; | ||
| Archive_File : constant String := Dirs.Compose (Folder, Archive_Name); | ||
| begin | ||
| return Hashes.Digest (Hashes.Compute.Hash_File (Kind, Archive_File)); |
There was a problem hiding this comment.
Just to be sure, is this doing a hash of the archive before it is extracted?
Being subtypes of string made too easy to mix them up
And refactored Source_Archive to fit in
Using GNAT.OS_Lib.Argument_String_To_List seems to fail on strings with pipes, so we need a way of directly passing such arguments to an OS shell
This way we can test integrity verification of VCSs without needing internet connectivity.
| end if; | ||
|
|
||
| -- Compute hashes from downloaded release and verify: | ||
| for Index_Hash of This.Base.Data.Hashes loop |
There was a problem hiding this comment.
So if multiple hashes are provided, all of them must be correct.
There was a problem hiding this comment.
Yes. We only support one now, but I think it may come in handy for future-proofing.
We still need this to merge alire-project/alire#210 and then we can add the missing hashes progressively.
This PR generalizes the
archive-hashintoorigin-hashesand implements integrity checking for git origins. Functional changes are:origin-hashes, that applies to all origins, instead ofarchive-hash, which only applied to archive sources.git archive | shaXsumtrick for platform-independent verification of git remotes.Unless mistaken, this covers the basic integrity verification that we wanted for the beta. I added hg/svn to #198 to track their progress. I also created #208 and #209 to track related improvements.