unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* New “guix refresh” command
@ 2013-04-24 22:24 Ludovic Courtès
  2013-04-25 21:27 ` Ludovic Courtès
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Ludovic Courtès @ 2013-04-24 22:24 UTC (permalink / raw)
  To: bug-guix

Hello!

There’s a new ‘guix refresh’ command.  The target audience is mostly
Guix developers: the command reports GNU packages that are not
up-to-date, and optionally updates the source files to reflect the new
version number and tarball hash.  (This is essentially a port of my
‘gnupdate’ program for Nixpkgs [0].)

When downloading new tarballs, it also retrieves signatures and checks
them with GPG, via the new (guix gnupg) module.  If the public key is
missing, it attempts to get it from keys.gnupg.net, and tries again; in
that case, the key is added to your keyring.  (It’s similar to what the
OpenSSH client does when introduced to new hosts.)

There’s room for improvement here and there, but the basic functionality
is in place, and it’s going to be profitable for the on-going round of
package updates.  :-)

Comments welcome!

Ludo’.

[0] http://thread.gmane.org/gmane.linux.distributions.nixos/3811

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-24 22:24 New “guix refresh” command Ludovic Courtès
@ 2013-04-25 21:27 ` Ludovic Courtès
  2013-04-26 16:16 ` Andreas Enge
  2013-05-07 19:03 ` Nikita Karetnikov
  2 siblings, 0 replies; 26+ messages in thread
From: Ludovic Courtès @ 2013-04-25 21:27 UTC (permalink / raw)
  To: bug-guix

I’ve added a --select option.  The idea is that in ‘master’ you would
run ‘guix refresh --select=non-core’ to avoid updating core packages,
which would otherwise trigger a full rebuild.

Conversely, on ‘core-updates’ you would happily run ‘guix refresh -s core’ 
to update only core packages.

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-24 22:24 New “guix refresh” command Ludovic Courtès
  2013-04-25 21:27 ` Ludovic Courtès
@ 2013-04-26 16:16 ` Andreas Enge
  2013-04-27  9:43   ` Ludovic Courtès
  2013-05-07 19:03 ` Nikita Karetnikov
  2 siblings, 1 reply; 26+ messages in thread
From: Andreas Enge @ 2013-04-26 16:16 UTC (permalink / raw)
  To: bug-guix

Am Donnerstag, 25. April 2013 schrieb Ludovic Courtès:
> There’s a new ‘guix refresh’ command.

It also requires a newer guile than 2.0.5:

guix refresh: warning: using Guile 2.0.5-deb+1-3, which does not support 
HTTP ((chunked)) encoding
guix refresh: error: download failed; use a newer Guile

Andreas

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-26 16:16 ` Andreas Enge
@ 2013-04-27  9:43   ` Ludovic Courtès
  2013-04-27 10:11     ` Andreas Enge
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-04-27  9:43 UTC (permalink / raw)
  To: Andreas Enge; +Cc: bug-guix

Andreas Enge <andreas@enge.fr> skribis:

> Am Donnerstag, 25. April 2013 schrieb Ludovic Courtès:
>> There’s a new ‘guix refresh’ command.
>
> It also requires a newer guile than 2.0.5:
>
> guix refresh: warning: using Guile 2.0.5-deb+1-3, which does not support 
> HTTP ((chunked)) encoding
> guix refresh: error: download failed; use a newer Guile

I believe commit 1424a96 (in master) fixes that and the related
substituter problem.  Could you try and report back?

TIA,
Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-27  9:43   ` Ludovic Courtès
@ 2013-04-27 10:11     ` Andreas Enge
  2013-04-27 21:04       ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Enge @ 2013-04-27 10:11 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

Am Samstag, 27. April 2013 schrieb Ludovic Courtès:
> I believe commit 1424a96 (in master) fixes that and the related
> substituter problem.  Could you try and report back?

For "refresh", things seem to work:

starting download of `guix-file.yU56z7' from 
`ftp://ftp.gnupg.org//gcrypt/libassuan/libassuan-2.1.0.tar.bz2'...
ftp://ftp.gnupg.org/.../libassuan-2.1.0.tar.bz2  524.9 KiB transferred
starting download of `guix-file.59q6um' from 
`ftp://ftp.gnupg.org//gcrypt/libassuan/libassuan-2.1.0.tar.bz2.sig'...
ftp://ftp.gnupg.org/.../libassuan-2.1.0.tar.bz2.sig        0.3 KiB 
transferred
gpg: Signatur vom Fr 22 Feb 2013 19:51:54 CET mittels RSA-Schlüssel ID 
4F25E3B6
gpg: Signatur kann nicht geprüft werden: Kein öffentlicher Schlüssel
gpg: fordere Schlüssel 4F25E3B6 von hkp-Server keys.gnupg.net an
gpg: Schlüssel 4F25E3B6: Öffentlicher Schlüssel "Werner Koch (dist sig)" 
importiert
gpg: keine uneingeschränkt vertrauenswürdigen Schlüssel gefunden
gpg: Anzahl insgesamt bearbeiteter Schlüssel: 1
gpg:                              importiert: 1  (RSA: 1)
gpg: Signatur vom Fr 22 Feb 2013 19:51:54 CET mittels RSA-Schlüssel ID 
4F25E3B6
gpg: Korrekte Signatur von "Werner Koch (dist sig)"
gpg: WARNUNG: Dieser Schlüssel trägt keine vertrauenswürdige Signatur!
gpg:          Es gibt keinen Hinweis, daß die Signatur wirklich dem 
vorgeblichen Besitzer gehört.
Haupt-Fingerabdruck  = D869 2123 C406 5DEA 5E0F  3AB5 249B 39D2 4F25 E3B6
gnu/packages/gnupg.scm:82:2: libassuan: updating from version 2.0.3 to 
version 2.1.0...
libassuan: #<<location> file: "gnu/packages/gnupg.scm" line: 82 column: 2>: 
no `version' field in source; skipping

For the substituter, the message is the same as before:

the following file will be downloaded:
   /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8
@ substituter-started /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8 
/usr/local/guix-git/libexec/guix/substitute-binary
guix substitute-binary: warning: using Guile 2.0.5-deb+1-3, which does not 
support () encoding
guix substitute-binary: error: download failed; use a newer Guile
@ substituter-failed /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8 
256 fetching path `/nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8' 
failed with exit code 1
guix build: error: build failed: some substitutes for the outputs of 
derivation `/nix/store/7b51j955338q2kxj3ssvly0i2pw20z7q-hello-2.8.drv' 
failed; try `--fallback'

Andreas

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-27 10:11     ` Andreas Enge
@ 2013-04-27 21:04       ` Ludovic Courtès
  2013-04-27 21:14         ` Andreas Enge
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-04-27 21:04 UTC (permalink / raw)
  To: Andreas Enge; +Cc: bug-guix

Andreas Enge <andreas@enge.fr> skribis:

> Am Samstag, 27. April 2013 schrieb Ludovic Courtès:
>> I believe commit 1424a96 (in master) fixes that and the related
>> substituter problem.  Could you try and report back?
>
> For "refresh", things seem to work:
>
> starting download of `guix-file.yU56z7' from 
> `ftp://ftp.gnupg.org//gcrypt/libassuan/libassuan-2.1.0.tar.bz2'...
> ftp://ftp.gnupg.org/.../libassuan-2.1.0.tar.bz2  524.9 KiB transferred
> starting download of `guix-file.59q6um' from 
> `ftp://ftp.gnupg.org//gcrypt/libassuan/libassuan-2.1.0.tar.bz2.sig'...
> ftp://ftp.gnupg.org/.../libassuan-2.1.0.tar.bz2.sig        0.3 KiB 
> transferred
> gpg: Signatur vom Fr 22 Feb 2013 19:51:54 CET mittels RSA-Schlüssel ID 
> 4F25E3B6
> gpg: Signatur kann nicht geprüft werden: Kein öffentlicher Schlüssel
> gpg: fordere Schlüssel 4F25E3B6 von hkp-Server keys.gnupg.net an
> gpg: Schlüssel 4F25E3B6: Öffentlicher Schlüssel "Werner Koch (dist sig)" 
> importiert
> gpg: keine uneingeschränkt vertrauenswürdigen Schlüssel gefunden
> gpg: Anzahl insgesamt bearbeiteter Schlüssel: 1
> gpg:                              importiert: 1  (RSA: 1)
> gpg: Signatur vom Fr 22 Feb 2013 19:51:54 CET mittels RSA-Schlüssel ID 
> 4F25E3B6
> gpg: Korrekte Signatur von "Werner Koch (dist sig)"
> gpg: WARNUNG: Dieser Schlüssel trägt keine vertrauenswürdige Signatur!
> gpg:          Es gibt keinen Hinweis, daß die Signatur wirklich dem 
> vorgeblichen Besitzer gehört.
> Haupt-Fingerabdruck  = D869 2123 C406 5DEA 5E0F  3AB5 249B 39D2 4F25 E3B6
> gnu/packages/gnupg.scm:82:2: libassuan: updating from version 2.0.3 to 
> version 2.1.0...
> libassuan: #<<location> file: "gnu/packages/gnupg.scm" line: 82 column: 2>: 
> no `version' field in source; skipping

The above warning shows that it doesn’t work as expected on 2.0.5.
Fixed by 8e77f41.

> For the substituter, the message is the same as before:
>
> the following file will be downloaded:
>    /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8
> @ substituter-started /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8 
> /usr/local/guix-git/libexec/guix/substitute-binary
> guix substitute-binary: warning: using Guile 2.0.5-deb+1-3, which does not 
> support () encoding

Should be fixed by 61ef22f.

Can you confirm that it all works beautifully?  :-)

TIA,
Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-27 21:04       ` Ludovic Courtès
@ 2013-04-27 21:14         ` Andreas Enge
  2013-04-27 22:35           ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Enge @ 2013-04-27 21:14 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

Am Samstag, 27. April 2013 schrieb Ludovic Courtès:
> Can you confirm that it all works beautifully?  :-)

No. But is there progress?

$ guix build hello

the following file will be downloaded:
   /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8
@ substituter-started /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8 
/usr/local/guix-git/libexec/guix/substitute-binary
downloading `/nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8' from 
`http://hydra.gnu.org/nar/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8'...
Backtrace:
In ice-9/boot-9.scm:
 149: 11 [catch #t #<catch-closure 22bd1a0> ...]
 157: 10 [#<procedure 225b0f0 ()>]
In unknown file:
   ?: 9 [catch-closure]
In ice-9/boot-9.scm:
  63: 8 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 407: 7 [eval # #]
In ice-9/boot-9.scm:
2111: 6 [save-module-excursion #<procedure 2260100 at 
ice-9/boot-9.scm:3646:3 ()>]
3651: 5 [#<procedure 2260100 at ice-9/boot-9.scm:3646:3 ()>]
In unknown file:
   ?: 4 [load-compiled/vm "/root/.cache/guile/ccache/2.0-
LE-8-2.0/usr/local/guix-git/bin/guix.go"]
In guix/ui.scm:
 417: 3 [guix-main "/usr/local/guix-git/bin/guix" "substitute-binary" ...]
In guix/scripts/substitute-binary.scm:
 446: 2 [guix-substitute-binary "--substitute" ...]
 359: 1 [filtered-port # #<input: r6rs-bytevector-input-port 2854000>]
In unknown file:
   ?: 0 [fileno #<input: r6rs-bytevector-input-port 2854000>]

ERROR: In procedure fileno:
ERROR: In procedure fileno: Wrong type argument in position 1 (expecting 
open file port): #<input: r6rs-bytevector-input-port 2854000>
Backtrace:
In ice-9/boot-9.scm:
 149: 12 [catch #t #<catch-closure 22bd1a0> ...]
 157: 11 [#<procedure 225b0f0 ()>]
In unknown file:
   ?: 10 [catch-closure]
In ice-9/boot-9.scm:
  63: 9 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 407: 8 [eval # #]
In ice-9/boot-9.scm:
2111: 7 [save-module-excursion #<procedure 2260100 at 
ice-9/boot-9.scm:3646:3 ()>]
3651: 6 [#<procedure 2260100 at ice-9/boot-9.scm:3646:3 ()>]
In unknown file:
   ?: 5 [load-compiled/vm "/root/.cache/guile/ccache/2.0-
LE-8-2.0/usr/local/guix-git/bin/guix.go"]
In guix/ui.scm:
 417: 4 [guix-main "/usr/local/guix-git/bin/guix" "substitute-binary" ...]
In guix/scripts/substitute-binary.scm:
 457: 3 [guix-substitute-binary "--substitute" ...]
In guix/nar.scm:
 177: 2 [restore-file #<input: #{read pipe}# 5> ...]
In guix/serialization.scm:
  77: 1 [read-string #<input: #{read pipe}# 5>]
  49: 0 [read-int #<input: #{read pipe}# 5>]

guix/serialization.scm:49:4: In procedure read-int:
guix/serialization.scm:49:4: In procedure bv-u32-ref: Wrong type argument 
in position 1 (expecting bytevector): #<eof>
@ substituter-failed /nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8 
256 fetching path `/nix/store/w2121wnp8xv3ycjsgj3ymhb147mrgpc9-hello-2.8' 
failed with exit code 1
guix build: error: build failed: some substitutes for the outputs of 
derivation `/nix/store/z3yggci7jzmarvz46fqw64vqx964h9yc-hello-2.8.drv' 
failed; try `--fallback'

Andreas

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-27 21:14         ` Andreas Enge
@ 2013-04-27 22:35           ` Ludovic Courtès
  2013-04-29 21:27             ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-04-27 22:35 UTC (permalink / raw)
  To: Andreas Enge; +Cc: bug-guix

Andreas Enge <andreas@enge.fr> skribis:

> Am Samstag, 27. April 2013 schrieb Ludovic Courtès:
>> Can you confirm that it all works beautifully?  :-)
>
> No. But is there progress?
>
> $ guix build hello

[...]

>  359: 1 [filtered-port # #<input: r6rs-bytevector-input-port 2854000>]
> In unknown file:
>    ?: 0 [fileno #<input: r6rs-bytevector-input-port 2854000>]
>
> ERROR: In procedure fileno:
> ERROR: In procedure fileno: Wrong type argument in position 1 (expecting 
> open file port): #<input: r6rs-bytevector-input-port 2854000>

Ah yes, that’s the failure I was initially expecting.  Good news, no?
:-)

I’ll have to think a bit to find how to rewrite ‘filtered-port’ to not
assume that it’s passed a file-backed port.

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-27 22:35           ` Ludovic Courtès
@ 2013-04-29 21:27             ` Ludovic Courtès
  2013-04-30 15:54               ` Andreas Enge
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-04-29 21:27 UTC (permalink / raw)
  To: Andreas Enge; +Cc: bug-guix

ludo@gnu.org (Ludovic Courtès) skribis:

> Andreas Enge <andreas@enge.fr> skribis:
>
>> Am Samstag, 27. April 2013 schrieb Ludovic Courtès:
>>> Can you confirm that it all works beautifully?  :-)
>>
>> No. But is there progress?
>>
>> $ guix build hello
>
> [...]
>
>>  359: 1 [filtered-port # #<input: r6rs-bytevector-input-port 2854000>]
>> In unknown file:
>>    ?: 0 [fileno #<input: r6rs-bytevector-input-port 2854000>]
>>
>> ERROR: In procedure fileno:
>> ERROR: In procedure fileno: Wrong type argument in position 1 (expecting 
>> open file port): #<input: r6rs-bytevector-input-port 2854000>
>
> Ah yes, that’s the failure I was initially expecting.

Commit e0fbbc8 should fix it.  Can you confirm?

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-29 21:27             ` Ludovic Courtès
@ 2013-04-30 15:54               ` Andreas Enge
  0 siblings, 0 replies; 26+ messages in thread
From: Andreas Enge @ 2013-04-30 15:54 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

Am Montag, 29. April 2013 schrieb Ludovic Courtès:
> Commit e0fbbc8 should fix it.  Can you confirm?

So far, I just tried the binary substituter, and it works. Excellent, 
thanks for all your effort!

Andreas

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-04-24 22:24 New “guix refresh” command Ludovic Courtès
  2013-04-25 21:27 ` Ludovic Courtès
  2013-04-26 16:16 ` Andreas Enge
@ 2013-05-07 19:03 ` Nikita Karetnikov
  2013-05-07 22:21   ` Ludovic Courtès
  2 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-05-07 19:03 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

> When downloading new tarballs, it also retrieves signatures and checks
> them with GPG, via the new (guix gnupg) module.

Could you point me to this part of the source code?  I fail to find it.

> If the public key is missing, it attempts to get it from keys.gnupg.net,
> and tries again; in that case, the key is added to your keyring.

I haven't tried the tool yet, but I'm suspicious.

First, what if the mirror is malicious but the key is there?  You'll
fetch a malicious tarball and a malicious key.  Is it possible to use
three mirrors to check keys and tarballs?

I also think that one must always check keys manually (using similar
pages [1]).  Maybe we should manually add fingerprints to a
licenses.scm-like file and use it along with keys.gnupg.net.  It sounds
tedious, but it'll be necessary only when you package something for the
first time.  What do you think?

It also bugs me that there are a lot of packages which are not signed at
all.  I guess I'll start to ask maintainers to add signatures (at least
for the future versions).  I hope you'll do the same.

Second, is there a way not to pollute my keyring with such keys or at
least mark them somehow (for example, as not trusted)?

[1] http://gcc.gnu.org/mirrors.html

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-07 19:03 ` Nikita Karetnikov
@ 2013-05-07 22:21   ` Ludovic Courtès
  2013-05-10  0:29     ` Nikita Karetnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-05-07 22:21 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> When downloading new tarballs, it also retrieves signatures and checks
>> them with GPG, via the new (guix gnupg) module.
>
> Could you point me to this part of the source code?  I fail to find it.

See ‘download-tarball’ in gnu-maintenance.scm.

>> If the public key is missing, it attempts to get it from keys.gnupg.net,
>> and tries again; in that case, the key is added to your keyring.
>
> I haven't tried the tool yet, but I'm suspicious.

Ah, I’m glad somebody chimes in.  ;-)

> First, what if the mirror is malicious but the key is there?  You'll
> fetch a malicious tarball and a malicious key.

Objects aren’t malicious.  Perhaps you’re talking about situations where
a mirror provides a tarball along with a valid signature, but said
signature is made with a random key, and the tarball is actually not
genuine, right?

First, note that ‘download-tarball’ fetches from ftp.gnu.org directly
(or ftp.gnupg.org, etc.), not from mirrors.

Second, this is the same model as used by the OpenSSH client.  When the
client is first introduced to a host, it presents you its key
fingerprint, you type ‘y’, and that key gets added to your known hosts
file.  From there on, person-in-the-middle attacks are trivially
detected as a key mismatch.

With this approach, introduction is the weak link.  It is mitigated by
the fact that, for instance, I’ve already imported and signed keys of
several GNU maintainers, and by common sense (manually checking the
signatures on a key, the tarball contents, etc.)  Also, keep in mind
that ‘guix refresh’ is primarily a maintainer’s tool.

It’s exactly what I would do manually.  What about you?

> Is it possible to use three mirrors to check keys and tarballs?

Check against what?  What do you want to address?

> I also think that one must always check keys manually (using similar
> pages [1]).  Maybe we should manually add fingerprints to a
> licenses.scm-like file and use it along with keys.gnupg.net.  It sounds
> tedious, but it'll be necessary only when you package something for the
> first time.  What do you think?

There’s the ftp.gnu.org/gnu/gnu-keyring.gpg file, which contains all the
keys ever allowed to sign GNU uploads.

But that file is itself currently unsigned.

Ideally (I think) that file would be signed, and the Guix repo would
contain the master key used to sign gnu-keyring.gpg.  From there, it
could fetch that keyring and authenticate it anytime, which in turn
could be used to authenticate GNU source tarballs, as needed for the
on-line auto-updater (see
<http://lists.gnu.org/archive/html/bug-guix/2013-03/msg00032.html>.)

This is similar to Debian’s approach, AIUI.

I’ve made this suggestion to one of the FSF sysadmins, but it seems to
need further discussion, and probably input from crypto-savvy people.

> It also bugs me that there are a lot of packages which are not signed at
> all.  I guess I'll start to ask maintainers to add signatures (at least
> for the future versions).  I hope you'll do the same.

All the packages on gnu{,pg}.org are signed.  I think very few GNU
packages are unsigned.

For non-GNU packages, the situation is not as good, and I agree we must
spread the word, but that won’t change overnight.

> Second, is there a way not to pollute my keyring with such keys or at
> least mark them somehow (for example, as not trusted)?

They are marked as such by default.

Problem is, I want to use my default keyring because it already contains
many keys that I signed.  So I don’t see how to accommodate both needs.

Thanks for sharing your thoughts and concerns!

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-07 22:21   ` Ludovic Courtès
@ 2013-05-10  0:29     ` Nikita Karetnikov
  2013-05-10 13:11       ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-05-10  0:29 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

> Objects aren’t malicious.  Perhaps you’re talking about situations where
> a mirror provides a tarball along with a valid signature, but said
> signature is made with a random key, and the tarball is actually not
> genuine, right?

Yep.

> Second, this is the same model as used by the OpenSSH client.  When the
> client is first introduced to a host, it presents you its key
> fingerprint, you type ‘y’, and that key gets added to your known hosts
> file.  From there on, person-in-the-middle attacks are trivially
> detected as a key mismatch.

AFAICT, 'guix refresh' doesn't allow to check fingerprints.  If so, we
must change it.

Am I mistaken?  I'm not sure because it fails on my machine:

# ./pre-inst-env guix refresh -u

[...]

In execlp of gpg2: No such file or directory
guix refresh: warning: signature verification failed for `guile-2.0.9.tar.gz'
guix refresh: warning: (could be because the public key is not in your keyring)
gnu/packages/guile.scm:48:12: guile: updating from version 1.8.8 to version 2.0.9...
Backtrace:
In ice-9/boot-9.scm:
 157: 12 [catch #t #<catch-closure 954b170> ...]
In unknown file:
   ?: 11 [apply-smob/1 #<catch-closure 954b170>]
In ice-9/boot-9.scm:
  63: 10 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 432: 9 [eval # #]
In ice-9/boot-9.scm:
2320: 8 [save-module-excursion #<procedure 93f9e80 at ice-9/boot-9.scm:3961:3 ()>]
3966: 7 [#<procedure 93f9e80 at ice-9/boot-9.scm:3961:3 ()>]
In unknown file:
   ?: 6 [load-compiled/vm "/root/.cache/guile/ccache/2.0-LE-4-2.0/home/guix-test2/scripts/guix.go"]
In guix/ui.scm:
 417: 5 [guix-main "/home/guix-test2/scripts/guix" "refresh" "-u"]
In ice-9/boot-9.scm:
 157: 4 [catch srfi-34 #<procedure 9858520 at guix/ui.scm:138:2 ()> ...]
In srfi/srfi-1.scm:
 619: 3 [for-each #<procedure 98580e0 at guix/scripts/refresh.scm:151:22 (package)> ...]
In guix/scripts/refresh.scm:
 167: 2 [#<procedure 98580e0 at guix/scripts/refresh.scm:151:22 (package)> #]
In ice-9/boot-9.scm:
 788: 1 [call-with-input-file #f ...]
In unknown file:
   ?: 0 [open-file #f "r" #:encoding #f #:guess-encoding #f]

ERROR: In procedure open-file:
ERROR: Wrong type (expecting string): #f

> It’s exactly what I would do manually.  What about you?

It depends.  I usually use a similar page [1] to compare fingerprints
and also check via keys.gnupg.net.  Sometimes I try to get more
information elsewhere.  Again, the sad truth is that it's easier not to
sign an ingenuine tarball at all.

>> Is it possible to use three mirrors to check keys and tarballs?

> Check against what?  What do you want to address?

Check them against each other.  But it's not the case because 'guix
refresh' uses one server per package.

> I’ve made this suggestion to one of the FSF sysadmins, but it seems to
> need further discussion, and probably input from crypto-savvy people.

OK. 

[1] http://gcc.gnu.org/mirrors.html

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-10  0:29     ` Nikita Karetnikov
@ 2013-05-10 13:11       ` Ludovic Courtès
  2013-05-10 22:54         ` Nikita Karetnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-05-10 13:11 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> Objects aren’t malicious.  Perhaps you’re talking about situations where
>> a mirror provides a tarball along with a valid signature, but said
>> signature is made with a random key, and the tarball is actually not
>> genuine, right?
>
> Yep.
>
>> Second, this is the same model as used by the OpenSSH client.  When the
>> client is first introduced to a host, it presents you its key
>> fingerprint, you type ‘y’, and that key gets added to your known hosts
>> file.  From there on, person-in-the-middle attacks are trivially
>> detected as a key mismatch.
>
> AFAICT, 'guix refresh' doesn't allow to check fingerprints.  If so, we
> must change it.

It doesn’t ask you to type ‘y’, but it does display the key fingerprint
when it first downloads it (well, gpg does.)

> Am I mistaken?  I'm not sure because it fails on my machine:
>
> # ./pre-inst-env guix refresh -u
>
> [...]
>
> In execlp of gpg2: No such file or directory

You need to have GnuPG 2.x installed:

  guix package -i gnupg

> guix refresh: warning: signature verification failed for `guile-2.0.9.tar.gz'
> guix refresh: warning: (could be because the public key is not in your keyring)
> gnu/packages/guile.scm:48:12: guile: updating from version 1.8.8 to version 2.0.9...

(Of course it shouldn’t try to update 1.8 to 2.0; future work...)

[...]

> In guix/scripts/refresh.scm:
>  167: 2 [#<procedure 98580e0 at guix/scripts/refresh.scm:151:22 (package)> #]
> In ice-9/boot-9.scm:
>  788: 1 [call-with-input-file #f ...]
> In unknown file:
>    ?: 0 [open-file #f "r" #:encoding #f #:guess-encoding #f]
>
> ERROR: In procedure open-file:
> ERROR: Wrong type (expecting string): #f

I’ve just changed it to gracefully handle this case.

>> It’s exactly what I would do manually.  What about you?
>
> It depends.  I usually use a similar page [1] to compare fingerprints
> and also check via keys.gnupg.net.

Well, it’s not clear that checking the checksum published on a web page
adds much to checking against a freshly download tarball (a sufficiently
motivated attacker could just as well be serving you a modified web
page, after all.)

>>> Is it possible to use three mirrors to check keys and tarballs?
>
>> Check against what?  What do you want to address?
>
> Check them against each other.  But it's not the case because 'guix
> refresh' uses one server per package.

Hmm I tend to think this is unneeded paranoia, because such things are
eventually checked by all of us anyway.

(BTW, keep in mind that Git commits are not signed.  That would be by
far the easiest attack vector.)

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-10 13:11       ` Ludovic Courtès
@ 2013-05-10 22:54         ` Nikita Karetnikov
  2013-05-11 10:10           ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-05-10 22:54 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

>> In execlp of gpg2: No such file or directory

> You need to have GnuPG 2.x installed:

>   guix package -i gnupg

Why is it trying to download tarballs after this error?  I think it
should print an error message and exit.

How can I use packages from my profile?  I've added
'/root/.guix-profile' to PATH (should we add it to 'pre-inst-env'?), but
'/root/.guix-profile/bin/gpg2' fails to resolve host 'keys.gnupg.net'.
'/etc/resolv.conf' isn't empty; '/usr/bin/gpg' works fine.

And I still think that 'guix refresh' must ask before importing a key.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-10 22:54         ` Nikita Karetnikov
@ 2013-05-11 10:10           ` Ludovic Courtès
  2013-05-11 14:05             ` Nikita Karetnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-05-11 10:10 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>>> In execlp of gpg2: No such file or directory
>
>> You need to have GnuPG 2.x installed:
>
>>   guix package -i gnupg
>
> Why is it trying to download tarballs after this error?  I think it
> should print an error message and exit.

Doesn’t fe3e603 address this?  Oh, maybe it doesn’t distinguish between
“command not found” and some other error, that’s what you mean?

> How can I use packages from my profile?  I've added
> '/root/.guix-profile' to PATH

+ /bin

> '/root/.guix-profile/bin/gpg2' fails to resolve host 'keys.gnupg.net'.

Interesting:

  guile -c '(gethostbyname "keys.gnupg.net")'  works
  guile -c '(getaddrinfo "keys.gnupg.net")'    doesn’t

I’m not sure why this is the case, but I think we’ll switch to
pgp.mit.edu for now (keys.gnupg.net also seems to be down from time to
time anyway.)

> '/etc/resolv.conf' isn't empty; '/usr/bin/gpg' works fine.

/usr/bin/gpg uses a different libc, and perhaps it uses the IPv4-only
gethostbyname too.

> And I still think that 'guix refresh' must ask before importing a key.

Patches welcome.  :-)

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-11 10:10           ` Ludovic Courtès
@ 2013-05-11 14:05             ` Nikita Karetnikov
  2013-05-24 10:19               ` Nikita Karetnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-05-11 14:05 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

> Oh, maybe it doesn’t distinguish between “command not found” and some
> other error, that’s what you mean?

Yes, I guess.

>> '/etc/resolv.conf' isn't empty; '/usr/bin/gpg' works fine.

> /usr/bin/gpg uses a different libc, and perhaps it uses the IPv4-only
> gethostbyname too.

>> And I still think that 'guix refresh' must ask before importing a key.

> Patches welcome.  :-)

Hmm, I'll have a look, but don't expect it in 0.2.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-11 14:05             ` Nikita Karetnikov
@ 2013-05-24 10:19               ` Nikita Karetnikov
  2013-05-24 12:54                 ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-05-24 10:19 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix


[-- Attachment #1.1: Type: text/plain, Size: 120 bytes --]

>> And I still think that 'guix refresh' must ask before importing a key.

> Patches welcome.  :-)

What do you think?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: gnupg.diff --]
[-- Type: text/x-diff, Size: 574 bytes --]

diff --git a/guix/gnupg.scm b/guix/gnupg.scm
index c17a495..ba5160a 100644
--- a/guix/gnupg.scm
+++ b/guix/gnupg.scm
@@ -143,7 +143,9 @@ missing key."
        status))
 
 (define (gnupg-receive-keys key-id server)
-  (system* (%gpg-command) "--keyserver" server "--recv-keys" key-id))
+  (system* (%gpg-command)
+           "--keyserver" server
+           "--search-keys" (string-append "0x" key-id)))
 
 (define* (gnupg-verify* sig file #:optional (server (%openpgp-key-server)))
   "Like `gnupg-verify', but try downloading the public key if it's missing.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-24 10:19               ` Nikita Karetnikov
@ 2013-05-24 12:54                 ` Ludovic Courtès
  2013-05-30  0:46                   ` Nikita Karetnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-05-24 12:54 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>  (define (gnupg-receive-keys key-id server)
> -  (system* (%gpg-command) "--keyserver" server "--recv-keys" key-id))
> +  (system* (%gpg-command)
> +           "--keyserver" server
> +           "--search-keys" (string-append "0x" key-id)))

As the name suggests, this procedure is meant to download a key, not to
search for a key.

The interactive behavior you wanted you involve telling the user that a
key needs to be downloaded before calling ‘gnupg-receive-keys’.

That could be done by changing ‘gnupg-verify*’.  An optional argument
could be added to select between interactive behavior (“do you want to
download this key and add it to your keyring?”), always-download, and
never-download.

WDYT?

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-24 12:54                 ` Ludovic Courtès
@ 2013-05-30  0:46                   ` Nikita Karetnikov
  2013-06-01 15:55                     ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-05-30  0:46 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix


[-- Attachment #1.1: Type: text/plain, Size: 1731 bytes --]

> That could be done by changing ‘gnupg-verify*’.  An optional argument
> could be added to select between interactive behavior (“do you want to
> download this key and add it to your keyring?”), always-download, and
> never-download.

I'm attaching my attempt.

There are two similar but unrelated problems:

1. The following function doesn't print the message.

(begin (format #t (_ "~a~a~!")
			   "Would you like to download this key "
			   "and add it to your keyring? (y/N) ")
	   (read-line))

2. 'else' doesn't work.

(else
 (and (receive?)
	  (download-and-try-again)))

# which gpg2
/root/.guix-profile/bin/gpg2
# gpg2 --delete-key EA52ECF4
# ./pre-inst-env guix refresh -u
accepted connection from pid 7779, uid 0
starting download of `guix-file.RAA3r7' from `ftp://ftp.gnu.org//gnu/guile/guile-2.0.9.tar.gz'...
ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz	100.0% of 7163.4 KiB
starting download of `guix-file.gJlE96' from `ftp://ftp.gnu.org//gnu/guile/guile-2.0.9.tar.gz.sig'...
ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key

(It should print the above message here, but it always tries to download
GCC instead.)

starting download of `guix-file.ZkDiI7' from `ftp://ftp.gnu.org//gnu/gcc/gcc-4.8.0/gcc-4.8.0.tar.bz2'...

To-do list:

1. Any argument except 'always', 'never', and 'interactive' should raise
   an error.

2. Fetch signatures first and don't download tarballs which can't be
   authenticated (when signatures are missing and 'never' is used).

3. How should I change 'receive?' to support i18n?

Anything else?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: download-sigs.diff --]
[-- Type: text/x-diff, Size: 11004 bytes --]

diff --git a/guix/gnu-maintenance.scm b/guix/gnu-maintenance.scm
index b54cd84..04d72d3 100644
--- a/guix/gnu-maintenance.scm
+++ b/guix/gnu-maintenance.scm
@@ -24,6 +24,7 @@
   #:use-module (ice-9 regex)
   #:use-module (ice-9 rdelim)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 optargs)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
@@ -341,7 +342,7 @@ pairs.  Example: (\"mit-scheme-9.0.1\" . \"/gnu/mit-scheme/stable.pkg/9.0.1\").
          (_ #f))))
 
 (define* (download-tarball store project directory version
-                           #:optional (archive-type "gz"))
+                           #:optional (archive-type "gz") download-sigs)
   "Download PROJECT's tarball over FTP and check its OpenPGP signature.  On
 success, return the tarball file name."
   (let* ((server  (ftp-server/directory project))
@@ -350,7 +351,7 @@ success, return the tarball file name."
          (sig-url (string-append url ".sig"))
          (tarball (download-to-store store url))
          (sig     (download-to-store store sig-url)))
-    (let ((ret (gnupg-verify* sig tarball)))
+    (let ((ret (gnupg-verify* sig tarball download-sigs)))
       (if ret
           tarball
           (begin
@@ -359,7 +360,7 @@ success, return the tarball file name."
             (warning (_ "(could be because the public key is not in your keyring)~%"))
             #f)))))
 
-(define (package-update store package)
+(define* (package-update store package #:optional download-sigs)
   "Return the new version and the file name of the new version tarball for
 PACKAGE, or #f and #f when PACKAGE is up-to-date."
   (match (package-update-path package)
@@ -372,7 +373,7 @@ PACKAGE, or #f and #f when PACKAGE is up-to-date."
                               (file-extension (origin-uri source)))
                          "gz"))))
        (let ((tarball (download-tarball store name directory version
-                                        archive-type)))
+                                        archive-type download-sigs)))
          (values version tarball))))
     (_
      (values #f #f))))
diff --git a/guix/gnupg.scm b/guix/gnupg.scm
index c17a495..8d2a7e6 100644
--- a/guix/gnupg.scm
+++ b/guix/gnupg.scm
@@ -17,6 +17,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix gnupg)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 popen)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
@@ -145,16 +146,42 @@ missing key."
 (define (gnupg-receive-keys key-id server)
   (system* (%gpg-command) "--keyserver" server "--recv-keys" key-id))
 
-(define* (gnupg-verify* sig file #:optional (server (%openpgp-key-server)))
+(define* (gnupg-verify* sig file #:optional download-sigs
+                                            (server (%openpgp-key-server)))
   "Like `gnupg-verify', but try downloading the public key if it's missing.
 Return #t if the signature was good, #f otherwise."
   (let ((status (gnupg-verify sig file)))
     (or (gnupg-status-good-signature? status)
         (let ((missing (gnupg-status-missing-key? status)))
-          (and missing
-               (begin
-                 ;; Download the missing key and try again.
-                 (gnupg-receive-keys missing server)
-                 (gnupg-status-good-signature? (gnupg-verify sig file))))))))
+          (define (download-and-try-again)
+            (begin
+              ;; Download the missing key and try again.
+              (gnupg-receive-keys missing server)
+              (gnupg-status-good-signature? (gnupg-verify sig file))))
+
+          (define (receive?)
+            (string=? "y"               ; XXX: i18n
+
+                      ;; XXX: Doesn't print the message.
+                      ;; (begin (format #t (_ "~a~a~!")
+                      ;;                "Would you like to download this key "
+                      ;;                "and add it to your keyring? (y/N) ")
+                      ;;        (read-line))))
+
+                      (begin (format #t "~a~a~!"
+                                     "Would you like to download this key "
+                                     "and add it to your keyring? (y/N) ")
+                             (read-line))))
+
+          (and
+           missing
+           ;; XXX: 'else' doesn't work.
+           (cond ((string=? download-sigs "always")
+                  (download-and-try-again))
+                 ((string=? download-sigs "never")
+                  #f)
+                 (else
+                  (and (receive?)
+                       (download-and-try-again)))))))))
 
 ;;; gnupg.scm ends here
diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 10715eb..9beeddc 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -27,6 +27,7 @@
   #:use-module ((gnu packages base) #:select (%final-inputs))
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
+  #:use-module (ice-9 optargs)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
@@ -64,6 +65,9 @@
         (option '("gpg") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'gpg-command arg result)))
+        (option '(#\d "download-sigs") #t #f
+                (lambda (opt name arg result)
+                  (alist-cons 'download-sigs arg result)))
 
         (option '(#\h "help") #f #f
                 (lambda args
@@ -79,7 +83,11 @@ Update package definitions to match the latest upstream version.
 
 When PACKAGE... is given, update only the specified packages.  Otherwise
 update all the packages of the distribution, or the subset thereof
-specified with `--select'.\n"))
+specified with `--select'.
+
+'download-sigs' accepts one of the following arguments: 'interactive',
+'always', and 'never'.  When 'download-sigs' is not specified, assume
+'interactive'.\n"))
   (display (_ "
   -u, --update           update source files in place"))
   (display (_ "
@@ -90,6 +98,9 @@ specified with `--select'.\n"))
       --key-server=HOST  use HOST as the OpenPGP key server"))
   (display (_ "
       --gpg=COMMAND      use COMMAND as the GnuPG 2.x command"))
+  (display (_ "
+  -d, --download-sigs=ARG
+                         download and add signatures to your keyring"))
   (newline)
   (display (_ "
   -h, --help             display this help and exit"))
@@ -98,12 +109,12 @@ specified with `--select'.\n"))
   (newline)
   (show-bug-report-information))
 
-(define (update-package store package)
+(define* (update-package store package #:optional download-sigs)
   "Update the source file that defines PACKAGE with the new version."
   (let-values (((version tarball)
                 (catch #t
                   (lambda ()
-                    (package-update store package))
+                    (package-update store package download-sigs))
                   (lambda _
                     (values #f #f))))
                ((loc)
@@ -161,31 +172,33 @@ update would trigger a complete rebuild."
         ;; XXX: Fails to catch MPFR/MPC, whose *source* is used as input.
         (member (package-name package) names))))
 
-  (let* ((opts     (parse-options))
-         (update?  (assoc-ref opts 'update?))
-         (packages (match (concatenate
-                           (filter-map (match-lambda
-                                        (('argument . value)
-                                         (let ((p (find-packages-by-name value)))
-                                           (unless p
-                                             (leave (_ "~a: no package by that name")
-                                                    value))
-                                           p))
-                                        (_ #f))
-                                       opts))
-                     (()                          ; default to all packages
-                      (let ((select? (match (assoc-ref opts 'select)
-                                       ('core core-package?)
-                                       ('non-core (negate core-package?))
-                                       (_ (const #t)))))
-                        ;; TODO: Keep only the newest of each package.
-                        (fold-packages (lambda (package result)
-                                         (if (select? package)
-                                             (cons package result)
-                                             result))
-                                       '())))
-                     (some                        ; user-specified packages
-                      some))))
+  (let* ((opts          (parse-options))
+         (update?       (assoc-ref opts 'update?))
+         (download-sigs (assoc-ref opts 'download-sigs))
+         (packages
+          (match (concatenate
+                  (filter-map (match-lambda
+                               (('argument . value)
+                                (let ((p (find-packages-by-name value)))
+                                  (unless p
+                                    (leave (_ "~a: no package by that name")
+                                           value))
+                                  p))
+                               (_ #f))
+                              opts))
+                 (()                          ; default to all packages
+                  (let ((select? (match (assoc-ref opts 'select)
+                                        ('core core-package?)
+                                        ('non-core (negate core-package?))
+                                        (_ (const #t)))))
+                    ;; TODO: Keep only the newest of each package.
+                    (fold-packages (lambda (package result)
+                                     (if (select? package)
+                                         (cons package result)
+                                         result))
+                                   '())))
+                 (some                        ; user-specified packages
+                  some))))
     (with-error-handling
       (if update?
           (let ((store (open-connection)))
@@ -195,7 +208,7 @@ update would trigger a complete rebuild."
                            (%gpg-command
                             (or (assoc-ref opts 'gpg-command)
                                 (%gpg-command))))
-              (for-each (cut update-package store <>) packages)))
+              (for-each (cut update-package store <> download-sigs) packages)))
           (for-each (lambda (package)
                       (match (false-if-exception (package-update-path package))
                         ((new-version . directory)

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-05-30  0:46                   ` Nikita Karetnikov
@ 2013-06-01 15:55                     ` Ludovic Courtès
  2013-06-02 22:29                       ` Ludovic Courtès
  2013-06-07  5:26                       ` [PATCH] guix refresh: Add '--key-download' Nikita Karetnikov
  0 siblings, 2 replies; 26+ messages in thread
From: Ludovic Courtès @ 2013-06-01 15:55 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> That could be done by changing ‘gnupg-verify*’.  An optional argument
>> could be added to select between interactive behavior (“do you want to
>> download this key and add it to your keyring?”), always-download, and
>> never-download.
>
> I'm attaching my attempt.

Thanks for looking into it!

> There are two similar but unrelated problems:
>
> 1. The following function doesn't print the message.
>
> (begin (format #t (_ "~a~a~!")
> 			   "Would you like to download this key "
> 			   "and add it to your keyring? (y/N) ")
> 	   (read-line))

First the whole string should be enclosed in (_ ...), otherwise xgettext
will just extract "~a~a" for translation.

Second, use ~% (instead of ~!); that will add a newline, and presumably
cause the string to be output.

Lastly, the indentation of (read-line) is incorrect.

> To-do list:
>
> 1. Any argument except 'always', 'never', and 'interactive' should raise
>    an error.
>
> 2. Fetch signatures first and don't download tarballs which can't be
>    authenticated (when signatures are missing and 'never' is used).
>
> 3. How should I change 'receive?' to support i18n?
>
> Anything else?

Comments below:

> +  #:use-module (ice-9 optargs)

This module is not needed (it’s for command-line argument processing.)

>  (define* (download-tarball store project directory version
> -                           #:optional (archive-type "gz"))
> +                           #:optional (archive-type "gz") download-sigs)

Perhaps change it to

  #:key (key-download 'interactive)

and document KEY-DOWNLOAD in the docstring.

> +(define* (package-update store package #:optional download-sigs)
>    "Return the new version and the file name of the new version tarball for
>  PACKAGE, or #f and #f when PACKAGE is up-to-date."

Likewise.

> +(define* (gnupg-verify* sig file #:optional download-sigs
> +                                            (server (%openpgp-key-server)))
>    "Like `gnupg-verify', but try downloading the public key if it's missing.
>  Return #t if the signature was good, #f otherwise."

Likewise.

> +          (define (receive?)
> +            (string=? "y"               ; XXX: i18n

Guile’s (ice-9 i18n) exports ‘locale-yes-regexp’ and ‘locale-no-regexp’
(info "(guile) Accessing Locale Information").

> +          (and
> +           missing
> +           ;; XXX: 'else' doesn't work.
> +           (cond ((string=? download-sigs "always")
> +                  (download-and-try-again))
> +                 ((string=? download-sigs "never")
> +                  #f)

‘download-sigs’ (rather, ‘key-download’) should be a symbol, not a
string (this is a common convention).  So this will read:

  (case key-download
    ((never) #f)
    ((always)
     (download-and-try-again))
    (else
     ;; ...
     ))

> +        (option '(#\d "download-sigs") #t #f

"key-download".

> +                (lambda (opt name arg result)
> +                  (alist-cons 'download-sigs arg result)))

Ditto.

>  When PACKAGE... is given, update only the specified packages.  Otherwise
>  update all the packages of the distribution, or the subset thereof
> -specified with `--select'.\n"))
> +specified with `--select'.
> +
> +'download-sigs' accepts one of the following arguments: 'interactive',
> +'always', and 'never'.  When 'download-sigs' is not specified, assume
> +'interactive'.\n"))

This should go...

> +  (display (_ "
> +  -d, --download-sigs=ARG
> +                         download and add signatures to your keyring"))

... here, IMO.

The string should rather be:

  --key-download=POLICY
         handle missing OpenPGP keys according to POLICY (one of ...)

> +(define* (update-package store package #:optional download-sigs)

Rename accordingly.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: New “guix refresh” command
  2013-06-01 15:55                     ` Ludovic Courtès
@ 2013-06-02 22:29                       ` Ludovic Courtès
  2013-06-07  5:26                       ` [PATCH] guix refresh: Add '--key-download' Nikita Karetnikov
  1 sibling, 0 replies; 26+ messages in thread
From: Ludovic Courtès @ 2013-06-02 22:29 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

ludo@gnu.org (Ludovic Courtès) skribis:

>> +  #:use-module (ice-9 optargs)
>
> This module is not needed (it’s for command-line argument processing.)

Sorry, I just realized my mistake: (ice-9 optargs) was indeed for #:key,
#:optional, and #:rest handling, but it no longer needs to be imported
with Guile 2.0 (info "(guile) ice-9 optargs").

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] guix refresh: Add  '--key-download'.
  2013-06-01 15:55                     ` Ludovic Courtès
  2013-06-02 22:29                       ` Ludovic Courtès
@ 2013-06-07  5:26                       ` Nikita Karetnikov
  2013-06-07 16:19                         ` Ludovic Courtès
  1 sibling, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-06-07  5:26 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix


[-- Attachment #1.1: Type: text/plain, Size: 6595 bytes --]

> First the whole string should be enclosed in (_ ...), otherwise xgettext
> will just extract "~a~a" for translation.

Should I do the same here?

+                  (match arg
+                    ((or "interactive" "always" "never")
+                     (alist-cons 'key-download (string->symbol arg)
+                                 result))

> Perhaps change it to

>   #:key (key-download 'interactive)

I've tried that, but things like (package-update #:key-download
key-download) don't look right.  Here is a simplified example:

;; guix/scripts/refresh.scm
(define* (update-package #:key (key-download 'interactive))
  (package-update #:key-download key-download))

;; guix/gnu-maintenance.scm
(define* (download-tarball #:key (key-download 'interactive))
  (gnupg-verify* #:key-download key-download))

(define* (package-update #:key (key-download 'interactive))
  (download-tarball #:key-download key-download))

;; guix/gnupg.scm
(define* (gnupg-verify* #:key (key-download 'interactive))
  (begin (display key-download)
         (newline)))

scheme@(guile-user)> (update-package)
interactive
scheme@(guile-user)> (update-package #:key-download 'never)
never

> > +          (define (receive?)
> > +            (string=? "y"               ; XXX: i18n

> Guile’s (ice-9 i18n) exports ‘locale-yes-regexp’ and ‘locale-no-regexp’
> (info "(guile) Accessing Locale Information").

Is it fine now?

I'm attaching a patch.  Examples (some commands were omitted):

# ./pre-inst-env guix refresh -u

[...]

ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key
Would you like to download this key and add it to your keyring?
n
guix refresh: warning: signature verification failed for `guile-2.0.9.tar.gz'
guix refresh: warning: (could be because the public key is not in your keyring)

Should I prepend "guix refresh: " to the question?

# ./pre-inst-env guix refresh -u

[...]

ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key
Would you like to download this key and add it to your keyring?
y
gpg: requesting key EA52ECF4 from hkp server pgp.mit.edu
gpg: key EA52ECF4: public key "Ludovic Courtès <ludo@gnu.org>" imported
gpg: no ultimately trusted keys found
gpg: Total number processed: 1
gpg:               imported: 1
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Good signature from "Ludovic Courtès <ludo@gnu.org>"
gpg:                 aka "Ludovic Courtès <ludo@chbouib.org>"
gpg:                 aka "Ludovic Courtès <lcourtes@altern.org>"
gpg:                 aka "Ludovic Courtès (INRIA) <ludovic.courtes@inria.fr>"
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 83C4 F8E5 10A3 3B4C 5BEA  D15D 77DD 95E2 EA52 ECF4
gnu/packages/guile.scm:49:12: guile: updating from version 1.8.8 to version 2.0.9...

# ./pre-inst-env guix refresh -u --key-download=never

[...]

ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key
guix refresh: warning: signature verification failed for `guile-2.0.9.tar.gz'
guix refresh: warning: (could be because the public key is not in your keyring)

# ./pre-inst-env guix refresh -u --key-download=always

[...]

ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key
gpg: requesting key EA52ECF4 from hkp server pgp.mit.edu
gpg: key EA52ECF4: public key "Ludovic Courtès <ludo@gnu.org>" imported
gpg: no ultimately trusted keys found
gpg: Total number processed: 1
gpg:               imported: 1
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Good signature from "Ludovic Courtès <ludo@gnu.org>"
gpg:                 aka "Ludovic Courtès <ludo@chbouib.org>"
gpg:                 aka "Ludovic Courtès <lcourtes@altern.org>"
gpg:                 aka "Ludovic Courtès (INRIA) <ludovic.courtes@inria.fr>"
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 83C4 F8E5 10A3 3B4C 5BEA  D15D 77DD 95E2 EA52 ECF4
gnu/packages/guile.scm:49:12: guile: updating from version 1.8.8 to version 2.0.9...

# ./pre-inst-env guix refresh -u --key-download=interactive

ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key
Would you like to download this key and add it to your keyring?
n
guix refresh: warning: signature verification failed for `guile-2.0.9.tar.gz'
guix refresh: warning: (could be because the public key is not in your keyring)

# ./pre-inst-env guix refresh -u --key-download=interactive

ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Can't check signature: No public key
Would you like to download this key and add it to your keyring?
y
gpg: requesting key EA52ECF4 from hkp server pgp.mit.edu
gpg: key EA52ECF4: public key "Ludovic Courtès <ludo@gnu.org>" imported
gpg: no ultimately trusted keys found
gpg: Total number processed: 1
gpg:               imported: 1
gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
gpg: Good signature from "Ludovic Courtès <ludo@gnu.org>"
gpg:                 aka "Ludovic Courtès <ludo@chbouib.org>"
gpg:                 aka "Ludovic Courtès <lcourtes@altern.org>"
gpg:                 aka "Ludovic Courtès (INRIA) <ludovic.courtes@inria.fr>"
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 83C4 F8E5 10A3 3B4C 5BEA  D15D 77DD 95E2 EA52 ECF4
gnu/packages/guile.scm:49:12: guile: updating from version 1.8.8 to version 2.0.9...

# ./pre-inst-env guix refresh -u --key-download=foo
guix refresh: error: unsupported policy: foo


[-- Attachment #1.2: 0001-guix-refresh-Add-key-download.patch --]
[-- Type: text/x-diff, Size: 12099 bytes --]

From 394191811139e66183309bc44979b146d6a9f969 Mon Sep 17 00:00:00 2001
From: Nikita Karetnikov <nikita@karetnikov.org>
Date: Fri, 7 Jun 2013 04:14:17 +0000
Subject: [PATCH] guix refresh: Add  '--key-download'.

* guix/gnu-maintenance.scm (download-tarball, package-update): Add
  'key-download'.
  guix/gnupg.scm (gnupg-verify*): Add 'key-download' and adjust
  'gnupg-verify*' accordingly.
  guix/scripts/refresh.scm (show-help, %options): Add and document
  '--key-download'.
  (update-package): Add 'key-download'.
  (guix-refresh): Adjust to handle 'key-download'.
---
 guix/gnu-maintenance.scm |   17 ++++++---
 guix/gnupg.scm           |   38 ++++++++++++++++++----
 guix/scripts/refresh.scm |   79 +++++++++++++++++++++++++++++-----------------
 3 files changed, 92 insertions(+), 42 deletions(-)

diff --git a/guix/gnu-maintenance.scm b/guix/gnu-maintenance.scm
index b54cd84..0f1a05b 100644
--- a/guix/gnu-maintenance.scm
+++ b/guix/gnu-maintenance.scm
@@ -341,16 +341,19 @@ pairs.  Example: (\"mit-scheme-9.0.1\" . \"/gnu/mit-scheme/stable.pkg/9.0.1\").
          (_ #f))))
 
 (define* (download-tarball store project directory version
-                           #:optional (archive-type "gz"))
+                           #:optional (archive-type "gz")
+                                      (key-download 'interactive))
   "Download PROJECT's tarball over FTP and check its OpenPGP signature.  On
-success, return the tarball file name."
+success, return the tarball file name.  KEY-DOWNLOAD specifies a download
+policy for missing OpenPGP keys; allowed values: INTERACTIVE (default),
+ALWAYS, and NEVER."
   (let* ((server  (ftp-server/directory project))
          (base    (string-append project "-" version ".tar." archive-type))
          (url     (string-append "ftp://" server "/" directory "/" base))
          (sig-url (string-append url ".sig"))
          (tarball (download-to-store store url))
          (sig     (download-to-store store sig-url)))
-    (let ((ret (gnupg-verify* sig tarball)))
+    (let ((ret (gnupg-verify* sig tarball key-download)))
       (if ret
           tarball
           (begin
@@ -359,9 +362,11 @@ success, return the tarball file name."
             (warning (_ "(could be because the public key is not in your keyring)~%"))
             #f)))))
 
-(define (package-update store package)
+(define* (package-update store package #:optional (key-download 'interactive))
   "Return the new version and the file name of the new version tarball for
-PACKAGE, or #f and #f when PACKAGE is up-to-date."
+PACKAGE, or #f and #f when PACKAGE is up-to-date.  KEY-DOWNLOAD specifies a
+download policy for missing OpenPGP keys; allowed values: ALWAYS, NEVER, and
+INTERACTIVE (default)."
   (match (package-update-path package)
     ((version . directory)
      (let-values (((name)
@@ -372,7 +377,7 @@ PACKAGE, or #f and #f when PACKAGE is up-to-date."
                               (file-extension (origin-uri source)))
                          "gz"))))
        (let ((tarball (download-tarball store name directory version
-                                        archive-type)))
+                                        archive-type key-download)))
          (values version tarball))))
     (_
      (values #f #f))))
diff --git a/guix/gnupg.scm b/guix/gnupg.scm
index c17a495..5396f20 100644
--- a/guix/gnupg.scm
+++ b/guix/gnupg.scm
@@ -21,7 +21,9 @@
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 i18n)
   #:use-module (srfi srfi-1)
+  #:use-module (guix ui)
   #:export (%gpg-command
             %openpgp-key-server
             gnupg-verify
@@ -145,16 +147,38 @@ missing key."
 (define (gnupg-receive-keys key-id server)
   (system* (%gpg-command) "--keyserver" server "--recv-keys" key-id))
 
-(define* (gnupg-verify* sig file #:optional (server (%openpgp-key-server)))
+(define* (gnupg-verify* sig file
+                        #:optional (key-download 'interactive)
+                                   (server (%openpgp-key-server)))
   "Like `gnupg-verify', but try downloading the public key if it's missing.
-Return #t if the signature was good, #f otherwise."
+Return #t if the signature was good, #f otherwise.  KEY-DOWNLOAD specifies a
+download policy for missing OpenPGP keys; allowed values: ALWAYS, NEVER, and
+INTERACTIVE (default)."
   (let ((status (gnupg-verify sig file)))
     (or (gnupg-status-good-signature? status)
         (let ((missing (gnupg-status-missing-key? status)))
-          (and missing
-               (begin
-                 ;; Download the missing key and try again.
-                 (gnupg-receive-keys missing server)
-                 (gnupg-status-good-signature? (gnupg-verify sig file))))))))
+          (define (download-and-try-again)
+            (begin
+              ;; Download the missing key and try again.
+              (gnupg-receive-keys missing server)
+              (gnupg-status-good-signature? (gnupg-verify sig file))))
+
+          (define (receive?)
+            (let ((answer
+                   (_ (begin (format #t "~a~a~%"
+                                     "Would you like to download this key "
+                                     "and add it to your keyring?")
+                             (read-line)))))
+              (string-match (locale-yes-regexp) answer)))
+
+          (and
+           missing
+           (case key-download
+             ((never) #f)
+             ((always)
+              (download-and-try-again))
+             (else
+              (and (receive?)
+                   (download-and-try-again)))))))))
 
 ;;; gnupg.scm ends here
diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 10715eb..f54b5ad 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,15 @@
         (option '("gpg") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'gpg-command arg result)))
+        (option '("key-download") #t #f
+                (lambda (opt name arg result)
+                  (match arg
+                    ((or "interactive" "always" "never")
+                     (alist-cons 'key-download (string->symbol arg)
+                                 result))
+                    (_
+                     (leave (_ "unsupported policy: ~a~%")
+                            arg)))))
 
         (option '(#\h "help") #f #f
                 (lambda args
@@ -90,6 +100,11 @@ specified with `--select'.\n"))
       --key-server=HOST  use HOST as the OpenPGP key server"))
   (display (_ "
       --gpg=COMMAND      use COMMAND as the GnuPG 2.x command"))
+  (display (_ "
+      --key-download=POLICY
+                         handle missing OpenPGP keys according to POLICY:
+                         'always', 'never', and 'interactive', which is also
+                         used when 'key-download' is not specified"))
   (newline)
   (display (_ "
   -h, --help             display this help and exit"))
@@ -98,12 +113,14 @@ specified with `--select'.\n"))
   (newline)
   (show-bug-report-information))
 
-(define (update-package store package)
-  "Update the source file that defines PACKAGE with the new version."
+(define* (update-package store package #:optional (key-download 'interactive))
+  "Update the source file that defines PACKAGE with the new version.
+KEY-DOWNLOAD specifies a download policy for missing OpenPGP keys; allowed
+values: INTERACTIVE (default), ALWAYS, and NEVER."
   (let-values (((version tarball)
                 (catch #t
                   (lambda ()
-                    (package-update store package))
+                    (package-update store package key-download))
                   (lambda _
                     (values #f #f))))
                ((loc)
@@ -161,31 +178,33 @@ update would trigger a complete rebuild."
         ;; XXX: Fails to catch MPFR/MPC, whose *source* is used as input.
         (member (package-name package) names))))
 
-  (let* ((opts     (parse-options))
-         (update?  (assoc-ref opts 'update?))
-         (packages (match (concatenate
-                           (filter-map (match-lambda
-                                        (('argument . value)
-                                         (let ((p (find-packages-by-name value)))
-                                           (unless p
-                                             (leave (_ "~a: no package by that name")
-                                                    value))
-                                           p))
-                                        (_ #f))
-                                       opts))
-                     (()                          ; default to all packages
-                      (let ((select? (match (assoc-ref opts 'select)
-                                       ('core core-package?)
-                                       ('non-core (negate core-package?))
-                                       (_ (const #t)))))
-                        ;; TODO: Keep only the newest of each package.
-                        (fold-packages (lambda (package result)
-                                         (if (select? package)
-                                             (cons package result)
-                                             result))
-                                       '())))
-                     (some                        ; user-specified packages
-                      some))))
+  (let* ((opts         (parse-options))
+         (update?      (assoc-ref opts 'update?))
+         (key-download (assoc-ref opts 'key-download))
+         (packages
+          (match (concatenate
+                  (filter-map (match-lambda
+                               (('argument . value)
+                                (let ((p (find-packages-by-name value)))
+                                  (unless p
+                                    (leave (_ "~a: no package by that name")
+                                           value))
+                                  p))
+                               (_ #f))
+                              opts))
+                 (()                          ; default to all packages
+                  (let ((select? (match (assoc-ref opts 'select)
+                                        ('core core-package?)
+                                        ('non-core (negate core-package?))
+                                        (_ (const #t)))))
+                    ;; TODO: Keep only the newest of each package.
+                    (fold-packages (lambda (package result)
+                                     (if (select? package)
+                                         (cons package result)
+                                         result))
+                                   '())))
+                 (some                        ; user-specified packages
+                  some))))
     (with-error-handling
       (if update?
           (let ((store (open-connection)))
@@ -195,7 +214,9 @@ update would trigger a complete rebuild."
                            (%gpg-command
                             (or (assoc-ref opts 'gpg-command)
                                 (%gpg-command))))
-              (for-each (cut update-package store <>) packages)))
+              (for-each
+               (cut update-package store <> key-download)
+               packages)))
           (for-each (lambda (package)
                       (match (false-if-exception (package-update-path package))
                         ((new-version . directory)
-- 
1.7.5.4


[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] guix refresh: Add  '--key-download'.
  2013-06-07  5:26                       ` [PATCH] guix refresh: Add '--key-download' Nikita Karetnikov
@ 2013-06-07 16:19                         ` Ludovic Courtès
  2013-06-08 11:19                           ` Nikita Karetnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Ludovic Courtès @ 2013-06-07 16:19 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> First the whole string should be enclosed in (_ ...), otherwise xgettext
>> will just extract "~a~a" for translation.
>
> Should I do the same here?
>
> +                  (match arg
> +                    ((or "interactive" "always" "never")
> +                     (alist-cons 'key-download (string->symbol arg)
> +                                 result))

No, command-line arguments are not i18n’d, so let’s stick to this.

>> Perhaps change it to
>
>>   #:key (key-download 'interactive)
>
> I've tried that, but things like (package-update #:key-download
> key-download) don't look right.  Here is a simplified example:
>
> ;; guix/scripts/refresh.scm
> (define* (update-package #:key (key-download 'interactive))
>   (package-update #:key-download key-download))

That’s fine, IMO.

In general, I would avoid having more than one optional arguments,
unless its meaning and position are obvious.

In this case, it’s not obvious IMO, so the best would probably to make
both ‘archive-type’ and ‘key-download’ keyword arguments.

> scheme@(guile-user)> (update-package)
> interactive
> scheme@(guile-user)> (update-package #:key-download 'never)
> never

It just occurred to me that it might be more intuitive to use one of

  'interactive
  #f   ; never download
  _    ; (any other value) always download

> I'm attaching a patch.  Examples (some commands were omitted):

The examples look great!

> # ./pre-inst-env guix refresh -u
>
> [...]
>
> ftp://ftp.gnu.org/.../guile-2.0.9.tar.gz.sig	100.0% of 0.2 KiB
> gpg: Signature made Wed 10 Apr 2013 06:14:45 AM UTC using DSA key ID EA52ECF4
> gpg: Can't check signature: No public key
> Would you like to download this key and add it to your keyring?
> n
> guix refresh: warning: signature verification failed for `guile-2.0.9.tar.gz'
> guix refresh: warning: (could be because the public key is not in your keyring)
>
> Should I prepend "guix refresh: " to the question?

No.

>  (define* (download-tarball store project directory version
> -                           #:optional (archive-type "gz"))
> +                           #:optional (archive-type "gz")
> +                                      (key-download 'interactive))

Rather make this #:key (key-download 'interactive).

> +(define* (package-update store package #:optional (key-download 'interactive))

Ditto.

> +          (and
> +           missing
> +           (case key-download
> +             ((never) #f)
> +             ((always)
> +              (download-and-try-again))
> +             (else
> +              (and (receive?)
> +                   (download-and-try-again)))))))))

I would write ‘missing’ on the same line as ‘and’.

Modulo these details, it seems ready to get it.

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] guix refresh: Add '--key-download'.
  2013-06-07 16:19                         ` Ludovic Courtès
@ 2013-06-08 11:19                           ` Nikita Karetnikov
  2013-06-08 14:48                             ` Ludovic Courtès
  0 siblings, 1 reply; 26+ messages in thread
From: Nikita Karetnikov @ 2013-06-08 11:19 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: bug-guix


[-- Attachment #1.1: Type: text/plain, Size: 637 bytes --]

> It just occurred to me that it might be more intuitive to use one of

>   'interactive
>   #f   ; never download
>   _    ; (any other value) always download

IMO, an attached version is better because we use high-level terms:
interactive, never, and always.

> Modulo these details, it seems ready to get it.

Can I push the attached version?

What should I do next?  For instance, I can change 'guix refresh' to
fetch signatures first and don't download tarballs that can't be
authenticated (when signatures are missing and 'never' is used).  Or I
can fix it not to mix version numbers (not to update Guile 1.8.8 to
version 2.0.9).


[-- Attachment #1.2: 0001-guix-refresh-Add-key-download.patch --]
[-- Type: text/x-diff, Size: 12858 bytes --]

From 911bd9c696b3104ac41f37dc0d2cf3741801d1d2 Mon Sep 17 00:00:00 2001
From: Nikita Karetnikov <nikita@karetnikov.org>
Date: Sat, 8 Jun 2013 10:35:11 +0000
Subject: [PATCH] guix refresh: Add '--key-download'.

* guix/gnu-maintenance.scm (download-tarball): Add a 'key-download'
  keyword argument and pass it to 'gnupg-verify*'.  Make
  'archive-type' a keyword argument.
  (package-update): Add a 'key-download' keyword argument.  Pass
  'archive-type' and 'key-download' keyword arguments to
  'download-tarball'.

* guix/gnupg.scm: Import (ice-9 i18n) and (guix ui).
  (gnupg-verify*): Add a 'key-download' keyword argument and adjust
  'gnupg-verify*' to use it.  Make 'server' a keyword argument.

* guix/scripts/refresh.scm (show-help, %options): Add and document
  '--key-download'.
  (update-package): Add a 'key-download' keyword argument and pass it
  to 'package-update'.
  (guix-refresh): Pass 'key-download' to 'update-package'.  Limit
  lines to a maximum of 79 characters.
---
 guix/gnu-maintenance.scm |   18 +++++++---
 guix/gnupg.scm           |   36 +++++++++++++++++---
 guix/scripts/refresh.scm |   79 +++++++++++++++++++++++++++++-----------------
 3 files changed, 92 insertions(+), 41 deletions(-)

diff --git a/guix/gnu-maintenance.scm b/guix/gnu-maintenance.scm
index b54cd84..ed446c4 100644
--- a/guix/gnu-maintenance.scm
+++ b/guix/gnu-maintenance.scm
@@ -341,16 +341,19 @@ pairs.  Example: (\"mit-scheme-9.0.1\" . \"/gnu/mit-scheme/stable.pkg/9.0.1\").
          (_ #f))))
 
 (define* (download-tarball store project directory version
-                           #:optional (archive-type "gz"))
+                           #:key (archive-type "gz")
+                                 (key-download 'interactive))
   "Download PROJECT's tarball over FTP and check its OpenPGP signature.  On
-success, return the tarball file name."
+success, return the tarball file name.  KEY-DOWNLOAD specifies a download
+policy for missing OpenPGP keys; allowed values: INTERACTIVE (default),
+ALWAYS, and NEVER."
   (let* ((server  (ftp-server/directory project))
          (base    (string-append project "-" version ".tar." archive-type))
          (url     (string-append "ftp://" server "/" directory "/" base))
          (sig-url (string-append url ".sig"))
          (tarball (download-to-store store url))
          (sig     (download-to-store store sig-url)))
-    (let ((ret (gnupg-verify* sig tarball)))
+    (let ((ret (gnupg-verify* sig tarball #:key-download key-download)))
       (if ret
           tarball
           (begin
@@ -359,9 +362,11 @@ success, return the tarball file name."
             (warning (_ "(could be because the public key is not in your keyring)~%"))
             #f)))))
 
-(define (package-update store package)
+(define* (package-update store package #:key (key-download 'interactive))
   "Return the new version and the file name of the new version tarball for
-PACKAGE, or #f and #f when PACKAGE is up-to-date."
+PACKAGE, or #f and #f when PACKAGE is up-to-date.  KEY-DOWNLOAD specifies a
+download policy for missing OpenPGP keys; allowed values: ALWAYS, NEVER, and
+INTERACTIVE (default)."
   (match (package-update-path package)
     ((version . directory)
      (let-values (((name)
@@ -372,7 +377,8 @@ PACKAGE, or #f and #f when PACKAGE is up-to-date."
                               (file-extension (origin-uri source)))
                          "gz"))))
        (let ((tarball (download-tarball store name directory version
-                                        archive-type)))
+                                        #:archive-type archive-type
+                                        #:key-download key-download)))
          (values version tarball))))
     (_
      (values #f #f))))
diff --git a/guix/gnupg.scm b/guix/gnupg.scm
index c17a495..40dc864 100644
--- a/guix/gnupg.scm
+++ b/guix/gnupg.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2010, 2011, 2013 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,7 +22,9 @@
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 i18n)
   #:use-module (srfi srfi-1)
+  #:use-module (guix ui)
   #:export (%gpg-command
             %openpgp-key-server
             gnupg-verify
@@ -145,16 +148,37 @@ missing key."
 (define (gnupg-receive-keys key-id server)
   (system* (%gpg-command) "--keyserver" server "--recv-keys" key-id))
 
-(define* (gnupg-verify* sig file #:optional (server (%openpgp-key-server)))
+(define* (gnupg-verify* sig file
+                        #:key (key-download 'interactive)
+                              (server (%openpgp-key-server)))
   "Like `gnupg-verify', but try downloading the public key if it's missing.
-Return #t if the signature was good, #f otherwise."
+Return #t if the signature was good, #f otherwise.  KEY-DOWNLOAD specifies a
+download policy for missing OpenPGP keys; allowed values: ALWAYS, NEVER, and
+INTERACTIVE (default)."
   (let ((status (gnupg-verify sig file)))
     (or (gnupg-status-good-signature? status)
         (let ((missing (gnupg-status-missing-key? status)))
+          (define (download-and-try-again)
+            ;; Download the missing key and try again.
+            (begin
+              (gnupg-receive-keys missing server)
+              (gnupg-status-good-signature? (gnupg-verify sig file))))
+
+          (define (receive?)
+            (let ((answer
+                   (_ (begin (format #t "~a~a~%"
+                                     "Would you like to download this key "
+                                     "and add it to your keyring?")
+                             (read-line)))))
+              (string-match (locale-yes-regexp) answer)))
+
           (and missing
-               (begin
-                 ;; Download the missing key and try again.
-                 (gnupg-receive-keys missing server)
-                 (gnupg-status-good-signature? (gnupg-verify sig file))))))))
+               (case key-download
+                 ((never) #f)
+                 ((always)
+                  (download-and-try-again))
+                 (else
+                  (and (receive?)
+                       (download-and-try-again)))))))))
 
 ;;; gnupg.scm ends here
diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 10715eb..e7eb578 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,15 @@
         (option '("gpg") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'gpg-command arg result)))
+        (option '("key-download") #t #f
+                (lambda (opt name arg result)
+                  (match arg
+                    ((or "interactive" "always" "never")
+                     (alist-cons 'key-download (string->symbol arg)
+                                 result))
+                    (_
+                     (leave (_ "unsupported policy: ~a~%")
+                            arg)))))
 
         (option '(#\h "help") #f #f
                 (lambda args
@@ -90,6 +100,11 @@ specified with `--select'.\n"))
       --key-server=HOST  use HOST as the OpenPGP key server"))
   (display (_ "
       --gpg=COMMAND      use COMMAND as the GnuPG 2.x command"))
+  (display (_ "
+      --key-download=POLICY
+                         handle missing OpenPGP keys according to POLICY:
+                         'always', 'never', and 'interactive', which is also
+                         used when 'key-download' is not specified"))
   (newline)
   (display (_ "
   -h, --help             display this help and exit"))
@@ -98,12 +113,14 @@ specified with `--select'.\n"))
   (newline)
   (show-bug-report-information))
 
-(define (update-package store package)
-  "Update the source file that defines PACKAGE with the new version."
+(define* (update-package store package #:key (key-download 'interactive))
+  "Update the source file that defines PACKAGE with the new version.
+KEY-DOWNLOAD specifies a download policy for missing OpenPGP keys; allowed
+values: INTERACTIVE (default), ALWAYS, and NEVER."
   (let-values (((version tarball)
                 (catch #t
                   (lambda ()
-                    (package-update store package))
+                    (package-update store package #:key-download key-download))
                   (lambda _
                     (values #f #f))))
                ((loc)
@@ -161,31 +178,33 @@ update would trigger a complete rebuild."
         ;; XXX: Fails to catch MPFR/MPC, whose *source* is used as input.
         (member (package-name package) names))))
 
-  (let* ((opts     (parse-options))
-         (update?  (assoc-ref opts 'update?))
-         (packages (match (concatenate
-                           (filter-map (match-lambda
-                                        (('argument . value)
-                                         (let ((p (find-packages-by-name value)))
-                                           (unless p
-                                             (leave (_ "~a: no package by that name")
-                                                    value))
-                                           p))
-                                        (_ #f))
-                                       opts))
-                     (()                          ; default to all packages
-                      (let ((select? (match (assoc-ref opts 'select)
-                                       ('core core-package?)
-                                       ('non-core (negate core-package?))
-                                       (_ (const #t)))))
-                        ;; TODO: Keep only the newest of each package.
-                        (fold-packages (lambda (package result)
-                                         (if (select? package)
-                                             (cons package result)
-                                             result))
-                                       '())))
-                     (some                        ; user-specified packages
-                      some))))
+  (let* ((opts         (parse-options))
+         (update?      (assoc-ref opts 'update?))
+         (key-download (assoc-ref opts 'key-download))
+         (packages
+          (match (concatenate
+                  (filter-map (match-lambda
+                               (('argument . value)
+                                (let ((p (find-packages-by-name value)))
+                                  (unless p
+                                    (leave (_ "~a: no package by that name")
+                                           value))
+                                  p))
+                               (_ #f))
+                              opts))
+                 (()                          ; default to all packages
+                  (let ((select? (match (assoc-ref opts 'select)
+                                        ('core core-package?)
+                                        ('non-core (negate core-package?))
+                                        (_ (const #t)))))
+                    ;; TODO: Keep only the newest of each package.
+                    (fold-packages (lambda (package result)
+                                     (if (select? package)
+                                         (cons package result)
+                                         result))
+                                   '())))
+                 (some                        ; user-specified packages
+                  some))))
     (with-error-handling
       (if update?
           (let ((store (open-connection)))
@@ -195,7 +214,9 @@ update would trigger a complete rebuild."
                            (%gpg-command
                             (or (assoc-ref opts 'gpg-command)
                                 (%gpg-command))))
-              (for-each (cut update-package store <>) packages)))
+              (for-each
+               (cut update-package store <> #:key-download key-download)
+               packages)))
           (for-each (lambda (package)
                       (match (false-if-exception (package-update-path package))
                         ((new-version . directory)
-- 
1.7.5.4


[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] guix refresh: Add '--key-download'.
  2013-06-08 11:19                           ` Nikita Karetnikov
@ 2013-06-08 14:48                             ` Ludovic Courtès
  0 siblings, 0 replies; 26+ messages in thread
From: Ludovic Courtès @ 2013-06-08 14:48 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: bug-guix

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> It just occurred to me that it might be more intuitive to use one of
>
>>   'interactive
>>   #f   ; never download
>>   _    ; (any other value) always download
>
> IMO, an attached version is better because we use high-level terms:
> interactive, never, and always.

Hmm yes but at the language level, ‘never’ is really #f.  Not big deal
though, so I’m fine either way.  (Actually, R6RS-style enums are probably
what we’d need here, but that’s OK.)

>> Modulo these details, it seems ready to get it.
>
> Can I push the attached version?
>
> What should I do next?  For instance, I can change 'guix refresh' to
> fetch signatures first and don't download tarballs that can't be
> authenticated (when signatures are missing and 'never' is used).

I don’t think it needs to be this fancy, IMO.

> Or I can fix it not to mix version numbers (not to update Guile 1.8.8
> to version 2.0.9).

This would be useful, because it’s a practical annoyance.

I’m not sure how to proceed.  The common pattern is packages that have
several series evolving in parallel, like GCC.  For those, we could add
something in the ‘properties’ field to say ‘dont-update’, and ‘guix
refresh’ could check for that item.

But what we really want to express is the fact that GCC 4.7.n should
only be upgraded to GCC 4.7.p, not GCC 4.8.

We could try to have a heuristic that guesses if we’re in such a
situation.  However that seems non-trivial, and possibly fragile.

Instead, the Grand Plan would be to have an additional field in
‘package’ records that provides info as to how to look for newer
versions.  Some of that info is currently in gnu-maintenance.scm (the
‘quirks’ variable).

I’m not sure about how to represent that info in a generic way.  There
are probably ideas to steal from Debian’s uscan and other distro tools.


Thoughts?

(Note: it’s OK, and perhaps even advisable, to start with the simplest
plan, and to leave the Grand Plan for later.)

>  (define* (download-tarball store project directory version
> -                           #:optional (archive-type "gz"))
> +                           #:key (archive-type "gz")
> +                                 (key-download 'interactive))
>    "Download PROJECT's tarball over FTP and check its OpenPGP signature.  On
> -success, return the tarball file name."
> +success, return the tarball file name.  KEY-DOWNLOAD specifies a download
> +policy for missing OpenPGP keys; allowed values: INTERACTIVE (default),
> +ALWAYS, and NEVER."

In the docstring, please write ‘interactive’, ‘always’, and ‘never’
lower-case and quoted, because these are not variable references.

(Same in other procedures.)

> +          (define (receive?)
> +            (let ((answer
> +                   (_ (begin (format #t "~a~a~%"
> +                                     "Would you like to download this key "
> +                                     "and add it to your keyring?")
> +                             (read-line)))))
> +              (string-match (locale-yes-regexp) answer)))

Should be:

  (let ((answer
          (begin
            (format #t (_ "Would you like..."))
            (read-line))))
    ...)

Please commit with these minor changes.

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2013-06-08 14:53 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-24 22:24 New “guix refresh” command Ludovic Courtès
2013-04-25 21:27 ` Ludovic Courtès
2013-04-26 16:16 ` Andreas Enge
2013-04-27  9:43   ` Ludovic Courtès
2013-04-27 10:11     ` Andreas Enge
2013-04-27 21:04       ` Ludovic Courtès
2013-04-27 21:14         ` Andreas Enge
2013-04-27 22:35           ` Ludovic Courtès
2013-04-29 21:27             ` Ludovic Courtès
2013-04-30 15:54               ` Andreas Enge
2013-05-07 19:03 ` Nikita Karetnikov
2013-05-07 22:21   ` Ludovic Courtès
2013-05-10  0:29     ` Nikita Karetnikov
2013-05-10 13:11       ` Ludovic Courtès
2013-05-10 22:54         ` Nikita Karetnikov
2013-05-11 10:10           ` Ludovic Courtès
2013-05-11 14:05             ` Nikita Karetnikov
2013-05-24 10:19               ` Nikita Karetnikov
2013-05-24 12:54                 ` Ludovic Courtès
2013-05-30  0:46                   ` Nikita Karetnikov
2013-06-01 15:55                     ` Ludovic Courtès
2013-06-02 22:29                       ` Ludovic Courtès
2013-06-07  5:26                       ` [PATCH] guix refresh: Add '--key-download' Nikita Karetnikov
2013-06-07 16:19                         ` Ludovic Courtès
2013-06-08 11:19                           ` Nikita Karetnikov
2013-06-08 14:48                             ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).