[bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers.

[Ludovic Courtès]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> I know it is irrelevant with the patch at hand.  Maybe not. :-)
>
>    1. Why this ’(not (not’ ?

This ensures the result is a Boolean.

[...]

>> +(define (check-narinfo-authorization narinfo)
>> +  "Print a warning when NARINFO is not signed by an authorized key."
>> +  (unless (valid-narinfo? narinfo)
>
> …I entered in this part – hence the look up (guix pki) ;-).  Well, my
> mistake is hard to reproduce outside of Guix development tree but
> ’valid-narinfo?’ returns false for more cases than just
> unauthorized-key.  Therefore, the hint could be misleading.

It’s true that ‘valid-narinfo?’ catches more cases, but the other cases
where it returns #f are situations where the substitute server is bogus.
So I chose to favor conciseness here.

> Since we are discussing about an helper, I would run ’signature-case’
> here in check-narinfo.  For example, if the case is 'unauthorized-key,
> then I would check is %acl-file exists.  Maybe display the full
> %acl-file explaining that the key is not in, etc.

Right, checking for ‘%acl-file’ is a good idea; I wouldn’t display its
contents though because that’d be intimidating and unhelpful IMO.

> Moreover, running “guix challenge coreutils” does not warn about
> anything […]

That’s on purpose:

--8<---------------cut here---------------start------------->8---
(define (compare-contents items servers)
  "Challenge the substitute servers whose URLs are listed in SERVERS by
comparing the hash of the substitutes of ITEMS that they serve.  Return the
list of <comparison-report> objects.

This procedure does not authenticate narinfos from SERVERS, nor does it verify
that they are signed by an authorized public keys.  The reason is that, by
definition, we may want to target unknown servers.  Furthermore, no risk is
taken since we do not import the archives."
--8<---------------cut here---------------end--------------->8---

> guix weather: warning: could not determine current substitute URLs; using 
> defaults
> computing 1 package derivations for x86_64-linux...
> looking for 2 store items on https://ci.guix.gnu.org...
> guix weather: error: open-file: Permission denied: "/etc/guix/acl"

Uh, it should be able to deal with it gracefully.

> Hum? Maybe I am doing something wrong…  The file /etc/guix/acl has the
> permission:
>
>     -rw-------   1 root root   528  acl

It’s normally world-readable.

> Is it incorrect?  Well, if all are allowed to read (chmod a+r) then
> there is not error.  And it displays the warning:
>
> guix weather: warning: could not determine current substitute URLs; using 
> defaults
>
> And that’s because the daemon is not supporting the operation.  This
> warning appears to me misleading: personally I think that I am
> misconfigured something when that’s not the case.  Instead, I would
> display:
>
>     warning: using defaults substitute URLs

Yes, good idea.

I’ll send v2 soonish.

Thanks for your feedback!

Ludo’.