tar complains about too-long names (guix release)

  • Done
  • quality assurance status badge
Details
4 participants
  • Danny Milosavljevic
  • Efraim Flashner
  • Leo Famulari
  • Ludovic Courtès
Owner
unassigned
Submitted by
Danny Milosavljevic
Severity
important

Debbugs page

D
D
Danny Milosavljevic wrote on 4 Aug 2017 00:22
(address . bug-guix@gnu.org)
20170804092212.77f65fef@scratchpost.org
guix $ make release
... || chmod -R a+r "guix-0.13.0.1849-cf189-dirty"
tardir=guix-0.13.0.1849-cf189-dirty && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >guix-0.13.0.1849-cf189-dirty.tar.gz
gzip: warning: GZIP environment variable is deprecated; use an alias or script
tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch: file name is too long (max 99); not dumped
tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/libevent-2.0-evbuffer-add-use-last-with-datap.patch: file name is too long (max 99); not dumped
tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/python-genshi-stripping-of-unsafe-script-tags.patch: file name is too long (max 99); not dumped
tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/python2-pygobject-2-gi-info-type-error-domain.patch: file name is too long (max 99); not dumped
tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch: file name is too long (max 99); not dumped
tar: Exiting with failure status due to previous errors
make[1]: Leaving directory '/home/dannym/src/guix-master/guix'
L
L
Ludovic Courtès wrote on 5 Sep 2017 06:03
control message for bug #27943
(address . control@debbugs.gnu.org)
87pob5s3gb.fsf@gnu.org
severity 27943 important
L
L
Ludovic Courtès wrote on 28 Nov 2017 06:26
Re: bug#27943: tar complains about too-long names (guix release)
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
87shcyzdhg.fsf@gnu.org
Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (12 lines)
> guix $ make release
> ... || chmod -R a+r "guix-0.13.0.1849-cf189-dirty"
> tardir=guix-0.13.0.1849-cf189-dirty && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >guix-0.13.0.1849-cf189-dirty.tar.gz
> gzip: warning: GZIP environment variable is deprecated; use an alias or script
> tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch: file name is too long (max 99); not dumped
> tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/libevent-2.0-evbuffer-add-use-last-with-datap.patch: file name is too long (max 99); not dumped
> tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/python-genshi-stripping-of-unsafe-script-tags.patch: file name is too long (max 99); not dumped
> tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/python2-pygobject-2-gi-info-type-error-domain.patch: file name is too long (max 99); not dumped
> tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch: file name is too long (max 99); not dumped
> tar: Exiting with failure status due to previous errors
> make[1]: Leaving directory '/home/dannym/src/guix-master/guix'

“make dist” works fine for me with tar 1.29:

Toggle snippet (5 lines)
|| chmod -R a+r "guix-0.13.0.3626-da9b8"
tardir=guix-0.13.0.3626-da9b8 && ${TAR-tar} chof - "$tardir" | eval GZIP= gzip --best -c >guix-0.13.0.3626-da9b8.tar.gz
make[1]: Leaving directory '/home/ludo/src/guix'

Actually,
“guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch”
is 101-character long, so without the “-dirty” prefix as above, we’re
doing OK. :-)

Anyway, commit eef01cfe8eac8dee8ecf727e4ca459ae065e15ea augments the
‘patch-file-names’ linter to catch this issue.

There’s one problematic case left, which is t1lib, but I volunteered
Efraim to split the big CVE patch in several ones. :-)

Thanks,
Ludo’.
E
E
Efraim Flashner wrote on 30 Nov 2017 05:05
(name . Ludovic Courtès)(address . ludo@gnu.org)
20171130130510.GT991@macbook41
On Tue, Nov 28, 2017 at 03:26:03PM +0100, Ludovic Courtès wrote:
Toggle quote (38 lines)
> Hi Danny,
>
> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>
> > guix $ make release
> > ... || chmod -R a+r "guix-0.13.0.1849-cf189-dirty"
> > tardir=guix-0.13.0.1849-cf189-dirty && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >guix-0.13.0.1849-cf189-dirty.tar.gz
> > gzip: warning: GZIP environment variable is deprecated; use an alias or script
> > tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch: file name is too long (max 99); not dumped
> > tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/libevent-2.0-evbuffer-add-use-last-with-datap.patch: file name is too long (max 99); not dumped
> > tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/python-genshi-stripping-of-unsafe-script-tags.patch: file name is too long (max 99); not dumped
> > tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/python2-pygobject-2-gi-info-type-error-domain.patch: file name is too long (max 99); not dumped
> > tar: guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch: file name is too long (max 99); not dumped
> > tar: Exiting with failure status due to previous errors
> > make[1]: Leaving directory '/home/dannym/src/guix-master/guix'
>
> “make dist” works fine for me with tar 1.29:
>
> --8<---------------cut here---------------start------------->8---
> || chmod -R a+r "guix-0.13.0.3626-da9b8"
> tardir=guix-0.13.0.3626-da9b8 && ${TAR-tar} chof - "$tardir" | eval GZIP= gzip --best -c >guix-0.13.0.3626-da9b8.tar.gz
> make[1]: Leaving directory '/home/ludo/src/guix'
> --8<---------------cut here---------------end--------------->8---
>
> Actually,
> “guix-0.13.0.1849-cf189-dirty/gnu/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch”
> is 101-character long, so without the “-dirty” prefix as above, we’re
> doing OK. :-)
>
> Anyway, commit eef01cfe8eac8dee8ecf727e4ca459ae065e15ea augments the
> ‘patch-file-names’ linter to catch this issue.
>
> There’s one problematic case left, which is t1lib, but I volunteered
> Efraim to split the big CVE patch in several ones. :-)
>
> Thanks,
> Ludo’.

It gets worse than that, our t1lib-CVE-2010-2462 is also CVE-2011-0433
and CVE-2011-5244.¹

I tried creating a blank patch (touch t1lib-CVE...) and adding that to
satisfy the linter (and bookeeping) but unsuprisingly patch didn't like
trying to apply a blank file as a patch.

Debian removed it after squeeze², which was Debian 6, so about 6 years
ago. Gentoo apparently still has it³. We don't have anything that
depends on it so I'm in favor of removing it; even the upstream homepage
is gone.

This doesn't deal with the possibility that patches that address
multiple CVEs that can't be split easily and have a very long name will
continue to occur, so the best option I can think of right now is to
change the linter to logic like this:

CVE- -> The following are all CVEs
YYYY-ZZZZ???? -> Full CVE reference
ZZZZ???? -> Follows the year of the previous CVE

which would change t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554 ->
t1lib-CVE-2011-1552+1553+1554,
and our under-referenced t1lib-CVE-2010-2642 ->
t1lib-CVE-2010-2642+2011-0433+5244



--
Efraim Flashner <efraim@flashner.co.il> אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAlogAgMACgkQQarn3Mo9
g1HHCxAAjGof9tX07DvEkvuq8ApQF3ZEaSPToPDhIiBYwAbLMJL6jlnCyMJkhjh3
xd61ne6z4pE0XeO57FBXfyEKDCC4mY9UqxtN6db1N9w1E9IPyQMeX2MUaN8WzjeW
Mg1lfzWcLhJYMGDH7HMc1PKPG2Hl1k/hOQ+AydRH+69felyufVx4YWzc+hRqGEou
ovUmT+BJqEeurlbn5NXMFP0LT3/945oqFeKhVIq6b0wa3cJ4ADNkAesvvDWzqz68
UlfDsc4WzSltt2kdTJZLbGgriGQRUl2j2d33ySunTQ/o67vTxyyXbZK42K6ddWdj
rmxqzU9riLib5vYv7ky2qjfXnTGW0tF4Vwp7HNjNmxj4mWhFwJvCfb5v/g0N8zrO
f3lykvOwcR4FJGF0X5WDAASGm93cw+NYGQGbi/1ErfOBFzSMPT+PvL/KzEOo3VEe
/40PX+LRQs4LAASP2wEFMPy1k6VkgqExtyXUVaUEc2o494jwqWuOD/OldZy+iiSd
x28oLd4Rjictu97eNVfoRjM/uH1SqRq/g4BQ/UC9SRctKJNB3jHqLoMYNsT07Ot5
QHiD3e2fp6R/ggq/u21uyAB29yYmAMvjeL5VKleJID5/SjrLdBJWAyfANI1P3wob
ECyN5hfoWJDXWFIbJcFV8lp2wSz6OsvU5QDm8ROz2FmmaCQw/00=
=hNom
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 30 Nov 2017 05:55
(name . Efraim Flashner)(address . efraim@flashner.co.il)
877eu750rb.fsf@gnu.org
Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> skribis:

Toggle quote (7 lines)
> It gets worse than that, our t1lib-CVE-2010-2462 is also CVE-2011-0433
> and CVE-2011-5244.¹
>
> I tried creating a blank patch (touch t1lib-CVE...) and adding that to
> satisfy the linter (and bookeeping) but unsuprisingly patch didn't like
> trying to apply a blank file as a patch.

Yeah that’s no good.

Toggle quote (5 lines)
> Debian removed it after squeeze², which was Debian 6, so about 6 years
> ago. Gentoo apparently still has it³. We don't have anything that
> depends on it so I'm in favor of removing it; even the upstream homepage
> is gone.

I don’t have an opinion. Could you poll guix-devel?

Toggle quote (14 lines)
> This doesn't deal with the possibility that patches that address
> multiple CVEs that can't be split easily and have a very long name will
> continue to occur, so the best option I can think of right now is to
> change the linter to logic like this:
>
> CVE- -> The following are all CVEs
> YYYY-ZZZZ???? -> Full CVE reference
> ZZZZ???? -> Follows the year of the previous CVE
>
> which would change t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554 ->
> t1lib-CVE-2011-1552+1553+1554,
> and our under-referenced t1lib-CVE-2010-2642 ->
> t1lib-CVE-2010-2642+2011-0433+5244

I thought about it, but since it’s an unsual case, what about adding a
special property to packages instead? You’d write:

(package
;; …
(properties '((fixed-vulnerabilities "CVE-123-4567" "CVE-123-4568"))))

‘guix lint’ would honor this property, and that would address both cases
like this and situations where a CVE is known to no longer apply, as is
the case with unversioned CVEs¹.

Thoughts?

Ludo’.

E
E
Efraim Flashner wrote on 30 Nov 2017 13:49
(name . Ludovic Courtès)(address . ludo@gnu.org)
20171130214901.GA19582@macbook41
On Thu, Nov 30, 2017 at 02:55:52PM +0100, Ludovic Courtès wrote:
Toggle quote (51 lines)
> Hi Efraim,
>
> Efraim Flashner <efraim@flashner.co.il> skribis:
>
> > It gets worse than that, our t1lib-CVE-2010-2462 is also CVE-2011-0433
> > and CVE-2011-5244.¹
> >
> > I tried creating a blank patch (touch t1lib-CVE...) and adding that to
> > satisfy the linter (and bookeeping) but unsuprisingly patch didn't like
> > trying to apply a blank file as a patch.
>
> Yeah that’s no good.
>
> > Debian removed it after squeeze², which was Debian 6, so about 6 years
> > ago. Gentoo apparently still has it³. We don't have anything that
> > depends on it so I'm in favor of removing it; even the upstream homepage
> > is gone.
>
> I don’t have an opinion. Could you poll guix-devel?
>
> > This doesn't deal with the possibility that patches that address
> > multiple CVEs that can't be split easily and have a very long name will
> > continue to occur, so the best option I can think of right now is to
> > change the linter to logic like this:
> >
> > CVE- -> The following are all CVEs
> > YYYY-ZZZZ???? -> Full CVE reference
> > ZZZZ???? -> Follows the year of the previous CVE
> >
> > which would change t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554 ->
> > t1lib-CVE-2011-1552+1553+1554,
> > and our under-referenced t1lib-CVE-2010-2642 ->
> > t1lib-CVE-2010-2642+2011-0433+5244
>
> I thought about it, but since it’s an unsual case, what about adding a
> special property to packages instead? You’d write:
>
> (package
> ;; …
> (properties '((fixed-vulnerabilities "CVE-123-4567" "CVE-123-4568"))))
>
> ‘guix lint’ would honor this property, and that would address both cases
> like this and situations where a CVE is known to no longer apply, as is
> the case with unversioned CVEs¹.
>
> Thoughts?
>
> Ludo’.
>
> ¹ http://www.openwall.com/lists/oss-security/2017/03/15/3

I like that idea. It also allows us to mitigate a CVE without needing to
specifically add a patch. I've attached my first attempt at implementing
it.

--
Efraim Flashner <efraim@flashner.co.il> אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
From ad48d84c8659985d706cfe2f8e07314d6017611a Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
Date: Thu, 30 Nov 2017 23:41:29 +0200
Subject: [PATCH 1/2] lint: 'check-vulnerabilities' also checks package
properties.

* guix/scripts/lint.scm (check-vulnerabilities): Also check for CVEs
listed as mitigated in the package properties.
---
guix/scripts/lint.scm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

Toggle diff (27 lines)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 1b43b0a63..8112595c8 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -7,6 +7,7 @@
;;; Copyright © 2016 Hartmut Goebel <h.goebel@crazy-compilers.com>
;;; Copyright © 2017 Alex Kost <alezost@gmail.com>
;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2017 Efraim Flashner <efraim@flashner.co.il>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -881,10 +882,11 @@ the NIST server non-fatal."
(or (and=> (package-source package)
origin-patches)
'())))
+ (known-safe (assq-ref (package-properties package) 'fixed-vulnerabilities))
(unpatched (remove (lambda (vuln)
(find (cute string-contains
<> (vulnerability-id vuln))
- patches))
+ (append patches known-safe)))
vulnerabilities)))
(unless (null? unpatched)
(emit-warning package
--
2.15.0
From 3ae1af75fe7304a05ca8ac0edd8582d581108d05 Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
Date: Thu, 30 Nov 2017 23:46:55 +0200
Subject: [PATCH 2/2] gnu: t1lib: Change how patched CVEs are listed.

* gnu/packages/fontutils.scm (t1lib)[source]: Change patch name.
[properties]: New field, register patched CVEs.
* gnu/packages/patches/CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch:
Rename to CVE-2011-1552+.patch.
* gnu/local.mk (dist_patch_DATA): Change patch name.
---
gnu/local.mk | 2 +-
gnu/packages/fontutils.scm | 8 ++++++--
...E-2011-1553+CVE-2011-1554.patch => t1lib-CVE-2011-1552+.patch} | 0
3 files changed, 7 insertions(+), 3 deletions(-)
rename gnu/packages/patches/{t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch => t1lib-CVE-2011-1552+.patch} (100%)

Toggle diff (46 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 05a86ac17..398839682 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1079,7 +1079,7 @@ dist_patch_DATA = \
%D%/packages/patches/synfigstudio-fix-ui-with-gtk3.patch \
%D%/packages/patches/t1lib-CVE-2010-2642.patch \
%D%/packages/patches/t1lib-CVE-2011-0764.patch \
- %D%/packages/patches/t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch \
+ %D%/packages/patches/t1lib-CVE-2011-1552+.patch \
%D%/packages/patches/tar-CVE-2016-6321.patch \
%D%/packages/patches/tar-skip-unreliable-tests.patch \
%D%/packages/patches/tcl-mkindex-deterministic.patch \
diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
index d2306a942..2edbe31d1 100644
--- a/gnu/packages/fontutils.scm
+++ b/gnu/packages/fontutils.scm
@@ -302,9 +302,9 @@ high quality, anti-aliased and subpixel rendered text on a display.")
(sha256 (base32
"0nbvjpnmcznib1nlgg8xckrmsw3haa154byds2h90y2g0nsjh4w2"))
(patches (search-patches
- "t1lib-CVE-2010-2642.patch"
+ "t1lib-CVE-2010-2642.patch" ; 2011-0443, 2011-5244
"t1lib-CVE-2011-0764.patch"
- "t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch"))))
+ "t1lib-CVE-2011-1552+.patch")))) ; 2011-1553, 2011-1554
(build-system gnu-build-system)
(arguments
;; Making the documentation requires latex, but t1lib is also an input
@@ -323,6 +323,10 @@ describe character bitmaps. It contains the bitmap data as well as some
metric information. But t1lib is in itself entirely independent of the
X11-system or any other graphical user interface.")
(license license:gpl2)
+ (properties `((fixed-vulnerabilities . ("CVE-2011-0433"
+ "CVE-2011-1553"
+ "CVE-2011-1554"
+ "CVE-2011-5244"))))
(home-page "http://www.t1lib.org/")))
(define-public teckit
diff --git a/gnu/packages/patches/t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch b/gnu/packages/patches/t1lib-CVE-2011-1552+.patch
similarity index 100%
rename from gnu/packages/patches/t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch
rename to gnu/packages/patches/t1lib-CVE-2011-1552+.patch
--
2.15.0
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAlogfMoACgkQQarn3Mo9
g1EixQ//T4irVbn4pz4m4o1Mqj2CV261AwsQRtntK0HcnkWaJ4weK+ZsQsDQu8aU
Mi/QR2r2aMpOuDaBs97j1BL9Pv7HcSDJSpZgxRPdue9GL/1q8NuyQAizayhNXR9r
rJb+ayiROe6aAtF2t2SeQdX2sWufn6liCDu+4854+dbmGgru5l0ipbgNyFXTQ53d
TIHXZF074HSaZMMa/14AWcqxqHxsh37ch5ObSCi+P0IVlIF/bKrdBP3e8fmJdLNW
Z7EEbgEKzuV09tNmx7LSNIBdqMNdpdmLdgtUFl/ATdjdy+QYfEu4I43rguUse1DY
2gcTfkCI+ToTjn+j9DLQDuYeTkrjWMIH845ZfOIm6CGjgqkqG+06DiBn222C6Y04
/+vCJ2USHhn89y6eIFg4I8CpSR0Qp7+0r6Jv2Vjq4A//aeDKNZ44ww3/66HNKGuv
cKajdCW2QQESiZMeAU9wTFfku7UR0dwIimm49HQui1rlRGKUoNwcAUs0o7uU8wcG
ygRe7CIjv+XEqn9wMtrbJJ6gTWEB7NEDhspirIbczm5K7Uyc/FExSN+WZbTr4ZCk
YpnS5ntuOIiGTeOTOZTPAmf9iL/1edJe1emfgTkzoKV7UrOMXg6yXigYe1CtG7Ux
VrrmIzMPS2/xVq/YSKeaSrpp1uctNMoy9hbmP185DRD8npURrAw=
=izJw
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 30 Nov 2017 15:12
(name . Efraim Flashner)(address . efraim@flashner.co.il)
20171130231220.GA908@jasmine.lan
Toggle quote (14 lines)
> On Thu, Nov 30, 2017 at 02:55:52PM +0100, Ludovic Courtès wrote:
> > I thought about it, but since it’s an unsual case, what about adding a
> > special property to packages instead? You’d write:
> >
> > (package
> > ;; …
> > (properties '((fixed-vulnerabilities "CVE-123-4567" "CVE-123-4568"))))
> >
> > ‘guix lint’ would honor this property, and that would address both cases
> > like this and situations where a CVE is known to no longer apply, as is
> > the case with unversioned CVEs¹.
> >
> > Thoughts?

I'd rather the property's name more clearly reflect that it doesn't
actually fix the vulnerability, but just prevents the linter from
complaining about it.

Someone who sees this property used in a package could reasonably assume
that it's required to list all fixed CVEs in a 'fixed-vulnerabilities'
list, and that it is the "single source of truth" for which bugs apply
to a package. But, it would not actually have anything to do with that,
just being a way to silence the linter.

However, I can't think of a good idea for another name...

On Thu, Nov 30, 2017 at 11:49:01PM +0200, Efraim Flashner wrote:
Toggle quote (4 lines)
> I like that idea. It also allows us to mitigate a CVE without needing to
> specifically add a patch. I've attached my first attempt at implementing
> it.

I think of `guix lint -c cve` as one of many tools for discovering
important problems in our packages, but I don't think that we must
absolutely silence the linter. It's always going to be imprecise, with
both false negative and positive results.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAlogkFEACgkQJkb6MLrK
fwgoSg/9GN8oCFfGMD0DVD61waePPphdeLs8gJWY9x17ctOKnMYPTjPOzdd9MHpL
ZdEOJYzrfaIw8eqk8ew3Hv8xaa/EDrxYU4annXB1vrzS3DI3rCTNgbMSISb8XFWk
hDxrLPoK+MN4jUWoTYmbGSgM7Sxn7optqa1ohMbl7xAnRuNwOHNgQoOT8ibuVP8H
HaFLCXHg7hp7QqoKib9QGH+D3LfGZ0kRuAQj2KBugOf9CcXP1UjU7lP04igLaAWp
c0pYHiRF1329b+P7Q1jQTrWK7rvT1nhRlmhX/rGMS7X0ag6g2Ue/6YefMgyI+uFV
zsE4olKFRbAvNkoYSjKr9TMBxkLPlSgkYdAdDSjXxbKWvieSShXWN1X4+CWgDAtH
1Q5yxjFkRVww0e0jlah3fLM5O5F2In5n6Anbf5UHec3MpehisTu3jJOmZMuOxaMs
xJ2XcwcL8/FL3omrPGLFCbq0ZQG1HYz2lKy7klUGwOLMeHNyeR6Mk1LK3NKPH2Ob
FsCfQZM9i+2g+Y2H/daZGiCYuSrhQliicZQSBLOAMfFz7Y+C6z17gnbLA0vFTcsM
rruuun+0xd4UApPT7mPLYGN/1kasg2Wbgj5i8vIGUdmnRyIEV6JPAk60ng/sVDUR
5r/cRzfNew/SvVBYUQQZs7f4+b0++Eo/XVW6J+NROeGQ5Yue3do=
=Z612
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 1 Dec 2017 08:50
(name . Leo Famulari)(address . leo@famulari.name)
87k1y6e6km.fsf@gnu.org
Leo Famulari <leo@famulari.name> skribis:

Toggle quote (24 lines)
>> On Thu, Nov 30, 2017 at 02:55:52PM +0100, Ludovic Courtès wrote:
>> > I thought about it, but since it’s an unsual case, what about adding a
>> > special property to packages instead? You’d write:
>> >
>> > (package
>> > ;; …
>> > (properties '((fixed-vulnerabilities "CVE-123-4567" "CVE-123-4568"))))
>> >
>> > ‘guix lint’ would honor this property, and that would address both cases
>> > like this and situations where a CVE is known to no longer apply, as is
>> > the case with unversioned CVEs¹.
>> >
>> > Thoughts?
>
> I'd rather the property's name more clearly reflect that it doesn't
> actually fix the vulnerability, but just prevents the linter from
> complaining about it.
>
> Someone who sees this property used in a package could reasonably assume
> that it's required to list all fixed CVEs in a 'fixed-vulnerabilities'
> list, and that it is the "single source of truth" for which bugs apply
> to a package. But, it would not actually have anything to do with that,
> just being a way to silence the linter.

Yes, I see it as a last resort, and thus rarely used. When used, it
should be accompanied by a comment clearly explaining what we’re doing.

I think people are unlikely to see it as a “single source of truth”
because it’ll be used in a handful of packages only, and because
comments there should make it clear that it’s really just to placate the
linter.

Toggle quote (2 lines)
> However, I can't think of a good idea for another name...

Maybe ‘lint-hidden-vulnerabilities’ or ‘hidden-vulnerabilities’, or
‘ignored-vulnerabilities’, or…? What’s you preference? :-)

Toggle quote (10 lines)
> On Thu, Nov 30, 2017 at 11:49:01PM +0200, Efraim Flashner wrote:
>> I like that idea. It also allows us to mitigate a CVE without needing to
>> specifically add a patch. I've attached my first attempt at implementing
>> it.
>
> I think of `guix lint -c cve` as one of many tools for discovering
> important problems in our packages, but I don't think that we must
> absolutely silence the linter. It's always going to be imprecise, with
> both false negative and positive results.

I agree. Like patch file names, I view this new property as a way to
silence the reader when we have reliable info to do that.

Would you be OK with a more appropriate name and the understanding that
it’s there to address rare cases like this one?

Thanks for your feedback!

Ludo’.
L
L
Leo Famulari wrote on 1 Dec 2017 10:20
(name . Ludovic Courtès)(address . ludo@gnu.org)
20171201182059.GA2324@jasmine.lan
On Fri, Dec 01, 2017 at 05:50:01PM +0100, Ludovic Courtès wrote:
Toggle quote (3 lines)
> Maybe ‘lint-hidden-vulnerabilities’ or ‘hidden-vulnerabilities’, or
> ‘ignored-vulnerabilities’, or…? What’s you preference? :-)

I like 'lint-hidden-vulnerabilities' because it communicates that we are
"hiding" a vulnerability somehow and that it's related to the linter.

Maybe even 'lint-hidden-cve', since it's really about the CVE system and
not so much about vulnerabilities as vulnerabilities.

Toggle quote (3 lines)
> Would you be OK with a more appropriate name and the understanding that
> it’s there to address rare cases like this one?

Yes, definitely!
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAlohnYsACgkQJkb6MLrK
fwivJhAAmqe/GNiLu+MUezbP2wUv96wijcpR1lT5wP0RZatNYNWOoIlttZBm9ea8
j/bsJYaNve2owUBgRQrkGqqH9jevDjIUlKSCYSYR7oim78iFIaxVzyb4vt/3E6Jm
L11+fr/kTyGtecW4/c0Xq8JSmfGyeGc5cSpgwCu15Qy8KySjRUZMJo5PS0MHqZ/w
IK9Gex2gmtPOfhXj2fknTA3HMsuZ2GQiOB4O18pJ/kJl8WHgamEmy8q42h8NX0n0
4rD4yex5ODm9UCXMKRzo+4cAGdbE4dApDyxhm7FGR/Zz4s3pB9SWn2udOsd/mBJq
gK2tnIpou9gXFqqoXP64HSgIsNI5vc8Z3VyZKGz+pbgj/b9+QmIJpQq6Acpd+d7i
uZNwFVf2v28PeeLtHeWvpT9BVLYjS7cTZ8kTPBMr0pT5/3ZfAOZuB3aAuXJi6NQl
3Hfb1IY2yMnN53KYcNanW/GxkYBPv3Az+W69d9UyfKtK4cNuu/A3g7YtSt8BXlYz
oLuXL0L/zd4PoSNclxGrlRWO0P61ajO0IFPfvCccdIFHPB91JKKYtvamSsBAKjX+
PrSfD3SbckEEKr3W8f09td09EAbbuFt9l6XGWcsYj448dC6CXVY5iXhLwFhOGpQt
eH7+kC1TfWMGLmeGddviMe3Mo+F2mhfHTMq3V0Ea0w9b/4D/ZHw=
=Axiu
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 2 Dec 2017 01:55
(name . Efraim Flashner)(address . efraim@flashner.co.il)
87po7x3152.fsf@gnu.org
Efraim Flashner <efraim@flashner.co.il> skribis:

Toggle quote (30 lines)
> From ad48d84c8659985d706cfe2f8e07314d6017611a Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Thu, 30 Nov 2017 23:41:29 +0200
> Subject: [PATCH 1/2] lint: 'check-vulnerabilities' also checks package
> properties.
>
> * guix/scripts/lint.scm (check-vulnerabilities): Also check for CVEs
> listed as mitigated in the package properties.
> ---
> guix/scripts/lint.scm | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index 1b43b0a63..8112595c8 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -7,6 +7,7 @@
> ;;; Copyright © 2016 Hartmut Goebel <h.goebel@crazy-compilers.com>
> ;;; Copyright © 2017 Alex Kost <alezost@gmail.com>
> ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
> +;;; Copyright © 2017 Efraim Flashner <efraim@flashner.co.il>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -881,10 +882,11 @@ the NIST server non-fatal."
> (or (and=> (package-source package)
> origin-patches)
> '())))
> + (known-safe (assq-ref (package-properties package) 'fixed-vulnerabilities))

Can you change that to ‘lint-hidden-cve’ as Leo suggested?

Toggle quote (7 lines)
> (unpatched (remove (lambda (vuln)
> (find (cute string-contains
> <> (vulnerability-id vuln))
> - patches))
> + (append patches known-safe)))
> vulnerabilities)))

To be accurate, we’d rather do:

(remove (lambda (vuln)
(let ((id (vulnerability-id vuln)))
(or (find … patches)
(member id known-safe))))
…)

Also could you add a simple test in tests/lint.scm? You can start from
one of the existing CVE tests in there and just add a ‘properties’ field
to the test package.

Thank you!

Ludo’.
L
L
Ludovic Courtès wrote on 2 Dec 2017 01:57
(name . Efraim Flashner)(address . efraim@flashner.co.il)
87lgil311c.fsf@gnu.org
Efraim Flashner <efraim@flashner.co.il> skribis:

Toggle quote (11 lines)
> From 3ae1af75fe7304a05ca8ac0edd8582d581108d05 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Thu, 30 Nov 2017 23:46:55 +0200
> Subject: [PATCH 2/2] gnu: t1lib: Change how patched CVEs are listed.
>
> * gnu/packages/fontutils.scm (t1lib)[source]: Change patch name.
> [properties]: New field, register patched CVEs.
> * gnu/packages/patches/CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch:
> Rename to CVE-2011-1552+.patch.
> * gnu/local.mk (dist_patch_DATA): Change patch name.

[...]

Toggle quote (18 lines)
> (patches (search-patches
> - "t1lib-CVE-2010-2642.patch"
> + "t1lib-CVE-2010-2642.patch" ; 2011-0443, 2011-5244
> "t1lib-CVE-2011-0764.patch"
> - "t1lib-CVE-2011-1552+CVE-2011-1553+CVE-2011-1554.patch"))))
> + "t1lib-CVE-2011-1552+.patch")))) ; 2011-1553, 2011-1554
> (build-system gnu-build-system)
> (arguments
> ;; Making the documentation requires latex, but t1lib is also an input
> @@ -323,6 +323,10 @@ describe character bitmaps. It contains the bitmap data as well as some
> metric information. But t1lib is in itself entirely independent of the
> X11-system or any other graphical user interface.")
> (license license:gpl2)
> + (properties `((fixed-vulnerabilities . ("CVE-2011-0433"
> + "CVE-2011-1553"
> + "CVE-2011-1554"
> + "CVE-2011-5244"))))

Perhaps move ‘properties’ right below ‘patches’ for clarity. And
s/fixed-vulnerabilities/lint-hidden-cve/. :-)

OK with these changes, thank you!

Ludo’.
L
L
Ludovic Courtès wrote on 8 Jan 2018 06:27
control message for bug #27943
(address . control@debbugs.gnu.org)
87efn0fms4.fsf@gnu.org
tags 27943 fixed
close 27943
?
Your comment

This issue is archived.

To comment on this conversation send an email to 27943@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 27943
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