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
D
D
Danny Milosavljevic wrote on 4 Aug 2017 09: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 15: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 15: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 14: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 14: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 22: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 1 Dec 2017 00: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 17: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 19: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 10: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 10: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 15:27
control message for bug #27943
(address . control@debbugs.gnu.org)
87efn0fms4.fsf@gnu.org
tags 27943 fixed
close 27943
?