bug#47228: Check binary consistency after grafting with e.g. ldd

bug#47228: Check binary consistency after grafting with e.g. ldd

[Léo Le Bouter via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

Hello!

We had an issue after grafting ImageMagick fixed by <
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd
>.

Basically Inkscape did not work because ImageMagick's soname had been
bumped (probably for forward compat?):

/gnu/store/g75q5v1gqi4x08qcf1ydfl9xhp4slmxy-inkscape-
1.0.2/bin/.inkscape-real: error while loading shared libraries:
libMagickCore-6.Q16.so.6: cannot open shared object file: No such file
or directory

It seems technically possible to automatically check for this kind of
breakage, therefore I suggest we run ldd (might actually run code from
the binary) or objdump -x (pure static analysis), so after grafting we
could check that every binary can load all it's dependents declared in
the ELF headers successfully and report errors if not?

What do you think?

Léo

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#47228: Check binary consistency after grafting with e.g. ldd

[Ludovic Courtès]

Hi,

(Cc: Leo Famulari who has been taking care of many security issues in
Guix over years.)

Léo Le Bouter <lle-bout@zaclys.net> skribis:

> We had an issue after grafting ImageMagick fixed by <
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd
>>.
>
> Basically Inkscape did not work because ImageMagick's soname had been
> bumped (probably for forward compat?):
>
> /gnu/store/g75q5v1gqi4x08qcf1ydfl9xhp4slmxy-inkscape-
> 1.0.2/bin/.inkscape-real: error while loading shared libraries:
> libMagickCore-6.Q16.so.6: cannot open shared object file: No such file
> or directory
>
> It seems technically possible to automatically check for this kind of
> breakage, therefore I suggest we run ldd (might actually run code from
> the binary) or objdump -x (pure static analysis), so after grafting we
> could check that every binary can load all it's dependents declared in
> the ELF headers successfully and report errors if not?
>
> What do you think?

I don’t think all the testing that needs to be done when grafting can be
automated.

In particular, packagers who want to introduce a replacement for a
library should use libabigail’s ‘abi-diff’ tool to check that the
package and its replacement are ABI-compatible.  It’s also a good idea
to make some quick manual tests.

The .so file symlinks in
<https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd>
look very scary to me.  To me, it’s likely to hide the ABI
incompatibility issue rather than “fix” it.

Léo, please make sure to submit patches for review, as noted in
<https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html>.
Such changes do not qualify as “trivial” and we should strive to get
more than one pair of eyeballs on it.

Leo F. has always done that, even with years of experience, and I think
it’s been fruitful, even when that meant delaying the patch by a couple
of days.

The good thing with being a “rolling release” distro is that we can
quickly roll out fixes; the bad thing is that we can just as quickly
roll out bugs.  :-)

Thanks,
Ludo’.

bug#47228: Check binary consistency after grafting with e.g. ldd

[Léo Le Bouter via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

On Thu, 2021-03-18 at 14:38 +0100, Ludovic Courtès wrote:
> I don’t think all the testing that needs to be done when grafting can
> be
> automated.

Not all but part of it?

> In particular, packagers who want to introduce a replacement for a
> library should use libabigail’s ‘abi-diff’ tool to check that the
> package and its replacement are ABI-compatible.  It’s also a good
> idea
> to make some quick manual tests.

That's great! Maybe we can have some quick tooling to in GNU Guix to
aid that?

> The .so file symlinks in
> <
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd
> >
> look very scary to me.  To me, it’s likely to hide the ABI
> incompatibility issue rather than “fix” it.

:-/ Yes it is scary, we were having an user with an Inkscape issue on
IRC and this commit fixed it for them and they could work without an
issue though, we were discussing with rekado and rekado suggested we
cheat like this and I've done it, the only alternative we have is
porting/applying all patches to our version by digging commit history
(with always the doubt of adding an incomplete fix which is likely if
we have to dig commit history manually).

If nobody can put time to dig patches for all individuals CVEs until we
ungraft then I'd rather have this scary commit in.

Léo

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#47228: Check binary consistency after grafting with e.g. ldd

[Ludovic Courtès]

Hi,

Léo Le Bouter <lle-bout@zaclys.net> skribis:

> On Thu, 2021-03-18 at 14:38 +0100, Ludovic Courtès wrote:
>> I don’t think all the testing that needs to be done when grafting can
>> be
>> automated.
>
> Not all but part of it?

Not even sure; at least I don’t have any ideas.

>> In particular, packagers who want to introduce a replacement for a
>> library should use libabigail’s ‘abi-diff’ tool to check that the
>> package and its replacement are ABI-compatible.  It’s also a good
>> idea
>> to make some quick manual tests.
>
> That's great! Maybe we can have some quick tooling to in GNU Guix to
> aid that?

Again it’s on a case-by-case basis, it depends on what you’re grafting,
so I wouldn’t know how to do that.

Perhaps a first step would be consolidate this “insider knowledge” about
security updates and grafts into a check list.

>> The .so file symlinks in
>> <
>> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd
>> >
>> look very scary to me.  To me, it’s likely to hide the ABI
>> incompatibility issue rather than “fix” it.
>
> :-/ Yes it is scary, we were having an user with an Inkscape issue on
> IRC and this commit fixed it for them and they could work without an
> issue though, we were discussing with rekado and rekado suggested we
> cheat like this and I've done it, the only alternative we have is
> porting/applying all patches to our version by digging commit history
> (with always the doubt of adding an incomplete fix which is likely if
> we have to dig commit history manually).

It’s the kind of patch that should be reviewed before it gets in.

In this case, review will have to happen after the fact, but it still
has to happen IMO.  I’d prefer not to do it myself; perhaps Leo F. can
take a look?

> If nobody can put time to dig patches for all individuals CVEs until we
> ungraft then I'd rather have this scary commit in.

Security is a spectrum; we’ll never close all CVEs.  :-)

Security issues often call for quick reaction, but to me that doesn’t
mean we should dismiss our practices and workflow, in particular peer
review.

Thanks,
Ludo’.