* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
@ 2021-01-30 1:04 Leo Famulari
2021-03-11 0:14 ` zimoun
2021-03-11 22:29 ` Ludovic Courtès
0 siblings, 2 replies; 9+ messages in thread
From: Leo Famulari @ 2021-01-30 1:04 UTC (permalink / raw)
To: 46182
We could also make it warn about use of the HTTP protocol (as opposed to
HTTPS). Your thoughts?
* guix/lint.scm (check-git-protocol): New procedure.
(%local-checkers): Add 'git-protocol' checker.
* doc/guix.texi (Invoking guix lint): Document it.
---
doc/guix.texi | 6 +++++-
guix/lint.scm | 25 ++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index ff9e8da2e0..d17e2f2e96 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -28,7 +28,7 @@ Copyright @copyright{} 2014, 2015, 2016 Alex Kost@*
Copyright @copyright{} 2015, 2016 Mathieu Lirzin@*
Copyright @copyright{} 2014 Pierre-Antoine Rault@*
Copyright @copyright{} 2015 Taylan Ulrich Bayırlı/Kammer@*
-Copyright @copyright{} 2015, 2016, 2017, 2019, 2020 Leo Famulari@*
+Copyright @copyright{} 2015, 2016, 2017, 2019, 2020, 2021 Leo Famulari@*
Copyright @copyright{} 2015, 2016, 2017, 2018, 2019, 2020 Ricardo Wurmus@*
Copyright @copyright{} 2016 Ben Woodcroft@*
Copyright @copyright{} 2016, 2017, 2018 Chris Marusich@*
@@ -11736,6 +11736,10 @@ Parse the @code{source} URL to determine if a tarball from GitHub is
autogenerated or if it is a release tarball. Unfortunately GitHub's
autogenerated tarballs are sometimes regenerated.
+@item git-protocol
+Check if the package's source code is fetched using the insecure @code{git://}
+protocol.
+
@item derivation
Check that the derivation of the given packages can be successfully
computed for all the supported systems (@pxref{Derivations}).
diff --git a/guix/lint.scm b/guix/lint.scm
index 311bc94cc3..5a609b0454 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -11,6 +11,7 @@
;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net>
;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com>
;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com>
+;;; Copyright © 2021 Leo Famulari <leo@famulari.name>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -51,7 +52,7 @@
#:use-module (guix gnu-maintenance)
#:use-module (guix cve)
#:use-module ((guix swh) #:hide (origin?))
- #:autoload (guix git-download) (git-reference?
+ #:autoload (guix git-download) (git-reference? git-fetch
git-reference-url git-reference-commit)
#:use-module (guix import stackage)
#:use-module (ice-9 match)
@@ -84,6 +85,7 @@
check-source
check-source-file-name
check-source-unstable-tarball
+ check-git-protocol
check-mirror-url
check-github-url
check-license
@@ -918,6 +920,23 @@ descriptions maintained upstream."
(origin-uris origin))
'())))
+(define (check-git-protocol package)
+ "Emit a warning if PACKAGE's source URI protocol is 'git://'."
+ (define (check-source-uri-scheme uri)
+ (if (eqv? (uri-scheme uri) 'git)
+ (list
+ (make-warning package
+ (G_ "the source URI should not use the git:// protocol")
+ #:field 'source))
+ '()))
+
+ (let ((origin (package-source package)))
+ (if (and (origin? origin)
+ (eqv? (origin-method origin) git-fetch))
+ (check-source-uri-scheme
+ (string->uri (git-reference-url (origin-uri origin))))
+ '())))
+
(define (check-mirror-url package)
"Check whether PACKAGE uses source URLs that should be 'mirror://'."
(define (check-mirror-uri uri) ;XXX: could be optimized
@@ -1476,6 +1495,10 @@ or a list thereof")
(name 'source-unstable-tarball)
(description "Check for autogenerated tarballs")
(check check-source-unstable-tarball))
+ (lint-checker
+ (name 'git-protocol)
+ (description "Check for use of the git:// protocol")
+ (check check-git-protocol))
(lint-checker
(name 'derivation)
(description "Report failure to compile a package to a derivation")
--
2.30.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2021-01-30 1:04 [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker Leo Famulari
@ 2021-03-11 0:14 ` zimoun
2021-03-11 1:46 ` Leo Famulari
2021-03-11 22:29 ` Ludovic Courtès
1 sibling, 1 reply; 9+ messages in thread
From: zimoun @ 2021-03-11 0:14 UTC (permalink / raw)
To: Leo Famulari; +Cc: 46182
[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]
Hi Leo,
Giving a look to the bug tracker for the next release, I see this
bug. :-)
On Fri, 29 Jan 2021 at 20:04, Leo Famulari <leo@famulari.name> wrote:
> We could also make it warn about use of the HTTP protocol (as opposed to
> HTTPS). Your thoughts?
>
> * guix/lint.scm (check-git-protocol): New procedure.
> (%local-checkers): Add 'git-protocol' checker.
> * doc/guix.texi (Invoking guix lint): Document it.
The doc/ does not apply anymore.
Instead of these ’eqv?’
> +(define (check-git-protocol package)
> + "Emit a warning if PACKAGE's source URI protocol is 'git://'."
> + (define (check-source-uri-scheme uri)
> + (if (eqv? (uri-scheme uri) 'git)
[...]
> + (let ((origin (package-source package)))
> + (if (and (origin? origin)
> + (eqv? (origin-method origin) git-fetch))
> + (check-source-uri-scheme
> + (string->uri (git-reference-url (origin-uri origin))))
> + '())))
I propose ’match’ which is more coherent with the Guix style. Well,
from my understanding. :-)
Patch attached. Well, it could be nice to add a test in
tests/guix-lint.sh for that. WDYT?
Cheers,
simon
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lint-git-protocol.patch --]
[-- Type: text/x-diff, Size: 2155 bytes --]
diff --git a/guix/lint.scm b/guix/lint.scm
index 311bc94cc3..980f77c736 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -51,7 +51,7 @@
#:use-module (guix gnu-maintenance)
#:use-module (guix cve)
#:use-module ((guix swh) #:hide (origin?))
- #:autoload (guix git-download) (git-reference?
+ #:autoload (guix git-download) (git-reference? git-fetch
git-reference-url git-reference-commit)
#:use-module (guix import stackage)
#:use-module (ice-9 match)
@@ -84,6 +84,7 @@
check-source
check-source-file-name
check-source-unstable-tarball
+ check-git-protocol
check-mirror-url
check-github-url
check-license
@@ -918,6 +919,26 @@ descriptions maintained upstream."
(origin-uris origin))
'())))
+(define (check-git-protocol package)
+ "Emit a warning if PACKAGE's source URI protocol is 'git://'."
+ (define (check-source-uri-scheme uri)
+ (match (uri-scheme uri)
+ ('git
+ (list
+ (make-warning package
+ (G_ "the source URI should not use the git:// protocol")
+ #:field 'source)))
+ (_ '())))
+
+ (match (package-source package)
+ ((? origin? origin)
+ (match (origin-method origin)
+ (git-fetch
+ (check-source-uri-scheme
+ (string->uri (git-reference-url (origin-uri origin)))))
+ (_ '())))
+ (_ '())))
+
(define (check-mirror-url package)
"Check whether PACKAGE uses source URLs that should be 'mirror://'."
(define (check-mirror-uri uri) ;XXX: could be optimized
@@ -1476,6 +1497,10 @@ or a list thereof")
(name 'source-unstable-tarball)
(description "Check for autogenerated tarballs")
(check check-source-unstable-tarball))
+ (lint-checker
+ (name 'git-protocol)
+ (description "Check for use of the git:// protocol")
+ (check check-git-protocol))
(lint-checker
(name 'derivation)
(description "Report failure to compile a package to a derivation")
^ permalink raw reply related [flat|nested] 9+ messages in thread* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2021-03-11 0:14 ` zimoun
@ 2021-03-11 1:46 ` Leo Famulari
2021-03-11 9:44 ` zimoun
0 siblings, 1 reply; 9+ messages in thread
From: Leo Famulari @ 2021-03-11 1:46 UTC (permalink / raw)
To: zimoun; +Cc: 46182
On Thu, Mar 11, 2021 at 01:14:33AM +0100, zimoun wrote:
> I propose ’match’ which is more coherent with the Guix style. Well,
> from my understanding. :-)
I have heard that before, but I don't know how to use it 🤷
If this new patch is working for you, please push!
^ permalink raw reply [flat|nested] 9+ messages in thread
* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2021-03-11 1:46 ` Leo Famulari
@ 2021-03-11 9:44 ` zimoun
2023-10-20 2:22 ` Maxim Cournoyer
0 siblings, 1 reply; 9+ messages in thread
From: zimoun @ 2021-03-11 9:44 UTC (permalink / raw)
To: Leo Famulari; +Cc: 46182
Hi Leo,
On Wed, 10 Mar 2021 at 20:46, Leo Famulari <leo@famulari.name> wrote:
> On Thu, Mar 11, 2021 at 01:14:33AM +0100, zimoun wrote:
>> I propose ’match’ which is more coherent with the Guix style. Well,
>> from my understanding. :-)
>
> I have heard that before, but I don't know how to use it 🤷
The section [1] in the manual is worth to read because running and
playing with the examples gives a good feeling on how to use it. :-)
> If this new patch is working for you, please push!
I do not have this power. :-)
Cheers,
simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2021-03-11 9:44 ` zimoun
@ 2023-10-20 2:22 ` Maxim Cournoyer
2023-10-20 12:45 ` Simon Tournier
0 siblings, 1 reply; 9+ messages in thread
From: Maxim Cournoyer @ 2023-10-20 2:22 UTC (permalink / raw)
To: zimoun; +Cc: 46182, Leo Famulari
Hello,
zimoun <zimon.toutoune@gmail.com> writes:
> Hi Leo,
>
> On Wed, 10 Mar 2021 at 20:46, Leo Famulari <leo@famulari.name> wrote:
>> On Thu, Mar 11, 2021 at 01:14:33AM +0100, zimoun wrote:
>>> I propose ’match’ which is more coherent with the Guix style. Well,
>>> from my understanding. :-)
>>
>> I have heard that before, but I don't know how to use it 🤷
>
> The section [1] in the manual is worth to read because running and
> playing with the examples gives a good feeling on how to use it. :-)
>
>> If this new patch is working for you, please push!
>
> I do not have this power. :-)
No longer true ;-).
Thinking about this change though; why is it bad to fetch from git
places? There may be repos out there where it's the only offered way,
and as long as we're talking fixed output derivations, it seems moot
whether you use HTTPS, HTTP or X to retrieve the files (unless you are
worried about your traffic being monitored, but that's not in scope, I'd
say).
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 9+ messages in thread
* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2023-10-20 2:22 ` Maxim Cournoyer
@ 2023-10-20 12:45 ` Simon Tournier
2023-10-20 15:37 ` Maxim Cournoyer
0 siblings, 1 reply; 9+ messages in thread
From: Simon Tournier @ 2023-10-20 12:45 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 46182, Leo Famulari
Hi Maxim,
On Thu, 19 Oct 2023 at 22:22, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
> Thinking about this change though; why is it bad to fetch from git
> places? There may be repos out there where it's the only offered way,
> and as long as we're talking fixed output derivations, it seems moot
> whether you use HTTPS, HTTP or X to retrieve the files (unless you are
> worried about your traffic being monitored, but that's not in scope, I'd
> say).
Why would not it be in scope?
Being able to strongly verify (sha256) that the content you fetch is the
data you expect does not imply that the protocol for communicating
cannot be exploited for other means.
Well, git:// protocol is not supported by well-known forges. Quoting
Pro Git book:
The Cons
Due to the lack of TLS or other cryptography, cloning over
git:// might lead to an arbitrary code execution vulnerability,
and should therefore be avoided unless you know what you are
doing.
https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols
And I do not have enough imagination to find a way to exploit the git://
protocol. However, it appears to me a good practise to warn when this
protocol is used. Somehow, a lint message is a recommendation – a good
practise – and not an absolute truth. :-)
In short, from my point of view, the general rule reads: avoid git://
protocol if you can. Obviously, if you cannot because it is the only
offered way by some repositories, then let make an exception; but it
does mean that’s a good practise.
Cheers,
simon
^ permalink raw reply [flat|nested] 9+ messages in thread* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2023-10-20 12:45 ` Simon Tournier
@ 2023-10-20 15:37 ` Maxim Cournoyer
0 siblings, 0 replies; 9+ messages in thread
From: Maxim Cournoyer @ 2023-10-20 15:37 UTC (permalink / raw)
To: Simon Tournier; +Cc: 46182, Leo Famulari
Hi,
Simon Tournier <zimon.toutoune@gmail.com> writes:
> Hi Maxim,
>
> On Thu, 19 Oct 2023 at 22:22, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> Thinking about this change though; why is it bad to fetch from git
>> places? There may be repos out there where it's the only offered way,
>> and as long as we're talking fixed output derivations, it seems moot
>> whether you use HTTPS, HTTP or X to retrieve the files (unless you are
>> worried about your traffic being monitored, but that's not in scope, I'd
>> say).
>
> Why would not it be in scope?
>
> Being able to strongly verify (sha256) that the content you fetch is the
> data you expect does not imply that the protocol for communicating
> cannot be exploited for other means.
>
> Well, git:// protocol is not supported by well-known forges. Quoting
> Pro Git book:
>
> The Cons
>
> Due to the lack of TLS or other cryptography, cloning over
> git:// might lead to an arbitrary code execution vulnerability,
> and should therefore be avoided unless you know what you are
> doing.
>
> https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols
>
> And I do not have enough imagination to find a way to exploit the git://
> protocol. However, it appears to me a good practise to warn when this
> protocol is used. Somehow, a lint message is a recommendation – a good
> practise – and not an absolute truth. :-)
>
> In short, from my point of view, the general rule reads: avoid git://
> protocol if you can. Obviously, if you cannot because it is the only
> offered way by some repositories, then let make an exception; but it
> does mean that’s a good practise.
OK, fair. I remove my objection, but I dislike warnings when they
cannot be acted upon (e.g. 'no coverage in software heritage' -- OK
neat, but I can't do anything about it, and it may not even support that
tarball ingestion yet).
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 9+ messages in thread
* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2021-01-30 1:04 [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker Leo Famulari
2021-03-11 0:14 ` zimoun
@ 2021-03-11 22:29 ` Ludovic Courtès
2022-05-22 4:15 ` Maxim Cournoyer
1 sibling, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2021-03-11 22:29 UTC (permalink / raw)
To: Leo Famulari; +Cc: 46182
Hi!
Leo Famulari <leo@famulari.name> skribis:
> We could also make it warn about use of the HTTP protocol (as opposed to
> HTTPS). Your thoughts?
>
> * guix/lint.scm (check-git-protocol): New procedure.
> (%local-checkers): Add 'git-protocol' checker.
> * doc/guix.texi (Invoking guix lint): Document it.
Nice! I think it’s OK to use ‘eqv?’ here (‘eq?’, even).
One nit: it would be nice to add a positive and a negative test in
tests/lint.scm. You can run “make check TESTS=tests/lint.scm” then.
Otherwise LGTM!
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker.
2021-03-11 22:29 ` Ludovic Courtès
@ 2022-05-22 4:15 ` Maxim Cournoyer
0 siblings, 0 replies; 9+ messages in thread
From: Maxim Cournoyer @ 2022-05-22 4:15 UTC (permalink / raw)
To: Leo Famulari; +Cc: Ludovic Courtès, GNU Debbugs, 46182
tags 46182 +moreinfo
thanks
Hello,
Ludovic Courtès <ludo@gnu.org> writes:
> Hi!
>
> Leo Famulari <leo@famulari.name> skribis:
>
>> We could also make it warn about use of the HTTP protocol (as opposed to
>> HTTPS). Your thoughts?
>>
>> * guix/lint.scm (check-git-protocol): New procedure.
>> (%local-checkers): Add 'git-protocol' checker.
>> * doc/guix.texi (Invoking guix lint): Document it.
>
> Nice! I think it’s OK to use ‘eqv?’ here (‘eq?’, even).
>
> One nit: it would be nice to add a positive and a negative test in
> tests/lint.scm. You can run “make check TESTS=tests/lint.scm” then.
>
> Otherwise LGTM!
Gentle ping to Leo :-).
It looks near ready.
Thank you!
Maxim
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-10-20 15:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-30 1:04 [bug#46182] [PATCH] lint: Add 'check-git-protocol' checker Leo Famulari
2021-03-11 0:14 ` zimoun
2021-03-11 1:46 ` Leo Famulari
2021-03-11 9:44 ` zimoun
2023-10-20 2:22 ` Maxim Cournoyer
2023-10-20 12:45 ` Simon Tournier
2023-10-20 15:37 ` Maxim Cournoyer
2021-03-11 22:29 ` Ludovic Courtès
2022-05-22 4:15 ` Maxim Cournoyer
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.