Bug#851774: [pkg-gnupg-maint] Bug#851774: Stop using apt-key add to add keys in generators/60local

[David Kalnischkies]

On Sun, Feb 05, 2017 at 12:23:19AM -0500, Daniel Kahn Gillmor wrote:
> On Sat 2017-02-04 19:48:54 -0500, Cyril Brulebois wrote:
> > [ dkg wrote: ]
> >> Regardless of the choice of filesystem location (fragment directory or
> >> elsewhere), gpgv does want to see the curated keyrings it depends on
> >> in binary format, so on to the next bit:
> >
> > I'm a bit confused here: apt-get update (in a sid chroot, not attempted
> > in d-i) is fine with an armor key in the fragment directory; are you
> > saying that using the Signed-by option for sources.list would mean
> > having to have a (curated) keyring, and an non-armored version, hence
> > the need for the transformation you're suggesting below?
> 
> Sorry, i guess it's possible that apt is doing something fancier that i
> don't know about, then.
> 
> gpgv on its own expects the --keyring files it encounters to be either a
> sequence of raw OpenPGP packets that together form a series of OpenPGP
> certificates (a.k.a. "a keyring") or GnuPG's "keybox" format.  AFAIK,
> gpgv does not accept ascii-armored files for its --keyring argument.
> 
> maybe the apt folks can weight in on what's going on with armored
> fragments?  If it's converting them before handing them off to gpgv,
> maybe you can just count on it to convert the files that aren't in the
> fragment directory as well?

apt >= 1.4 uses basically the awk snippet (it is slightly more complex
to deal with two or more armor keys in one file, but that is implemented
more for our testcases than a real external requirement) [see apt-key
for implementation details].

Note that you can NOT use files in keybox format in exchange as apt
merges all keyrings into a big one with 'cat' to avoid both a dependency
on gnupg and to avoid running into limits on the amount of keyring files
(gpg has a limit of 40 keyring files in a single invocation – and there
is always the looming threat of having that be 1 one day…).

So, as long as you make it so that an armored file has the extension
'asc' and binary (OpenPGP packet) file has 'gpg' apt will do the right
thing with them in the fragment directory just as well as in Signed-By
[in stretch, but Signed-By is a new-in-stretch feature, too].


> > Remember we're talking about adding extra repositories with custom d-i
> > configuration, so I'm fine with people having broken stuff because they
> > pasted a whole mail…
> 
> agreed, we can expect these folks to get the details right.

For the same reason I wouldn't worry too much about people using *.asc
files with binary format contents and vice versa to be honest.


Best regards

David Kalnischkies


signature.asc
Description: PGP signature

Bug#851774: [pkg-gnupg-maint] Bug#851774: Stop using apt-key add to add keys in generators/60local

[Daniel Kahn Gillmor]

On Sat 2017-02-04 19:48:54 -0500, Cyril Brulebois wrote:
> I'm fine with (discovering and) following best practices.

:)

> [ dkg wrote: ]
>> Regardless of the choice of filesystem location (fragment directory or
>> elsewhere), gpgv does want to see the curated keyrings it depends on
>> in binary format, so on to the next bit:
>
> I'm a bit confused here: apt-get update (in a sid chroot, not attempted
> in d-i) is fine with an armor key in the fragment directory; are you
> saying that using the Signed-by option for sources.list would mean
> having to have a (curated) keyring, and an non-armored version, hence
> the need for the transformation you're suggesting below?

Sorry, i guess it's possible that apt is doing something fancier that i
don't know about, then.

gpgv on its own expects the --keyring files it encounters to be either a
sequence of raw OpenPGP packets that together form a series of OpenPGP
certificates (a.k.a. "a keyring") or GnuPG's "keybox" format.  AFAIK,
gpgv does not accept ascii-armored files for its --keyring argument.

maybe the apt folks can weight in on what's going on with armored
fragments?  If it's converting them before handing them off to gpgv,
maybe you can just count on it to convert the files that aren't in the
fragment directory as well?

> Remember we're talking about adding extra repositories with custom d-i
> configuration, so I'm fine with people having broken stuff because they
> pasted a whole mail…

agreed, we can expect these folks to get the details right.

> No awk in d-i, so I'll go with the strict version and we'll see if we
> have users who could complain and why.

bummer, no awk!

> Depending on answers to various questions above, we'll see about adding
> new applets to busybox if needed.

I hope you saw my followup using uudecode instead of base64.  However,
it's still awk-dependent. :/

 --dkg


signature.asc
Description: PGP signature

Bug#851774: [pkg-gnupg-maint] Bug#851774: Stop using apt-key add to add keys in generators/60local

[Cyril Brulebois]

Hi Daniel,

Daniel Kahn Gillmor  (2017-02-04):
> On Sat 2017-02-04 18:25:52 -0500, Cyril Brulebois wrote:
> 
> > I'm adding gnupg maintainers in cc since they might have interesting
> > tips for the implementation. Context: we need to replace apt-key add
> > calls with dropping files under the appropriate names for extra
> > repositories in a Debian Installer context.
> 
> Thanks for the Cc, KiBi.
> 
> I think that extra repositories should *not* have their keys added to
> /etc/apt/trusted.gpg.d/*.gpg ("the fragment directory") by default,
> since that authorizes the extra key to make valid signatures for the
> main archive.
> 
> If the extra repo has its own key, it should be authorized to make
> signatures only for the extra repo, and nothing else (similarly, the
> official debian archive keys *shouldn't* be authorized to make
> signatures for the extra repo).
> 
> So if we're talking about adding extra repositories for a debian stretch
> installer, as i said over on #853858:
> 
>   for Debian 9 ("stretch") and later, you should place these keys (in
>   binary form) someplace within /usr/local/share/keyrings/ and add a
>   "Signed-By:" option to the relevant apt sources (see sources.list(5)).
> 
> Does that strategy sound right overall to the rest of you?

I'm fine with (discovering and) following best practices.

> Regardless of the choice of filesystem location (fragment directory or
> elsewhere), gpgv does want to see the curated keyrings it depends on
> in binary format, so on to the next bit:

I'm a bit confused here: apt-get update (in a sid chroot, not attempted
in d-i) is fine with an armor key in the fragment directory; are you
saying that using the Signed-by option for sources.list would mean
having to have a (curated) keyring, and an non-armored version, hence
the need for the transformation you're suggesting below?

> > so I think we need to have some kind of autodetection code. gnupg
> > maintainers: is grepping for “BEGIN PGP PUBLIC KEY BLOCK” enough to
> > decide between armored and non-armored? Or do you have any better
> > solutions?
> 
> If the keyring is non-armored, i assume that we're just going to try
> to use it as-is, without transformation.  So the question is: which
> incoming keys do we want to try to transform?
> 
> I'd err on the strict side and say that we really only want files that
> contain nothing but a public key block.  That is, if there's any
> garbage text before the ASCII-armored header, we probably want to
> reject the file rather than trying to transform it.  This strictness
> avoids tripping up in really bizarre corner cases (like if someone
> provides an non-armored key that contains a notation, uid, uat, or
> other embedded data that itself has the string "BEGIN PGP PUBLIC KEY
> BLOCK" in it).  I can cook up such a perversity if it would make
> anyone happy ;)
> 
> The strictness does mean that people who'd, say, copied and pasted an
> entire e-mail message that includes a key and expected it to JustWork™
> might be disappointed, but i might be OK with that.  Being clean about
> what's in your repo keyrings is a habit we want to cultivate.

Remember we're talking about adding extra repositories with custom d-i
configuration, so I'm fine with people having broken stuff because they
pasted a whole mail…

> If you agree with being strict, then the following pipeline should
> return 0 if the keyring is ASCII-armored:
> 
> head -n1 | grep -Fxq -e '-BEGIN PGP PUBLIC KEY BLOCK-'

I might not have used all those flags but that's what I had in mind,
thanks. :)

> If you want to be a little less strict and permit arbitrary whitespace
> before the block, you could do:
> 
> awk '/[^[:space:]]/{ print $0; exit }' | grep -Fxq -e '-BEGIN PGP 
> PUBLIC KEY BLOCK-'

No awk in d-i, so I'll go with the strict version and we'll see if we
have users who could complain and why.

> I've tested and both of these pipelines appear to work with their
> busybox variants using busybox 1:1.22.0-19+b1 on amd64.

Thanks for checking but… nice try. ;-)

> So if you're OK with that test, then you need the transformation:
> 
> Over in #831409, i mentioned this simple pipeline to perform the actual
> transformation:
> 
>  awk '/^$/{ x = 1; } /^[^=-]/{ if (x) { print $0; } ; }' | base64 -d
> 
> Unfortunately, it looks to me like busybox doesn't offer a base64 applet
> at the moment, which would otherwise allow d-i to do the de-armoring
> entirely with busybox.  I could probably knock that applet together if
> people want it -- it looks like busybox already has b64 subroutines in
> it.

Depending on answers to various questions above, we'll see about adding
new applets to busybox if needed.

> Hope this helps!  I'm happy to follow up on it with you.

It definitely does (even if that means some more work than I initially
anticipated, but heh, I was asking for it! :)), thanks!


KiBi.


signature.asc
Description: Digital signature

Bug#851774: [pkg-gnupg-maint] Bug#851774: Stop using apt-key add to add keys in generators/60local

[Daniel Kahn Gillmor]

On Sat 2017-02-04 19:35:58 -0500, Daniel Kahn Gillmor wrote:
> Over in #831409, i mentioned this simple pipeline to perform the actual
> transformation:
>
>  awk '/^$/{ x = 1; } /^[^=-]/{ if (x) { print $0; } ; }' | base64 -d
>
> Unfortunately, it looks to me like busybox doesn't offer a base64 applet
> at the moment, which would otherwise allow d-i to do the de-armoring
> entirely with busybox.  I could probably knock that applet together if
> people want it -- it looks like busybox already has b64 subroutines in
> it.

ah, no need for a base64 applet, since busybox has a uudecode applet.

using only busybox, the following pipeline should be fine for
de-armoring:

awk 'BEGIN{ print "begin-base64 644 -" } /^$/{ x = 1; } /^[^=-]/{ if (x) { 
print $0; } ; } END{ print "" }' | uudecode

hth,

--dkg


signature.asc
Description: PGP signature

Bug#851774: [pkg-gnupg-maint] Bug#851774: Stop using apt-key add to add keys in generators/60local

[Daniel Kahn Gillmor]

Hi all--

On Sat 2017-02-04 18:25:52 -0500, Cyril Brulebois wrote:

> I'm adding gnupg maintainers in cc since they might have interesting
> tips for the implementation. Context: we need to replace apt-key add
> calls with dropping files under the appropriate names for extra
> repositories in a Debian Installer context.

Thanks for the Cc, KiBi.

I think that extra repositories should *not* have their keys added to
/etc/apt/trusted.gpg.d/*.gpg ("the fragment directory") by default,
since that authorizes the extra key to make valid signatures for the
main archive.

If the extra repo has its own key, it should be authorized to make
signatures only for the extra repo, and nothing else (similarly, the
official debian archive keys *shouldn't* be authorized to make
signatures for the extra repo).

So if we're talking about adding extra repositories for a debian stretch
installer, as i said over on #853858:

  for Debian 9 ("stretch") and later, you should place these keys (in
  binary form) someplace within /usr/local/share/keyrings/ and add a
  "Signed-By:" option to the relevant apt sources (see sources.list(5)).

Does that strategy sound right overall to the rest of you?

Regardless of the choice of filesystem location (fragment directory or
elsewhere), gpgv does want to see the curated keyrings it depends on in
binary format, so on to the next bit:

> so I think we need to have some kind of autodetection code. gnupg
> maintainers: is grepping for “BEGIN PGP PUBLIC KEY BLOCK” enough to
> decide between armored and non-armored? Or do you have any better
> solutions?

If the keyring is non-armored, i assume that we're just going to try to
use it as-is, without transformation.  So the question is: which
incoming keys do we want to try to transform?

I'd err on the strict side and say that we really only want files that
contain nothing but a public key block.  That is, if there's any garbage
text before the ASCII-armored header, we probably want to reject the
file rather than trying to transform it.  This strictness avoids
tripping up in really bizarre corner cases (like if someone provides an
non-armored key that contains a notation, uid, uat, or other embedded
data that itself has the string "BEGIN PGP PUBLIC KEY BLOCK" in it).  I
can cook up such a perversity if it would make anyone happy ;)

The strictness does mean that people who'd, say, copied and pasted an
entire e-mail message that includes a key and expected it to JustWork™
might be disappointed, but i might be OK with that.  Being clean about
what's in your repo keyrings is a habit we want to cultivate.

If you agree with being strict, then the following pipeline should
return 0 if the keyring is ASCII-armored:

head -n1 | grep -Fxq -e '-BEGIN PGP PUBLIC KEY BLOCK-'

If you want to be a little less strict and permit arbitrary whitespace
before the block, you could do:

awk '/[^[:space:]]/{ print $0; exit }' | grep -Fxq -e '-BEGIN PGP 
PUBLIC KEY BLOCK-'

I've tested and both of these pipelines appear to work with their
busybox variants using busybox 1:1.22.0-19+b1 on amd64.

So if you're OK with that test, then you need the transformation:

Over in #831409, i mentioned this simple pipeline to perform the actual
transformation:

 awk '/^$/{ x = 1; } /^[^=-]/{ if (x) { print $0; } ; }' | base64 -d

Unfortunately, it looks to me like busybox doesn't offer a base64 applet
at the moment, which would otherwise allow d-i to do the de-armoring
entirely with busybox.  I could probably knock that applet together if
people want it -- it looks like busybox already has b64 subroutines in
it.

Hope this helps!  I'm happy to follow up on it with you.

--dkg


signature.asc
Description: PGP signature

Bug#851774: Stop using apt-key add to add keys in generators/60local

[Cyril Brulebois]

Hi,

I'm adding gnupg maintainers in cc since they might have interesting
tips for the implementation. Context: we need to replace apt-key add
calls with dropping files under the appropriate names for extra
repositories in a Debian Installer context.

Julian Andres Klode  (2017-01-21):
> > On 18.01.2017 17:43, Marga Manterola wrote:
> > > For a long time it's been possible to preseed a local repository that
> > > has it's own keyring. However, with the latest changes related to gpg
> > > dependencies getting dropped in apt, this is no longer possible.
> > > 
> > > I'm setting severity as serious as adviced by Julien Cristau on IRC.
> > > With the current state of things, in order to install a local repository
> > > with a keyring the user needs to somehow create a script that will put
> > > the keyring in place before 60local runs, and not preseed the keyring at
> > > all.  If the keyring is preseeded, *the whole installation will fail*
> > > because apt-key add fails which causes 60local to fail, which causes the
> > > install base system step to fail.
> > > 
> > > This is the offending code:
> > > https://sources.debian.net/src/apt-setup/1:0.123/generators/60local/#L33
> > > 
> > > This is using the deprecated apt-key add functionality.  From the
> > > apt-key manpage:
> > > 
> > > COMMANDS
> > >add filename
> > > (...)
> > >Note: Instead of using this command a keyring should be
> > > placed directly in the /etc/apt/trusted.gpg.d/ directory with a
> > > descriptive name and either "gpg" or "asc" as file extension.
> > > 
> > > So, the right thing to do is to copy the file to the right path instead
> > > of calling apt-key add with it.
> > 
> > Does that mean that we actually have to infer (check using grep?) if the
> > file is armored or not? I think `apt-key add' just dealt with whatever
> > it got and put the key into the keyring using gpg's --import function.
> > So it's a little unfortunate that we'd now need to know the format of
> > what we need to put into the fragment directory.
> 
> AFAIK. If it is armored, it must be called .asc, if not it must be
> called .gpg. Armored files are supported since 1.4~beta1, that is,
> starting with stretch and Ubuntu zesty. GPG might also support
> unarmored content in .asc files (at least it does --unarmor with
> an unarmored file).
> 
> So, the answer is probably yes, but you could try out just
> storing everything in .asc files and it might work (but that's
> a bit ugly...).

I've tested with a custom repository and a sid chroot, and results are
as follows when only the said file has been added to trusted.gpg.d/:
| C2B35520.asc with asc contents: OK
| C2B35520.asc with gpg contents: KO
| C2B35520.gpg with gpg contents: OK
| C2B35520.gpg with asc contents: KO

so I think we need to have some kind of autodetection code. gnupg
maintainers: is grepping for “BEGIN PGP PUBLIC KEY BLOCK” enough to
decide between armored and non-armored? Or do you have any better
solutions?


KiBi.


signature.asc
Description: Digital signature

Bug#851774: Stop using apt-key add to add keys in generators/60local

[Julian Andres Klode]

On Sat, Jan 21, 2017 at 03:57:19PM +0100, Philipp Kern wrote:
> [Adding deity@l.d.o into the loop]
> 
> On 18.01.2017 17:43, Marga Manterola wrote:
> > For a long time it's been possible to preseed a local repository that
> > has it's own keyring. However, with the latest changes related to gpg
> > dependencies getting dropped in apt, this is no longer possible.
> > 
> > I'm setting severity as serious as adviced by Julien Cristau on IRC.
> > With the current state of things, in order to install a local repository
> > with a keyring the user needs to somehow create a script that will put
> > the keyring in place before 60local runs, and not preseed the keyring at
> > all.  If the keyring is preseeded, *the whole installation will fail*
> > because apt-key add fails which causes 60local to fail, which causes the
> > install base system step to fail.
> > 
> > This is the offending code:
> > https://sources.debian.net/src/apt-setup/1:0.123/generators/60local/#L33
> > 
> > This is using the deprecated apt-key add functionality.  From the
> > apt-key manpage:
> > 
> > COMMANDS
> >add filename
> > (...)
> >Note: Instead of using this command a keyring should be
> > placed directly in the /etc/apt/trusted.gpg.d/ directory with a
> > descriptive name and either "gpg" or "asc" as file extension.
> > 
> > So, the right thing to do is to copy the file to the right path instead
> > of calling apt-key add with it.
> 
> Does that mean that we actually have to infer (check using grep?) if the
> file is armored or not? I think `apt-key add' just dealt with whatever
> it got and put the key into the keyring using gpg's --import function.
> So it's a little unfortunate that we'd now need to know the format of
> what we need to put into the fragment directory.

AFAIK. If it is armored, it must be called .asc, if not it must be
called .gpg. Armored files are supported since 1.4~beta1, that is,
starting with stretch and Ubuntu zesty. GPG might also support
unarmored content in .asc files (at least it does --unarmor with
an unarmored file).

So, the answer is probably yes, but you could try out just
storing everything in .asc files and it might work (but that's
a bit ugly...).


-- 
Debian Developer - deb.li/jak | jak-linux.org - free software dev
  |  Ubuntu Core Developer |
When replying, only quote what is necessary, and write each reply
directly below the part(s) it pertains to ('inline').  Thank you.

Bug#851774: Stop using apt-key add to add keys in generators/60local

[Philipp Kern]

[Adding deity@l.d.o into the loop]

On 18.01.2017 17:43, Marga Manterola wrote:
> For a long time it's been possible to preseed a local repository that
> has it's own keyring. However, with the latest changes related to gpg
> dependencies getting dropped in apt, this is no longer possible.
> 
> I'm setting severity as serious as adviced by Julien Cristau on IRC.
> With the current state of things, in order to install a local repository
> with a keyring the user needs to somehow create a script that will put
> the keyring in place before 60local runs, and not preseed the keyring at
> all.  If the keyring is preseeded, *the whole installation will fail*
> because apt-key add fails which causes 60local to fail, which causes the
> install base system step to fail.
> 
> This is the offending code:
> https://sources.debian.net/src/apt-setup/1:0.123/generators/60local/#L33
> 
> This is using the deprecated apt-key add functionality.  From the
> apt-key manpage:
> 
> COMMANDS
>add filename
> (...)
>Note: Instead of using this command a keyring should be
> placed directly in the /etc/apt/trusted.gpg.d/ directory with a
> descriptive name and either "gpg" or "asc" as file extension.
> 
> So, the right thing to do is to copy the file to the right path instead
> of calling apt-key add with it.

Does that mean that we actually have to infer (check using grep?) if the
file is armored or not? I think `apt-key add' just dealt with whatever
it got and put the key into the keyring using gpg's --import function.
So it's a little unfortunate that we'd now need to know the format of
what we need to put into the fragment directory.

Kind regards
Philipp Kern



signature.asc
Description: OpenPGP digital signature

Bug#851774: Stop using apt-key add to add keys in generators/60local

[Marga Manterola]

Package: apt-setup
Version: 1:0.123
Severity: serious

For a long time it's been possible to preseed a local repository that has
it's own keyring. However, with the latest changes related to gpg
dependencies getting dropped in apt, this is no longer possible.

I'm setting severity as serious as adviced by Julien Cristau on IRC. With
the current state of things, in order to install a local repository with a
keyring the user needs to somehow create a script that will put the keyring
in place before 60local runs, and not preseed the keyring at all.  If the
keyring is preseeded, *the whole installation will fail* because apt-key
add fails which causes 60local to fail, which causes the install base
system step to fail.

This is the offending code:
https://sources.debian.net/src/apt-setup/1:0.123/generators/60local/#L33

This is using the deprecated apt-key add functionality.  From the apt-key
manpage:

COMMANDS
   add filename
(...)
   Note: Instead of using this command a keyring should be placed
directly in the /etc/apt/trusted.gpg.d/ directory with a descriptive name
and either "gpg" or "asc" as file extension.

So, the right thing to do is to copy the file to the right path instead of
calling apt-key add with it.

This was fixed in pbuilder back in September:

pbuilder (0.226.1) unstable; urgency=medium

  [ James Clarke ]
  * modules: add_additional_aptkeyrings:
Copy keyrings to /etc/apt/trusted.gpg.d instead of using apt-key.
We can no longer rely on being able to use apt-key in a minimal chroot,
because gnupg has been demoted to a Recommends in apt. Instead, the
keyrings can be copied directly into /etc/apt/trusted.gpg.d.
Moreover, `apt-key` usage has been discuraged over the past years.
This means that using the APTKEYRINGS option of pbuilder won't actually
work with chroots older than squeeze (APT version 0.7.25.1)

-- 
Regards,
Marga
-- 
Cheers,
Marga