Re: Needed: tooling to detect references to buggy */stable packages (was: Re: [PATCHES] ImageMagick security updates without grafting)
On Sun, 2021-04-04 at 16:14 -0400, Mark H Weaver wrote: > Maxime Devo wrote: > > * In some places we have the following pattern: > > > > [...] > I don't understand this. Why would it need to be made unconditional? I don't understand either anymore. > [...] > > At the present time, I'm more inclined to add machinery to automatically > add _implicit_ #:disallowed-references, to enforce this checking at > package build time. This would require rebuilding everything that > depends on a '*/stable' package, which means that this kind of tooling > could not be applied directly to 'master', but would need to go through > 'staging'. That seems good to me. I believe the current plan is: * Add a 'stable' property to the gtk-doc/stable, dblatex/stable ... packages. * Change gnu-build-system, glib-or-gtk-build-system ... to implicitely add packages in inputs, propagated-inputs or native-inputs that have the 'stable' property to #:disallowed-references, unless the package that is being built is a 'stable' package itself. And an idea for the future is: * Implicitely add all packages in native-inputs to #:disallowed-references, unless they are in inputs or propagated-inputs as well. * Verify everything still works well (when cross-compiling and when compiling natively), and fix breakage. Greetings, Maxime. signature.asc Description: This is a digitally signed message part
Re: Needed: tooling to detect references to buggy */stable packages (was: Re: [PATCHES] ImageMagick security updates without grafting)
Hi Maxime, Maxime Devos writes: > On Sun, 2021-03-28 at 18:33 -0400, Mark H Weaver wrote: >> Earlier, I wrote: >> > One thing to be very careful about is to only use 'gtk-doc/stable', >> > 'dblatex/stable', and 'imagemagick/stable' in native-inputs, and >> > moreover to make sure that no references to these */stable packages >> > remain in any package outputs. >> > >> > Of course, if any package retains references to its 'native-inputs', >> > that's always a bug, but I wouldn't be surprised if such bugs exist in >> > Guix. Such bugs might be relatively harmless now (except when >> > cross-compiling), but they could become a security bug if a package >> > retains a reference to 'imagemagick/stable'. > > It just occurred to me: could we automatically add all native-inputs > to #:disallowed-references when cross-compiling? This shouldn't break > any packages, except possibly when cross-compiling. > > Or stronger, add all native-inputs to #:disallowed-references (unless > they are also in inputs or propagated-inputs), even when compiling > natively? Yes, I think this is a good idea and worth considering. However, we'd need to consider the case where the same package is in both 'inputs' and 'native-inputs'. When cross-compiling, it's no problem, because the same package in those two fields will lead to distinct output paths, since they are compiled for different systems. However, when compiling natively, the outputs of packages occurring in both 'inputs' and 'native-inputs' should *not* be implicitly included in #:disallowed-references. > Problems include: > * I guess a world rebuild, as the derivations would be different. Indeed! > * In some places we have the following pattern: > > (native-inputs > `(("autoconf" ,autoconf) >("automake" ,automake) >("pkg-config" ,pkg-config) >,@(if (%current-target-system) > `(("guile" ,guile-3.0)) ;for 'guild compile' and 'guile-3.0.pc' > '( > (inputs > `(("guile" ,guile-3.0) >("lzlib" ,lzlib))) > (synopsis "Guile bindings to lzlib") > > The (if (%current-target-system) ...) would need to be made unconditional. I don't understand this. Why would it need to be made unconditional? > * I guess an option to disable this behaviour might be useful. > >> It occurs to me that we will need some tooling to ensure that no >> references to these buggy "*/stable" packages end up in package outputs >> that users actually use. Otherwise, it is likely that sooner or later, >> a runtime reference to one of these buggy packages will sneak in to our >> systems. >> >> An initial idea is that these "*/stable" packages could have a package >> property (perhaps named something like 'build-time-only') that indicates >> that references to its outputs should not occur within the outputs of >> any other package that does not have that same property. > > Would this be (a) something enforced by the build process (using > #:disallowed-references or #:allowed-references), or (b) a linter? I would prefer option (a) above. >> We'd also need to somehow ensure that users don't install these >> 'build-time-only' packages directly, at least not without an additional >> option (e.g. --force-unsafe-build-time-only) to override it. > > What about a developer running "guix environment eom"? IIUC, this would > make the developer vulnerable (at least, once I've gotten around replacing > the 'gtk-doc' input with 'gtk-doc/stable'), so it might make sense to > replace /stable -> unstable packages here. > > However, now the developer ends up with a different set of packages than > wil be seen in the build environment ... That's an excellent point, for which I don't have any good answer. I'm open to suggestions. > Is this something you will be writing "guix lint" checkers (or other > checkers) for yourself? At the present time, I'm more inclined to add machinery to automatically add _implicit_ #:disallowed-references, to enforce this checking at package build time. This would require rebuilding everything that depends on a '*/stable' package, which means that this kind of tooling could not be applied directly to 'master', but would need to go through 'staging'. What do you think? Thanks, Mark
Re: Needed: tooling to detect references to buggy */stable packages (was: Re: [PATCHES] ImageMagick security updates without grafting)
Mark H Weaver writes: > Earlier, I wrote: >> One thing to be very careful about is to only use 'gtk-doc/stable', >> 'dblatex/stable', and 'imagemagick/stable' in native-inputs, and >> moreover to make sure that no references to these */stable packages >> remain in any package outputs. >> >> Of course, if any package retains references to its 'native-inputs', >> that's always a bug, but I wouldn't be surprised if such bugs exist in >> Guix. Such bugs might be relatively harmless now (except when >> cross-compiling), but they could become a security bug if a package >> retains a reference to 'imagemagick/stable'. > > It occurs to me that we will need some tooling to ensure that no > references to these buggy "*/stable" packages end up in package outputs > that users actually use. Otherwise, it is likely that sooner or later, > a runtime reference to one of these buggy packages will sneak in to our > systems. The gnu-build-system takes a keyword #:disallowed-references that could be used here. -- Ricardo
Re: Needed: tooling to detect references to buggy */stable packages (was: Re: [PATCHES] ImageMagick security updates without grafting)
On Sun, 2021-03-28 at 18:33 -0400, Mark H Weaver wrote: > Earlier, I wrote: > > One thing to be very careful about is to only use 'gtk-doc/stable', > > 'dblatex/stable', and 'imagemagick/stable' in native-inputs, and > > moreover to make sure that no references to these */stable packages > > remain in any package outputs. > > > > Of course, if any package retains references to its 'native-inputs', > > that's always a bug, but I wouldn't be surprised if such bugs exist in > > Guix. Such bugs might be relatively harmless now (except when > > cross-compiling), but they could become a security bug if a package > > retains a reference to 'imagemagick/stable'. It just occurred to me: could we automatically add all native-inputs to #:disallowed-references when cross-compiling? This shouldn't break any packages, except possibly when cross-compiling. Or stronger, add all native-inputs to #:disallowed-references (unless they are also in inputs or propagated-inputs), even when compiling natively? Problems include: * I guess a world rebuild, as the derivations would be different. * In some places we have the following pattern: (native-inputs `(("autoconf" ,autoconf) ("automake" ,automake) ("pkg-config" ,pkg-config) ,@(if (%current-target-system) `(("guile" ,guile-3.0)) ;for 'guild compile' and 'guile-3.0.pc' '( (inputs `(("guile" ,guile-3.0) ("lzlib" ,lzlib))) (synopsis "Guile bindings to lzlib") The (if (%current-target-system) ...) would need to be made unconditional. * I guess an option to disable this behaviour might be useful. > It occurs to me that we will need some tooling to ensure that no > references to these buggy "*/stable" packages end up in package outputs > that users actually use. Otherwise, it is likely that sooner or later, > a runtime reference to one of these buggy packages will sneak in to our > systems. > > An initial idea is that these "*/stable" packages could have a package > property (perhaps named something like 'build-time-only') that indicates > that references to its outputs should not occur within the outputs of > any other package that does not have that same property. Would this be (a) something enforced by the build process (using #:disallowed-references or #:allowed-references), or (b) a linter? > We'd also need to somehow ensure that users don't install these > 'build-time-only' packages directly, at least not without an additional > option (e.g. --force-unsafe-build-time-only) to override it. What about a developer running "guix environment eom"? IIUC, this would make the developer vulnerable (at least, once I've gotten around replacing the 'gtk-doc' input with 'gtk-doc/stable'), so it might make sense to replace /stable -> unstable packages here. However, now the developer ends up with a different set of packages than wil be seen in the build environment ... > Additionally, it might be good to issue warnings if 'build-time-only' > packages are not hidden, This seems good to me. This should prevent "guix install imagemagick@bad-version". > or if they are found within the 'inputs' or > 'propagated-inputs' fields of any package that's not also > 'build-time-only'. Both of these last two checks have loopholes, > however, so they are not reliable indicators. But these (automatic "guix lint") checks could still catch many problems in practice before they are committed! > Thoughts? Other proposals? Is this something you will be writing "guix lint" checkers (or other checkers) for yourself? Greetings, Maxime. signature.asc Description: This is a digitally signed message part
Needed: tooling to detect references to buggy */stable packages (was: Re: [PATCHES] ImageMagick security updates without grafting)
Earlier, I wrote: > One thing to be very careful about is to only use 'gtk-doc/stable', > 'dblatex/stable', and 'imagemagick/stable' in native-inputs, and > moreover to make sure that no references to these */stable packages > remain in any package outputs. > > Of course, if any package retains references to its 'native-inputs', > that's always a bug, but I wouldn't be surprised if such bugs exist in > Guix. Such bugs might be relatively harmless now (except when > cross-compiling), but they could become a security bug if a package > retains a reference to 'imagemagick/stable'. It occurs to me that we will need some tooling to ensure that no references to these buggy "*/stable" packages end up in package outputs that users actually use. Otherwise, it is likely that sooner or later, a runtime reference to one of these buggy packages will sneak in to our systems. An initial idea is that these "*/stable" packages could have a package property (perhaps named something like 'build-time-only') that indicates that references to its outputs should not occur within the outputs of any other package that does not have that same property. We'd also need to somehow ensure that users don't install these 'build-time-only' packages directly, at least not without an additional option (e.g. --force-unsafe-build-time-only) to override it. Additionally, it might be good to issue warnings if 'build-time-only' packages are not hidden, or if they are found within the 'inputs' or 'propagated-inputs' fields of any package that's not also 'build-time-only'. Both of these last two checks have loopholes, however, so they are not reliable indicators. Thoughts? Other proposals? Regards, Mark