[pacman-dev] [PATCH] makepkg: Fix whirlpoolsums support
From: Luke Shumaker <lukeshu@parabola.nu> Commit 9cdfd187 introduced support for whirlpool checksums in v5.0.0. However, it was sloppy and missed several places where the list of checksums is used. So fix that. In several places, we can take advantage of the 'known_hash_algos' variable to simplify things a bit. Commit 57770125 switched from using OpenSSL to GNU coreutils for doing the checksums in v5.1.0. This broke the whirlpool support, as coreutils does not implement a 'whirlpoolsum' program. So go back to using openssl for whirlpool sums only. --- I'm not particularly attached to whirlpool support, and if your reaction is "let's formally drop whirlpool", I wouldn't be upset by that. A handful (15) of Parabola's PKGBUILDs use whirlpoolsums, which makes sense, because the author if the original whirlpoolsums commit is a Parabola contributor. But, if you want to drop whirlpool, I have no problem saying that those packages need to migrate to a different checksum algorithm at their next update. doc/PKGBUILD.5.asciidoc | 2 +- etc/makepkg.conf.in | 2 +- .../integrity/generate_checksum.sh.in | 12 +++++++++-- .../integrity/verify_checksum.sh.in | 13 ++++++++++-- .../libmakepkg/lint_pkgbuild/variable.sh.in | 4 ++-- scripts/libmakepkg/srcinfo.sh.in | 4 ++-- scripts/makepkg.sh.in | 20 +++++++++++++------ 7 files changed, 41 insertions(+), 16 deletions(-) diff --git a/doc/PKGBUILD.5.asciidoc b/doc/PKGBUILD.5.asciidoc index f12effde..5715c7aa 100644 --- a/doc/PKGBUILD.5.asciidoc +++ b/doc/PKGBUILD.5.asciidoc @@ -154,7 +154,7 @@ contain whitespace characters. be skipped. To easily generate md5sums, run ``makepkg -g >> PKGBUILD''. If desired, move the md5sums line to an appropriate location. -*sha1sums, sha224sums, sha256sums, sha384sums, sha512sums (arrays)*:: +*sha1sums, sha224sums, sha256sums, sha384sums, sha512sums, whirlpoolsums (arrays)*:: Alternative integrity checks that makepkg supports; these all behave similar to the md5sums option described above. To enable use and generation of these checksums, be sure to set up the `INTEGRITY_CHECK` option in diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in index b0fdea9e..22d298f7 100644 --- a/etc/makepkg.conf.in +++ b/etc/makepkg.conf.in @@ -87,7 +87,7 @@ BUILDENV=(!distcc color !ccache check !sign) # OPTIONS=(strip docs libtool staticlibs emptydirs zipman purge !debug) -#-- File integrity checks to use. Valid: md5, sha1, sha224, sha256, sha384, sha512 +#-- File integrity checks to use. Valid: md5, sha1, sha224, sha256, sha384, sha512, whirlpool INTEGRITY_CHECK=(md5) #-- Options to be used when stripping binaries. See `man strip' for details. STRIP_BINARIES="@STRIP_BINARIES@" diff --git a/scripts/libmakepkg/integrity/generate_checksum.sh.in b/scripts/libmakepkg/integrity/generate_checksum.sh.in index eb9b74fc..7480c0ee 100644 --- a/scripts/libmakepkg/integrity/generate_checksum.sh.in +++ b/scripts/libmakepkg/integrity/generate_checksum.sh.in @@ -59,8 +59,16 @@ generate_one_checksum() { if [[ ${netfile%%::*} != *.@(sig?(n)|asc) ]]; then local file file="$(get_filepath "$netfile")" || missing_source_file "$netfile" - sum="$("${integ}sum" "$file")" - sum=${sum%% *} + case "$integ" in + md5|sha1|sha224|sha256|sha384|sha512) + sum="$("${integ}sum" "$file")" + sum=${sum%% *} + ;; + whirlpool) + sum="$(openssl dgst -${integ} "$file")" + sum=${sum##* } + ;; + esac else sum="SKIP" fi diff --git a/scripts/libmakepkg/integrity/verify_checksum.sh.in b/scripts/libmakepkg/integrity/verify_checksum.sh.in index 532e0693..6cd30ab4 100644 --- a/scripts/libmakepkg/integrity/verify_checksum.sh.in +++ b/scripts/libmakepkg/integrity/verify_checksum.sh.in @@ -82,8 +82,17 @@ verify_integrity_one() { return 1 fi - local realsum="$("${integ}sum" "$file")" - realsum="${realsum%% *}" + local realsum + case "$integ" in + md5|sha1|sha224|sha256|sha384|sha512) + realsum="$("${integ}sum" "$file")" + realsum="${realsum%% *}" + ;; + whirlpool) + realsum="$(openssl dgst -${integ} "$file")" + realsum=${realsum##* } + ;; + esac if [[ ${expectedsum,,} = "$realsum" ]]; then printf '%s\n' "$(gettext "Passed")" >&2 else diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in index 5f783e83..63c4955b 100644 --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in @@ -33,8 +33,8 @@ lint_variable() { # TODO: refactor - similar arrays are used elsewhere local array=(arch backup checkdepends groups license noextract options validpgpkeys) - local arch_array=(conflicts depends makedepends md5sums optdepends provides - replaces sha1sums sha224sums sha256sums sha384sums sha512sums + local arch_array=(conflicts depends makedepends optdepends provides + replaces "${known_hash_algos[@]/%/sums}" source) local string=(changelog epoch install pkgbase pkgdesc pkgrel pkgver url) diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 509c4860..a97cdde5 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -63,7 +63,7 @@ srcinfo_write_section_details() { local attr package_arch a local multivalued_arch_attrs=(source provides conflicts depends replaces optdepends makedepends checkdepends - {md5,sha{1,224,256,384,512}}sums) + "${known_hash_algos[@]/%/sums}") for attr in "${singlevalued[@]}"; do pkgbuild_extract_to_srcinfo "$1" "$attr" 0 @@ -89,7 +89,7 @@ srcinfo_write_global() { local multivalued=(arch groups license checkdepends makedepends depends optdepends provides conflicts replaces noextract options backup - source validpgpkeys {md5,sha{1,224,256,384,512}}sums) + source validpgpkeys "${known_hash_algos[@]/%/sums}") srcinfo_open_section 'pkgbase' "${pkgbase:-$pkgname}" srcinfo_write_section_details '' diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2ac1bbca..92a0b692 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1017,10 +1017,18 @@ check_software() { local integlist IFS=$'\n' read -rd '' -a integlist < <(get_integlist) - local integ + local integ binary for integ in "${integlist[@]}"; do - if ! type -p "${integ}sum" >/dev/null; then - error "$(gettext "Cannot find the %s binary required for source file checksums operations.")" "${integ}sum" + case "$integ" in + md5|sha1|sha224|sha256|sha384|sha512) + binary="${integ}sum" + ;; + whirlpool) + binary=openssl + ;; + esac + if ! type -p "$binary" >/dev/null; then + error "$(gettext "Cannot find the %s binary required for source file checksums operations.")" "$binary" ret=1 fi done @@ -1416,12 +1424,12 @@ else fi unset pkgname pkgbase pkgver pkgrel epoch pkgdesc url license groups provides -unset md5sums replaces depends conflicts backup source install changelog build -unset sha{1,224,256,384,512}sums makedepends optdepends options noextract validpgpkeys +unset replaces depends conflicts backup source install changelog build +unset "${known_hash_algos[@]/%/sums}" makedepends optdepends options noextract validpgpkeys unset "${!makedepends_@}" "${!depends_@}" "${!source_@}" "${!checkdepends_@}" unset "${!optdepends_@}" "${!conflicts_@}" "${!provides_@}" "${!replaces_@}" unset "${!md5sums_@}" "${!sha1sums_@}" "${!sha224sums_@}" "${!sha256sums_@}" -unset "${!sha384sums_@}" "${!sha512sums_@}" +unset "${!sha384sums_@}" "${!sha512sums_@}" "${!whirlpoolsums_@}" BUILDFILE=${BUILDFILE:-$BUILDSCRIPT} if [[ ! -f $BUILDFILE ]]; then -- 2.18.0
On 8/27/18 4:02 PM, Luke Shumaker wrote:
From: Luke Shumaker <lukeshu@parabola.nu>
Commit 9cdfd187 introduced support for whirlpool checksums in v5.0.0. However, it was sloppy and missed several places where the list of checksums is used. So fix that. In several places, we can take advantage of the 'known_hash_algos' variable to simplify things a bit.
Commit 57770125 switched from using OpenSSL to GNU coreutils for doing the checksums in v5.1.0. This broke the whirlpool support, as coreutils does not implement a 'whirlpoolsum' program. So go back to using openssl for whirlpool sums only. --- I'm not particularly attached to whirlpool support, and if your reaction is "let's formally drop whirlpool", I wouldn't be upset by that.
A handful (15) of Parabola's PKGBUILDs use whirlpoolsums, which makes sense, because the author if the original whirlpoolsums commit is a Parabola contributor. But, if you want to drop whirlpool, I have no problem saying that those packages need to migrate to a different checksum algorithm at their next update. Huh, and we never documented that we supported it in the first place. :/
No wonder we didn't notice that this would break, and, equally, no wonder users didn't hit this in the 2.5 years since 5.0.0 was tagged... But, if we're going to support whirlpool then that means, going against the original intent of the patch which broke this, that we now need the openssl command-line tool even if built --with-crypto=nettle, because it doesn't look like nettle supports whirlpool any more than base64. -- Eli Schwartz Bug Wrangler and Trusted User
On Mon, 27 Aug 2018 16:17:31 -0400, Eli Schwartz wrote:
Huh, and we never documented that we supported it in the first place. :/
It did show up in the NEWS file, but no where else.
No wonder we didn't notice that this would break, and, equally, no wonder users didn't hit this in the 2.5 years since 5.0.0 was tagged...
But, if we're going to support whirlpool then that means, going against the original intent of the patch which broke this, that we now need the openssl command-line tool even if built --with-crypto=nettle, because it doesn't look like nettle supports whirlpool any more than base64.
It should only need the openssl command-line tool if it's actually going to use whirlpool for a given PKGBUILD (i.e., and optdepend at best). -- Happy hacking, ~ Luke Shumaker
On 28/08/18 06:02, Luke Shumaker wrote:
I'm not particularly attached to whirlpool support, and if your reaction is "let's formally drop whirlpool", I wouldn't be upset by that.
That is my reaction. A
This reverts commit 9cdfd18739cc4b0e2b2efeb9a92a3ea612c8505f. We've never documented whirlpoolsums support in the manpage and no one really seems to have realized we support it, let alone use it -- except for a few parabola packages, being the contributor's motivation for adding support. The problem is that for two years the code has been broken. In commit 577701250d645d1fc1a505cde34aedbeb3208ea5 we moved to coreutils to provide checksum commands, rather than openssl, but there is no whirlpoolsums binary. Properly fixing this would require re-adding a dependency on openssl, independent of the libalpm crypto backend -- which defeats the purpose of moving to coreutils in the general case. nettle-hash does not provide a whirlpool algorithm any more than it does base64 (the original reason for moving to coreutils). Therefore, we should just drop support for this again. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cf5fda91..f9c619f2 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -54,7 +54,7 @@ splitpkg_overrides=('pkgdesc' 'arch' 'url' 'license' 'groups' 'depends' 'options' 'install' 'changelog') readonly -a build_options splitpkg_overrides -known_hash_algos=('md5' 'sha1' 'sha224' 'sha256' 'sha384' 'sha512' 'whirlpool') +known_hash_algos=('md5' 'sha1' 'sha224' 'sha256' 'sha384' 'sha512') # Options ASDEPS=0 -- 2.18.0
participants (3)
-
Allan McRae
-
Eli Schwartz
-
Luke Shumaker