From: "Michał Górny" <mgorny@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Subject: Re: [gentoo-dev] [PATCH 2/3] verify-sig.eclass: Support `openssl dgst` format checksums
Date: Fri, 08 Sep 2023 12:03:47 +0200 [thread overview]
Message-ID: <d9ab2b383e69e171280483bd731c3fb09e677c1b.camel@gentoo.org> (raw)
In-Reply-To: <ua5u2o2wu@gentoo.org>
On Mon, 2023-09-04 at 08:42 +0200, Ulrich Mueller wrote:
> > > > > > On Mon, 04 Sep 2023, Michał Górny wrote:
>
> > --- a/eclass/verify-sig.eclass
> > +++ b/eclass/verify-sig.eclass
> > @@ -214,12 +214,15 @@ verify-sig_verify_message() {
> > }
>
>
>
>
> > # @FUNCTION: verify-sig_verify_unsigned_checksums
> > -# @USAGE: <checksum-file> <algo> <files>
> > +# @USAGE: <checksum-file> <format> <files>
>
> Below, verify-sig_verify_signed_checksums() still says "algo", change
> that too for consistency?
If I must.
>
> > # @DESCRIPTION:
> > # Verify the checksums for all files listed in the space-separated list
> > -# <files> (akin to ${A}) using a <checksum-file>. <algo> specifies
> > -# the checksum algorithm (e.g. sha256). <checksum-file> can be "-"
> > -# for stdin.
> > +# <files> (akin to ${A}) using a <checksum-file>. <format> specifies
> > +# the checksum file format. <checksum-file> can be "-" for stdin.
> > +#
> > +# The following formats are supported:
> > +# - sha256 -- sha256sum (<hash> <filename>)
> > +# - openssl-dgst -- openssl dgst (<algo>(<filename>)=<hash>)
>
> This won't be rendered as a list in the man page, but will be rewrapped
> as a paragraph. (Putting a space before the "-" will help.)
>
> The existing variable documentation of VERIFY_SIG_METHOD suffers from
> the same problem, BTW.
Hmm, I can't get it to work with the space either. Whatever I do,
eclass-to-manpage.awk seems to copy it verbatim.
>
> > #
> > # The function dies if one of the files does not match checksums or
> > # is missing from the checksum file.
> > @@ -234,32 +237,46 @@ verify-sig_verify_unsigned_checksums() {
> > local algo=${2}
>
> Maybe rename the variable to "format", when the documentation now says
> that the second parameter specifies the format?
>
> > local files=()
> > read -r -d '' -a files <<<"${3}"
> > - local chksum_prog chksum_len
> > + local chksum_prog chksum_len format=coreutils
>
> And rename this one too. (I don't find it intuitive for a checksum
> format to be named "coreutils", when coreutils provides cksum, md5sum,
> b2sum, etc.)
>
> >
> > case ${algo} in
> > sha256)
> > - chksum_prog=sha256sum
> > chksum_len=64
> > ;;
> > + openssl-dgst)
> > + format=${algo}
> > + ;;
> > *)
> > - die "${FUNCNAME}: unknown checksum algo ${algo}"
> > + die "${FUNCNAME}: unknown checksum format ${algo}"
> > ;;
> > esac
> >
> > [[ ${checksum_file} == - ]] && checksum_file=/dev/stdin
> > - local checksum filename junk ret=0 count=0
> > - while read -r checksum filename junk; do
> > - if [[ ${checksum} == "-----BEGIN" ]]; then
> > + local line checksum filename junk ret=0 count=0
> > + while read -r line; do
> > + if [[ ${line} == "-----BEGIN"* ]]; then
> > die "${FUNCNAME}: PGP armor found, use verify-sig_verify_signed_checksums instead"
> > fi
> >
> > - [[ ${#checksum} -eq ${chksum_len} ]] || continue
> > - [[ -z ${checksum//[0-9a-f]} ]] || continue
> > - has "${filename}" "${files[@]}" || continue
> > - [[ -z ${junk} ]] || continue
> > -
> > - "${chksum_prog}" -c --strict - <<<"${checksum} ${filename}"
> > - if [[ ${?} -eq 0 ]]; then
> > + case ${format} in
> > + coreutils)
> > + read -r checksum filename junk <<<"${line}"
> > + [[ ${#checksum} -ne ${chksum_len} ]] && continue
> > + [[ -n ${checksum//[0-9a-f]} ]] && continue
> > + [[ -n ${junk} ]] && continue
> > + ;;
> > + openssl-dgst)
> > + [[ ${line} != *"("*")="* ]] && continue
> > + checksum=${line##*)=}
> > + algo=${line%%(*}
> > + filename=${line#*(}
> > + filename=${filename%)=*}
> > + ;;
> > + esac
> > +
> > + ! has "${filename}" "${files[@]}" && continue
>
> This might be clearer if it was written as:
>
> has "${filename}" "${files[@]}" || continue
Negative logic is never clearer.
>
> > +
> > + if "${algo,,}sum" -c --strict - <<<"${checksum} ${filename}"; then
> > (( count++ ))
> > else
> > ret=1
--
Best regards,
Michał Górny
next prev parent reply other threads:[~2023-09-08 10:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-04 1:55 [gentoo-dev] [PATCH 0/3] verify-sig.eclass: `openssl dgst` format support Michał Górny
2023-09-04 1:55 ` [gentoo-dev] [PATCH 1/3] eclass/tests: Add initial tests for verify-sig Michał Górny
2023-09-04 1:55 ` [gentoo-dev] [PATCH 2/3] verify-sig.eclass: Support `openssl dgst` format checksums Michał Górny
2023-09-04 6:42 ` Ulrich Mueller
2023-09-08 10:03 ` Michał Górny [this message]
2023-09-08 10:06 ` Michał Górny
2023-09-08 13:06 ` Ulrich Mueller
2023-09-04 1:55 ` [gentoo-dev] [PATCH 3/3] verify-sig.eclass: Fix handling multiple/duplicate signatures Michał Górny
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d9ab2b383e69e171280483bd731c3fb09e677c1b.camel@gentoo.org \
--to=mgorny@gentoo.org \
--cc=gentoo-dev@lists.gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox