[bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channel intro commit.

[Maxime Devos]

Attila Lendvai schreef op zo 26-09-2021 om 12:19 [+0200]:
> * guix/git-authenticate.scm (authenticate-commit): Reword and extend the error
> message to point to the relevant part of the manual.
> (authenticate-repository): Explicitly authenticate the channel introduction
> commit, so that it's also rejected unless it is signed by an authorized
> key. Otherwise only the second commit would yield an error, which
> is confusing.
> ---
> 
> here's how i tested this:
> 
> i set up pulling from a local checkout of guix.
> in that branch i created a signed dummy commit, and added it as a channel
> introduction, replacing guix in my /etc/guix/channels.scm. then tried to
> guix pull, which worked.
> 
> then i added another dummy commit, which resulted in an error when pulling.
> 
> then i reset the branch back to only contain the first commit, and added
> this code that then resulted in an error even with a single commit.
> 
> i have encountered it while i was trying to set up my local checkout to
> test my patches on my live guix, and i was utterly confused why my commit
> was rejected as unauthenticated (i misunderstood how git-authenticate
> works).
> 
>  guix/git-authenticate.scm | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/guix/git-authenticate.scm b/guix/git-authenticate.scm
> index ab3fcd8b2f..7d66bf0754 100644
> --- a/guix/git-authenticate.scm
> +++ b/guix/git-authenticate.scm
> @@ -236,8 +236,8 @@ not specify anything, fall back to 
> DEFAULT-AUTHORIZATIONS."
>              (condition
>               (&unauthorized-commit-error (commit id)
>                                           (signing-key signing-key)))
> -            (formatted-message (G_ "commit ~a not signed by an authorized \
> -key: ~a")
> +            (formatted-message (G_ "commit ~a is signed by an unauthorized \
> +key: ~a\nSee info guix \"Specifying Channel Authorizations\".")
>                                 (oid->string id)
>                                 (openpgp-format-fingerprint
>                                  (openpgp-public-key-fingerprint
> @@ -424,7 +424,12 @@ denoting the authorized keys for commits whose parent 
> lack the
>          ;; If it's our first time, verify START-COMMIT's signature.
>          (when (null? authenticated-commits)
>            (verify-introductory-commit repository keyring
> -                                      start-commit signer))
> +                                      start-commit signer)
> +          ;; Explicitly authenticate the channel introduction commit, so that
> +          ;; it's also rejected unless it's signed by an authorized
> +          ;; key. Otherwise only the second commit would yield an error, 
> which
> +          ;; is confusing.
> +          (authenticate-commits repository (list start-commit)))

Could you add a test to tests/git-authenticate.scm, verifying the right comit
is reported?  (Maybe use unauthorized-commit-error?, guard and
authenticate-repository.)

I'm not sure explicitely validating the start commit is sufficient.  What 
happens
in the following scenario:

(Order of commits)
  0. start commit
  1. valid (already authenticated?) commit
  2. invalid commit
  3. invalid commit

Is commit 2 reported, or commit 3 reported?  I think commit 2 should be 
reported,
but from your messages on IRC, I think you saw commit 3 being reported?

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part