On Sat, 2016-11-05 at 01:03 +0100, Philip Hands wrote:
> Hi,
>
> First, apologies for the timing on this, real-life has eaten all my time
> of late, and when I had some time to spare the InRelease split bug threw
> a spanner in the works.
>
> Here are some patches that I've been failing to get into d-i for a
> couple of releases -- it would be really nice to avoid making that 3
> releases.
>
> https://anonscm.debian.org/cgit/d-i/preseed.git/log/?h=pu/preseed-fetch-checksum
> and
> https://anonscm.debian.org/cgit/d-i/debian-installer-utils.git/log/?h=pu/preseed-fetch-checksum
I have some review comments.
I think fetch-url should delete the destination file if its checksum is
incorrect. The destination might be used as a cache location and
assumed to be valid at a later stage.
I think the commit to preseed that relies on this ("allow preseed_fetch
to pass all it's params onto fetch-url") should also add a versioned
dependency on debian-installer-utils. The commit message could also be
improved - this change doesn't *allow* passing those parameters; it
*does* pass them. Finally, it adds a spurious blank line further down
debian/changelog.
A later commit ("distinguish checksum mismatches from retrieval
errors") does:
- if ! preseed_fetch "$location" "$tmp" "$sum"; then
+ retval=0
+ preseed_fetch "$location" "$tmp" "$checksum" || retval=$?
+ case $retval in
+ 0) ;;
+ 2)
+ log "checksum error \"$location\", expected \"$checksum\""
+ error checksum_error "$location" "$checksum"
+ ;;
It looks like all instances of "$checksum" should be "$sum".
Another commit ("update auto-install/defaultroot for stretch") bumps
the changelog version from 1.73 to 1.76. Huh?
The last commit ("add a hook for looking up preseed checksums") does:
+if [ "$LOOKUP_CHECKSUM" = 1 ] ; then
+ # this is yet to be written, so leave room for dropping it into place during preseeding
+ if [ -f /bin/preseed_lookup_checksum ] ; then
+ # the "$@" parameter should be empty, but we might as well pass it in, so that we can pass it back out if need be
+ set -- $(/bin/preseed_lookup_checksum "$src" "$@")
+ fi
+fi
which seems to mean that when the -C option is used and
/bin/preseed_lookup_checksum is missing then checksums are not
verified. Shouldn't preseed_fetch fail in this case?
[...]
> P.S. I think this would be better if it were using SHA>=265, but
> we're
still on MD5SUM elsewhere in the code, so I guess that will need to
> wait.
I agree, MD5 should not be relied on for more than error detection.
Ben.
--
Ben Hutchings
The world is coming to an end. Please log off.
Attachment:
signature.asc
Description: This is a digitally signed message part