[PATCH] pull: Add fine-grained control for `guix pull --allow-downgrades`.

  • Open
  • quality assurance status badge
Details
2 participants
  • pelzflorian (Florian Pelz)
  • Rostislav Svoboda
Owner
unassigned
Submitted by
Rostislav Svoboda
Severity
normal
R
R
Rostislav Svoboda wrote on 12 Apr 15:13 +0200
(name . Rostislav Svoboda)(address . Rostislav.Svoboda@gmail.com)
3dbbb59fdc650a20a0eb853a0d30aaccae1beae5.1712927299.git.Rostislav.Svoboda@gmail.com
Introduce the ability to specify channels for downgrades in `guix pull`,
enhancing security by enabling users to trust certain channels over
others. This update maintains backward compatibility and updates relevant
documentation.

* guix/scripts/pull.scm (allow-downgrades): Option accepts a list of
downgradable channels, add '-a' as its short version.
(%default-options): Remove validate-pull.
(channels-with-validations): New procedure.
* guix/channels.scm (latest-channel-instances): Signature change.
* doc/guix.texi (Invoking guix pull): Document changes.

Change-Id: If947a2453c520463d77da9591af9ac03e6472afc
---
doc/guix.texi | 21 ++++++----
guix/channels.scm | 61 +++++++++++++++++------------
guix/scripts/pull.scm | 89 +++++++++++++++++++++++++++++++++++++------
3 files changed, 127 insertions(+), 44 deletions(-)

Toggle diff (357 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 5827e0de14..6126c1b5ef 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -4565,15 +4565,22 @@ Invoking guix pull
Show which channel commit(s) would be used and what would be built or
substituted but do not actually do it.
-@item --allow-downgrades
-Allow pulling older or unrelated revisions of channels than those
-currently in use.
+@item --allow-downgrades[=channels]
+@itemx -a [channels]
+Allows pulling older or unrelated revisions of specified channels, or
+all channels if none are specified.
@cindex downgrade attacks, protection against
-By default, @command{guix pull} protects against so-called ``downgrade
-attacks'' whereby the Git repository of a channel would be reset to an
-earlier or unrelated revision of itself, potentially leading you to
-install older, known-vulnerable versions of software packages.
+By default, @command{guix pull} safeguards against so-called ``downgrade
+attacks``, where a channel's Git repository is reset to a previous or
+unrelated revision, potentially causing the installation of older,
+vulnerable software versions. Without specifying channels, this
+protection is disabled entirely, posing a security risk.
+
+It's advisable to permit downgrades only for channels you trust
+implicitly, such as those you maintain. For all other channels,
+including the official Guix channel, downgrade protection remains
+recommended.
@quotation Note
Make sure you understand its security implications before using
diff --git a/guix/channels.scm b/guix/channels.scm
index 66f3122f79..af5a0b26c4 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -497,26 +497,35 @@ (define (channel-instance-primary-url instance)
(define* (latest-channel-instances store channels
#:key
- (current-channels '())
- (authenticate? #t)
- (validate-pull
- ensure-forward-channel-update))
+ (channel-validation-pairs '())
+ (authenticate? #t))
"Return a list of channel instances corresponding to the latest checkouts of
CHANNELS and the channels on which they depend.
When AUTHENTICATE? is true, authenticate the subset of CHANNELS that has a
\"channel introduction\".
-CURRENT-CHANNELS is the list of currently used channels. It is compared
-against the newly-fetched instances of CHANNELS, and VALIDATE-PULL is called
-for each channel update and can choose to emit warnings or raise an error,
-depending on the policy it implements."
+CHANNEL-VALIDATION-PAIRS is a list of pairs of currently used channels with their
+respective validation procedures: (current-channel . validate-pull). The
+current-channel is compared against the newly-fetched instances of CHANNELS, and its
+validate-pull procedure is called for each channel update and can choose to emit
+warnings or raise an error, depending on the policy it implements."
(define (current-commit name)
- ;; Return the current commit for channel NAME.
- (any (lambda (channel)
- (and (eq? (channel-name channel) name)
- (channel-commit channel)))
- current-channels))
+ "Return the current commit for channel NAME."
+ (any (lambda (channel-with-validation)
+ (let ((channel (car channel-with-validation)))
+ (and (eq? (channel-name channel) name)
+ (channel-commit channel))))
+ channel-validation-pairs))
+
+ (define (current-validate-pull name)
+ "Return the desired validate-pull procedure for channel NAME."
+ (any (lambda (channel-with-validation)
+ (let ((channel (car channel-with-validation))
+ (validate-pull (cdr channel-with-validation)))
+ (and (eq? (channel-name channel) name)
+ validate-pull)))
+ channel-validation-pairs))
(define instance-name
(compose channel-name channel-instance-channel))
@@ -544,20 +553,22 @@ (define* (latest-channel-instances store channels
(if (and previous
(not (more-specific? channel previous)))
(loop rest previous-channels instances)
- (begin
+ (let ((current (current-commit (channel-name channel)))
+ (validate-pull (current-validate-pull (channel-name channel))))
+ ;; (format #t "channel '~a' is validated by '~a'~%"
+ ;; (channel-name channel) (procedure-name validate-pull))
(format (current-error-port)
(G_ "Updating channel '~a' from Git repository at '~a'...~%")
(channel-name channel)
(channel-url channel))
- (let* ((current (current-commit (channel-name channel)))
- (instance
- (latest-channel-instance store channel
- #:authenticate?
- authenticate?
- #:validate-pull
- validate-pull
- #:starting-commit
- current)))
+ (let ((instance
+ (latest-channel-instance store channel
+ #:authenticate?
+ authenticate?
+ #:validate-pull
+ validate-pull
+ #:starting-commit
+ current)))
(when authenticate?
;; CHANNEL is authenticated so we can trust the
;; primary URL advertised in its metadata and warn
@@ -1001,7 +1012,7 @@ (define latest-channel-instances*
(define* (latest-channel-derivation #:optional (channels %default-channels)
#:key
- (current-channels '())
+ (channel-validation-pairs '())
(validate-pull
ensure-forward-channel-update))
"Return as a monadic value the derivation that builds the profile for the
@@ -1010,7 +1021,7 @@ (define* (latest-channel-derivation #:optional (channels %default-channels)
(mlet %store-monad ((instances
(latest-channel-instances* channels
#:current-channels
- current-channels
+ channel-validation-pairs
#:validate-pull
validate-pull)))
(channel-instances->derivation instances)))
diff --git a/guix/scripts/pull.scm b/guix/scripts/pull.scm
index 58d3cd7e83..c662e88771 100644
--- a/guix/scripts/pull.scm
+++ b/guix/scripts/pull.scm
@@ -76,8 +76,7 @@ (define %default-options
(graft? . #t)
(debug . 0)
(verbosity . 1)
- (authenticate-channels? . #t)
- (validate-pull . ,ensure-forward-channel-update)))
+ (authenticate-channels? . #t)))
(define (show-help)
(display (G_ "Usage: guix pull [OPTION]...
@@ -94,7 +93,8 @@ (define (show-help)
(display (G_ "
--branch=BRANCH download the tip of the specified \"guix\" channel BRANCH"))
(display (G_ "
- --allow-downgrades allow downgrades to earlier channel revisions"))
+ -a, --allow-downgrades[=CHANNELS]
+ allow downgrades to earlier revisions of CHANNELS"))
(display (G_ "
--disable-authentication
disable channel authentication"))
@@ -176,10 +176,37 @@ (define %options
(option '("branch") #t #f
(lambda (opt name arg result)
(alist-cons 'ref `(branch . ,arg) result)))
- (option '("allow-downgrades") #f #f
+ (option '(#\a "allow-downgrades") #f #t
(lambda (opt name arg result)
- (alist-cons 'validate-pull warn-about-backward-updates
- result)))
+ (cond
+ ((string? arg)
+ ((compose
+ (cut alist-cons 'allow-downgrades <>
+ (alist-delete 'allow-downgrades result))
+ (cut append
+ (or (assoc-ref result 'allow-downgrades)
+ (list))
+ <>))
+ ;; Values may be also comma-separated. Possibilities:
+ ;; -a val1 -a val2,val3 -a val4 -aval5
+ (string-tokenize arg
+ (char-set-complement (char-set #\,)))))
+ ((boolean? arg)
+ ;; The command contains this option with no value
+ ;; specified, (`arg' is #f). We'll interpreted this as
+ ;; 'all channels can be downgraded'
+ (alist-cons 'allow-downgrades #t result))
+ (else
+ ((compose
+ (lambda (text)
+ (raise (condition (&message (message text)))))
+ (cut format #f <>
+ "You found a bug:" arg name
+ version system %guix-version
+ %guix-bug-report-address))
+ "~a The value '~a' of the '~a' option is unrecognized.
+(version: ~s; system: ~s; host version: ~s)
+Please report the COMPLETE output above by email to <~a>.~%")))))
(option '("disable-authentication") #f #f
(lambda (opt name arg result)
(alist-cons 'authenticate-channels? #f result)))
@@ -828,6 +855,41 @@ (define (validate-cache-directory-ownership)
@command{sudo -i} or equivalent if you really want to pull as ~a.")
dir:user our:user)))))))))))
+(define (channels-with-validations downgradable-candidates channels)
+ "Return a list of pairs: channel + pull-validation procedure. The procedure
+is `warn-about-backward-updates' if a given channel is among the
+DOWNGRADABLE-CANDIDATES or `ensure-forward-channel-update' otherwise. E.g.:
+
+((channel1 . #<procedure warn-about-backward-updates ...>)
+ (channel2 . #<procedure ensure-forward-channel-update ...>))"
+ (cond
+ ((and (list? downgradable-candidates) (not (null? downgradable-candidates)))
+ (let ((downgradables-candidate-names (map string->symbol
+ downgradable-candidates))
+ (channels-names (map channel-name channels)))
+ (map (lambda (name)
+ (unless (member name channels-names)
+ (leave (G_ "'~a' must be one of '~a~'%") name channels-names)))
+ downgradables-candidate-names)
+ (let* ((downgradables-names
+ (filter (cut member <> downgradables-candidate-names)
+ channels-names))
+ (downgradables
+ (filter (compose (cut member <> downgradables-names)
+ (cut channel-name <>))
+ channels))
+ (non-downgradables (lset-difference equal? channels
+ downgradables)))
+ (append
+ (map (cut cons <> warn-about-backward-updates) downgradables)
+ (map (cut cons <> ensure-forward-channel-update) non-downgradables)))))
+
+ ((and (boolean? downgradable-candidates) downgradable-candidates)
+ (map (cut cons <> warn-about-backward-updates) channels))
+
+ (else
+ (map (cut cons <> ensure-forward-channel-update) channels))))
+
(define-command (guix-pull . args)
(synopsis "pull the latest revision of Guix")
@@ -844,7 +906,7 @@ (define-command (guix-pull . args)
(dry-run? (assoc-ref opts 'dry-run?))
(profile (or (assoc-ref opts 'profile) %current-profile))
(current-channels (profile-channels profile))
- (validate-pull (assoc-ref opts 'validate-pull))
+ (allow-downgrades (assoc-ref opts 'allow-downgrades))
(authenticate? (assoc-ref opts 'authenticate-channels?)))
(cond
((assoc-ref opts 'query)
@@ -868,14 +930,17 @@ (define-command (guix-pull . args)
(set-build-options-from-command-line store opts)
(ensure-default-profile)
(honor-x509-certificates store)
-
(let* ((channels (channel-list opts))
+ (channel-validation-pairs
+ ;; Only current-channels can be checked against
+ ;; downgrade-attacks. New channels can't be
+ ;; downgraded. Their commit history is unknown yet.
+ (channels-with-validations allow-downgrades
+ current-channels))
(instances
(latest-channel-instances store channels
- #:current-channels
- current-channels
- #:validate-pull
- validate-pull
+ #:channel-validation-pairs
+ channel-validation-pairs
#:authenticate?
authenticate?)))
(format (current-error-port)

base-commit: 7af70efd7633b0d70091762cf43ce01a86176e8e
prerequisite-patch-id: e64f0d27446c1c560ad851f367a2472c14a7037e
prerequisite-patch-id: dfa2d04882577e60f7d473731e434454b8852644
prerequisite-patch-id: cc23ce978964d00cd66167c9465795838079d103
prerequisite-patch-id: f802f9482cfdb9b3b403616d8c2e91a252eee72c
prerequisite-patch-id: f1fd30f4906d43b4a0cbd16d45407cb75a3af0d5
prerequisite-patch-id: 375dece6d1cab20d1f4b4185c872634faa63d877
prerequisite-patch-id: 46680596f20c72c67fd869c057fb7eb0904f3bc9
prerequisite-patch-id: 866db02be533978a7953f5404a01335ae9434cb0
prerequisite-patch-id: e0e407612802204a5a17ae9678b78f13a6957a4a
prerequisite-patch-id: 1d7d9a6c7af37a60d9b24ec5b6ddfcb63bfa3658
prerequisite-patch-id: c54d19e7e00697430d955942249f8ac06a0d5e0d
prerequisite-patch-id: c51a937244a2bfd3098b14dacea70820e4175cd3
prerequisite-patch-id: 0d64205fece7716a15913d1b5b1e6264542c3e6b
prerequisite-patch-id: 48a4c1d593f45b030b20d21424423b18fc628be5
prerequisite-patch-id: 5a7198d3c2e5c1711c707875657886fada86045c
prerequisite-patch-id: a5426596293ad72222b40b165208718ac360f076
prerequisite-patch-id: 3d69d13f9f454733518329cf6c570dd3ad4e8ec4
prerequisite-patch-id: 9359f1cfb68b8f2c251db5c79cacd696ab7a61c7
prerequisite-patch-id: e3c63b7b5415c0defbb04b6332d7fe9c0f9f92d1
prerequisite-patch-id: 6820bafc717331edb240b9c5d0e6ea9b56f0d268
prerequisite-patch-id: 819a40d73fd85f0abbb6de717702cc350a638812
prerequisite-patch-id: 389cfca519329cbc85ed6cf9e4f19457584e7113
prerequisite-patch-id: 93ae07255880b16be653369b88ac4bab01531de4
prerequisite-patch-id: c4ae064694c171c4709e0bbbce7dcae9a3ed3640
prerequisite-patch-id: 8e3e598481b5985c08e5e0b47064b517b7303ec8
prerequisite-patch-id: 01933610286e38850935f6832db66bc68ce867d9
prerequisite-patch-id: c46d5e95188b94c38b0effb5eacbe9d645c88625
prerequisite-patch-id: 9f7f301a7e1617f3edd6bfaa89e99b33924a6041
prerequisite-patch-id: ca097b601f748da69c706eaf70ca94e66cd80494
prerequisite-patch-id: fb9f6159a1e6de7c5866068731156d459ad33b62
prerequisite-patch-id: 4f42d2dc345f4e5d534be7cf491a6955f06d9ebe
prerequisite-patch-id: 52a3ecab13d8feacab75727f9a09d5ab108d1a23
prerequisite-patch-id: 25b017882ab15f59a8b2c1613ab321795a17dde9
prerequisite-patch-id: 6d7299f9a81a7a93e47b38c00e0b1d924ce8b687
prerequisite-patch-id: d192b54857d9029fa7f88dae85aa4a3b5163c332
prerequisite-patch-id: 92919e6803391635e63c33196c2a13eaf8ebc283
prerequisite-patch-id: ed7ba0aab3ec834bcfbfd67281392e49d9cf67a5
prerequisite-patch-id: 82406c3bc8cfee6acc8c8079015070f283fe79e3
prerequisite-patch-id: a59d1f2dc396fb7a2d1b4d51d83b4f77659fd9c7
prerequisite-patch-id: 8fefac2b53befd391dff2c96f4c6a3699a3060d5
prerequisite-patch-id: 9ddab69dc4619551f7334fe4b400e592ed4393a7
prerequisite-patch-id: 0deaa2b9c7bc444b22053af11b234ca4ccf16399
prerequisite-patch-id: e0a866813a66138498944da75bdad86f596bdb7c
prerequisite-patch-id: 62736ca29e36894d18f65532cc28cfdc7336846c
prerequisite-patch-id: b586f071a3faa5dcab26a252e9e378ff7e9a0687
prerequisite-patch-id: 1812d030ccb0890fb5aa1f9397ed82fb21479825
prerequisite-patch-id: 31dd5fb7a5b242e8263d570162db341147211da6
prerequisite-patch-id: 3883ed174c39f0fc7931f50c71f2060504ff462a
prerequisite-patch-id: e90161d1e23ff002d4f2d857e596ba65eec483d8
prerequisite-patch-id: 61d34657ffbd7381cbde53c679129dc255a42ef9
prerequisite-patch-id: f17d9fa7c863667737420d2541bcec4537515e8b
prerequisite-patch-id: ff994a1d3e932e75eb88747bc1933690da835dba
prerequisite-patch-id: f3b5d769de6ad0adf1b416f0a6f0a2bbaf7ed223
prerequisite-patch-id: b646681f04f59b46dea1f7c6d4344578e0bd26ee
prerequisite-patch-id: 96aa449e0b10733f455552f96c3665cc52a2d8da
prerequisite-patch-id: b21ad691e53f0c96b6412e176413f904f5c8f46e
prerequisite-patch-id: 28a9474b4f516613c1e73504b44bb0716505bff1
prerequisite-patch-id: 7c7d301e9827ddb4bbd5054e4d227e02561c4cf2
prerequisite-patch-id: 0801807425306d34c12e0718fd67973092e84b56
prerequisite-patch-id: c059905a50861b9c0e0c3c0359db4847c6bb3386
prerequisite-patch-id: abb0c728d67f1e57a87d5cbfec93bb77b4a766c2
prerequisite-patch-id: 823fb528dee836deec3c5154a23167d773f4bbb3
prerequisite-patch-id: 11a7b07fbbbb45cbdea63321fa5657a3037a69b8
prerequisite-patch-id: 1900015cfa7370761c371d243038caa8e0576d05
prerequisite-patch-id: b724fddb5e4f81644384296be36f695c48d866df
prerequisite-patch-id: ab85ac4bd58ecfcc65ce6b003a48530172040ddb
prerequisite-patch-id: 653a88a96f1c8bf90f82e4d0ee5828e8e417ca69
prerequisite-patch-id: ecd314828713060dbe48b00d6962adcf58a4d419
prerequisite-patch-id: 833f9bd5e73f7119d93de182c39dfb96fd6b7863
prerequisite-patch-id: 00bc17f527ee712025e18cdc5469de2b59a04fcd
prerequisite-patch-id: 4015e7d087d5f7b9248fe4c8b0a28a05c3af1c02
prerequisite-patch-id: 8d82f9e6e183647a0bf71e23815e2941e44a3f2b
prerequisite-patch-id: 6474ef2249845b7bfafc6165e31a1de6c9dfcf18
prerequisite-patch-id: 532fa26ec1e1eddead55a04e7dd81d336434cec6
prerequisite-patch-id: d0c7f0a3c9c701c752390e7f8874b831a51b4721
prerequisite-patch-id: 5e832cefe1aa9bfe819544fb306c6b23c2c9e7f0
prerequisite-patch-id: d66e51cdcff3ede7b23d5091984a1aade854ac98
prerequisite-patch-
This message was truncated. Download the full message here.
R
R
Rostislav Svoboda wrote on 12 Apr 21:43 +0200
CAEtmmewJTCSnE+Fn2-bxdA8jWJEwVYDKHX8rnHHx8y5YczOQaQ@mail.gmail.com
Argh, the patch flawed. Please ignore it for now. Sorry.
Cheers, Bost
R
P
P
pelzflorian (Florian Pelz) wrote on 16 Apr 10:16 +0200
(name . Rostislav Svoboda)(address . rostislav.svoboda@gmail.com)
87sezl90yc.fsf@pelzflorian.de
Hello Rostislav. I’m the wrong person to judge Guix core patches’
implications. I have run

./pre-inst-env etc/teams.scm cc-members \
0002-pull-Add-fine-grained-control-for-guix-pull-allow-do.patch

which fails, so I run format-patch again and it no longer fails and I
Cc’d the listed people now.

Otherwise, I can say that in comments and doc/guix.texi changes, you
should put two spaces after sentences (typewriter sentence spacing).

I do not know if core teams maintainers are fine with putting procedures
in cons cells given to channels-with-validations, or if you better use
symbols, but I cannot judge really.

Regards,
Florian
R
R
Rostislav Svoboda wrote on 18 Apr 13:42 +0200
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
CAEtmmewOLvv23g7nrvAC-gHJRtprnhrGkpcn29Mx=4aq0SKD0g@mail.gmail.com
Hello Florian,

Toggle quote (7 lines)
> I have run
>
> ./pre-inst-env etc/teams.scm cc-members \
> 0002-pull-Add-fine-grained-control-for-guix-pull-allow-do.patch
>
> which fails, so I run format-patch again and it no longer fails

Ah. Thank you.

Toggle quote (2 lines)
> putting procedures in cons cells [... or ...] better use symbols

I asked ChatGPT4 about it. It thinks it's fine ;-)
It claims (as I expected) "When you place a procedure in a cons cell,
you are not copying the procedure itself into the cell. Instead, you
store a reference (or pointer) to the procedure." and "security
implications are more about the integrity and trustworthiness of the
procedures themselves"

So yeah, good job from me, right? :-)

Cheers Bost
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 70353@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 70353
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch