Re: Needed: tooling to detect references to buggy */stable packages (was: Re: [PATCHES] ImageMagick security updates without grafting)

2021-04-05 Thread Maxime Devos
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)

2021-04-04 Thread Mark H Weaver
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)

2021-03-29 Thread Ricardo Wurmus


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)

2021-03-29 Thread Maxime Devos
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)

2021-03-28 Thread Mark H Weaver
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