On Thu, 2024-11-28 at 10:36 -0500, Eli Schwartz wrote: > On 11/28/24 8:10 AM, Michał Górny wrote: > > > > +# separated by colons. The allowed values for a location are: > > > +# > > > +# - gentoo -- fetch key by fingerprint from https://keys.gentoo.org > > > +# > > > +# - github -- fetch key from github.com/${name}.pgp > > > +# > > > +# - openpgp -- fetch key by fingerprint from https://keys.openpgp.org > > > +# > > > +# - ubuntu -- fetch key by fingerprint from http://keyserver.ubuntu.com > > > (the default) > > > > I'd go without a default. Typing 6 more letters doesn't cost anything, > > and makes the contents more consistent. Also saves us from regretting > > having chosen a bad default in the future. > > > > > +# > > > +# - none -- do not add to SRC_URI, the ebuild will provide a custom > > > download location > > > > Perhaps "manual"? "None" sounds like there would be no key at all. > > > Maybe I could just document as a recommendation to use ubuntu. Other > sources are likely to be extremely unreliable, unfortunately. > openpgp.org only works if the key owner manually verifies their email, > for example.
Yeah. I don't see why anyone would specify a non-working location (such
as openpgp.org).
> > > + # new key
> > > + found=0
> > > + elif [[ ${found} = 0 && ${line[0]} = fpr ]]; then
> > > + # primary fingerprint
> > > + imported_keys+=("${line[9]}")
> > > + found=1
> > > + fi
> > > + done < <(gpg --batch --list-keys --keyid-format=long --with-colons ||
> > > die)
> >
> > Why do you need --keyid-format? You're using fingerprints only, aren't
> > you?
>
>
> I'm used to it mattering in various contexts and added it instinctively.
> You're right, it doesn't do anything here.
Yeah. And for a minute, it made me worry that you're using long ids
instead of fingerprints.
> > > + fi
> > > + if [[ ${#missing_keys[@]} != 0 ]]; then
> > > + die "too few keys found. Unavailable keys: ${missing_keys[@]}"
> > > + fi
> > > +}
> > > +
> > > +
> > > +sec-keys_src_test() {
> > > + local -x GNUPGHOME=${WORKDIR}/gnupg
> > > + local key fingerprint name server
> > > + local gpg_command=(gpg --export-options export-minimal)
> > > +
> > > + for fingerprint in "${SEC_KEYS_VALIDPGPKEYS[@]%%:*}"; do
> > > + "${gpg_command[@]}" --export "${fingerprint}" | pgpdump >
> > > "${fingerprint}.pgpdump" || die
> > > + done
> > > +
> > > + # Best-effort attempt to check for updates. keyservers can and usually
> > > do
> > > + # fail for weird reasons, (such as being unable to import a key without
> > > a
> > > + # uid) as well as normal reasons, like the key being exclusive to a
> > > + # different keyserver. this isn't a reason to fail src_test.
> >
> > Well, I dare say that if refreshing against the server specified
> > as the reference source fails, that would count as a reason to fail.
> > Consider the case of someone removing a compromised key instead
> > of revoking it.
>
>
> This doesn't test a useful property.
>
> People cannot "remove" compromised keys from a keyserver to begin with.
> If they did, then checking to build the package with GENTOO_MIRRORS=
> DISTDIR=$(mktemp -d) is a significantly more useful test.
In fact, this is the kind of test I was originally thinking of -- i.e.
literally refetching the file manually and seeing if the minimal export
differs...
> > > +sec-keys_src_install() {
> > > + local -x GNUPGHOME=${WORKDIR}/gnupg
> > > + local fingerprint
> > > + local gpg_command=(gpg --no-permission-warning --export-options
> > > export-minimal)
> > > +
> > > + for fingerprint in "${SEC_KEYS_VALIDPGPKEYS[@]%%:*}"; do
> > > + local uids=()
> > > + mapfile -t uids < <("${gpg_command[@]}" --list-key
> > > --with-colons ${fingerprint} | awk -F: '/^uid/{print $10}' || die)
> > > + edo "${gpg_command[@]}" "${uids[@]/#/--comment=}" --export
> > > --armor "${fingerprint}" >> ${PN#openpgp-keys-}.asc
> > > + done
> >
> > That looks like something you could do in src_compile() already.
>
>
> Perhaps. But it felt like exporting keys is work that is conceptually
> part of installing, in much the way that running a meson project's
> `meson install` step does more than just copy files into ${D} -- it also
> processes those files in order to do things like patch the rpath.
>
> I guess I am not too attached to either approach.
>
>
> > Also, I'm confused by the purpose of this whole logic. After all, you
> > have already verified that there are no stray keys in the keyring,
> > right? So why not just export the whole thing?
>
>
> Because this is doing additional steps that aren't just exporting the
> whole thing?
>
Then perhaps you should add an explanatory comment instead of expecting
people to catch that from a command so long it doesn't fit on people's
screens.
--
Best regards,
Michał Górny
signature.asc
Description: This is a digitally signed message part
