Subject: [bug#41767] [PATCH 4/9] channels: 'latest-channel-instance' authenticates Git checkouts.

From: Maxim Cournoyer <[email-masked]>
Date: Tue, 09 Jun 2020 13:49:24 -0400

Hello!

Ludovic Courtès <[email-masked]> writes:

> Fixes <https://bugs.gnu.org/22883>.

[...]

> +;; Channel introductions.  A "channel introduction" provides a commit/signer
> +;; pair that specifies the first commit of the authentication process as well
> +;; as its signer's fingerprint.  The pair must be signed by the signer of that
> +;; commit so that only them may emit this introduction.  Introductions are
> +;; used to bootstrap trust in a channel.
> +(define-record-type <channel-introduction>
> +  (make-channel-introduction first-signed-commit first-commit-signer
> +                             signature)
> +  channel-introduction?
> +  (first-signed-commit  channel-introduction-first-signed-commit) ;hex string
> +  (first-commit-signer  channel-introduction-first-commit-signer) ;bytevector
> +  (signature            channel-introduction-signature))          ;string
> +
> +(define %guix-channel-introduction
> +  ;; Introduction of the official 'guix channel.  The chosen commit is the
> +  ;; first one that introduces '.guix-authorizations' on the 'core-updates'
> +  ;; branch that was eventually merged in 'master'.  Any branch starting
> +  ;; before that commit cannot be merged or it will be rejected by 'guix pull'
> +  ;; & co.
> +  (make-channel-introduction
> +   "87a40d7203a813921b3ef0805c2b46c0026d6c31"
> +   (base16-string->bytevector
> +    (string-downcase
> +     (string-filter char-set:hex-digit            ;mbakke
> +                    "BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))
> +   #f))                   ;TODO: Add an intro signature so it can be exported.

The GnuPG key fingerprint is SHA1 derived, which isn't cryptographically
secure.  This doesn't mean fingerprints are unsafe *now* (given that
forging a key to match it isn't currently practical), but I don't think
we should create something *today* that relies on SHA1 for trust.  My
point is made moot by the fact that Git uses SHA1 too... but that's
another issue.  Just saying, but not blocking or requesting change, as I
don't have a good solution for that, short of patching GnuPG and Git.

[...]

> +    ;; When COMMITS is empty, it's either because AUTHENTICATED-COMMITS
> +    ;; contains END-COMMIT or because END-COMMIT is not a descendant of
> +    ;; START-COMMIT.  Check that.
> +    (if (null? commits)
> +        (match (commit-relation start-commit end-commit)
> +          ((or 'self 'ancestor 'descendant) #t)   ;nothing to do!
> +          ('unrelated
> +           (raise
> +            (condition
> +             (&message
> +              (message
> +               (format #f (G_ "'~a' is not related to introductory \
> +commit of channel '~a'~%")
> +                       (oid->string (commit-id end-commit))
> +                       (channel-name channel))))))))
> +        (begin
> +          (format (current-error-port)
> +                  (G_ "Authenticating channel '~a', \
> +commits ~a to ~a (~h new commits)...~%")
> +                  (channel-name channel)
> +                  (commit-short-id start-commit)
> +                  (commit-short-id end-commit)
> +                  (length commits))
> +
> +          ;; If it's our first time, verify CHANNEL's introductory commit.
> +          (when (null? authenticated-commits)
> +            (verify-introductory-commit repository
> +                                        (channel-introduction channel)
> +                                        keyring))
> +
> +          (call-with-progress-reporter reporter
> +            (lambda (report)
> +              (authenticate-commits repository commits
> +                                    #:keyring keyring
> +                                    #:report-progress report)))
> +
> +          (unless (null? commits)

That condition is already checked above, but OK to be defensive.

> +            (cache-authenticated-commit cache-key
> +                                        (oid->string
> +                                         (commit-id end-commit))))))))
> +
>  (define* (latest-channel-instance store channel
>                                    #:key (patches %patches)
>                                    starting-commit)
> @@ -225,6 +387,14 @@ relation to STARTING-COMMIT when provided."
>                  (update-cached-checkout (channel-url channel)
>                                          #:ref (channel-reference channel)
>                                          #:starting-commit starting-commit)))
> +    (if (channel-introduction channel)
> +        (authenticate-channel channel checkout commit)
> +        ;; TODO: Warn for all the channels once the authentication interface
> +        ;; is public.
> +        (when (guix-channel? channel)
> +          (warning (G_ "the code of channel '~a' cannot be authenticated~%")
> +                   (channel-name channel))))
> +

Perhaps the warning message could say why.

[...]

> +(unless (gpg+git-available?) (test-skip 1))
> +(test-assert "authenticate-channel, wrong first commit signer"
> +  (with-fresh-gnupg-setup (list %ed25519-public-key-file
> +                                %ed25519-secret-key-file
> +                                %ed25519bis-public-key-file
> +                                %ed25519bis-secret-key-file)
> +    (with-temporary-git-repository directory
> +        `((add ".guix-channel"
> +               ,(object->string
> +                 '(channel (version 0)
> +                           (keyring-reference "master"))))
> +          (add ".guix-authorizations"
> +               ,(object->string
> +                 `(authorizations (version 0)
> +                                  ((,(key-fingerprint
> +                                      %ed25519-public-key-file)
> +                                    (name "Charlie"))))))
> +          (add "signer.key" ,(call-with-input-file %ed25519-public-key-file
> +                               get-string-all))
> +          (commit "first commit"
> +                  (signer ,(key-fingerprint %ed25519-public-key-file))))
> +      (with-repository directory repository
> +        (let* ((commit1 (find-commit repository "first"))
> +               (intro   ((@@ (guix channels) make-channel-introduction)
> +                         (commit-id-string commit1)
> +                         (openpgp-public-key-fingerprint
> +                          (read-openpgp-packet
> +                           %ed25519bis-public-key-file)) ;different key
> +                         #f))                     ;no signature
> +               (channel (channel (name 'example)
> +                                 (url (string-append "file://" directory))
> +                                 (introduction intro))))
> +          (guard (c ((message? c)
> +                     (->bool (string-contains (condition-message c)
> +                                              "initial commit"))))
> +            (authenticate-channel channel directory
> +                                  (commit-id-string commit1)
> +                                  #:keyring-reference-prefix "")
> +            'failed))))))

Eh, I like what you did there :-)  Very expressive way to setup your
test environment.

So far LGTM.

Maxim