Check binary consistency after grafting with e.g. ldd

  • Done
  • quality assurance status badge
Details
2 participants
  • Léo Le Bouter
  • Ludovic Courtès
Owner
unassigned
Submitted by
Léo Le Bouter
Severity
normal

Debbugs page

L
L
Léo Le Bouter wrote on 18 Mar 2021 03:37
(address . bug-guix@gnu.org)
22d6fde28a4646254061f56c342fb75d2a2846d9.camel@zaclys.net
Hello!

We had an issue after grafting ImageMagick fixed by <
Toggle quote (2 lines)
>.

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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEFIvLi9gL+xax3g6RRaix6GvNEKYFAmBTLU0ACgkQRaix6GvN
EKZJrg//Wdostj078M1GrVVw5Zi0WBgqQp2GEk10TE0a3gwmbcqOEjwzR1kk7lRc
ic4UqVVkhT/gu+89B3d+MXc1c9I/etHqZfAZmrBzNso26zi5qsmL2V/HA82KdQRE
fAO7ijsgavQoZ+8m/+k8oE7frlEkUqeTx57ZL++XQByOMGVK+Q+VRmPTt0Uej3MA
Um1GqmOaA44W/Bbec6rf0lNKQ08v1kCGDbDRRqDrzkT4apiA/wYgYgCkS9JWKL0I
tQaAbgSJj3AygHQwgZX/SGjFk3ZjdwnX0YHOfwg6VTLLT1KYdxJPy4EkYE+cVQ5M
v1mdMN9t0hJIgb3nWLs3Jha46WpTBPf2U7v2/kbH/HbD0FaZdrFV82k7gm8eh1l0
neOyiEi19dtZr2OqCuobl6DhsQZndA7RD4+UtenMbVu5wQfX3bZiO+RuHIijPTs3
jBd39RIfPfzJQ+80A+x7QxBX1bIy8yKCvWIOBtC/YQ5uo25XX/QhA9x2tHQvTbPF
frtLwRGXtmS6hR824fL1oBQnWwm8JCvodU80PqSnCEhBqR6dX1OeNZPx7ucGgJ7m
LKFg7GgkziEYgv1RhV/uqF2bonXdgSQEV6ZDsqCOQledxB/DxQB+z6uw8DKiLldy
gPhZ+ZgiDoRTmLXCmMqkTrI8IigpN0ARkkUcTdhp21bnfUWQ1oM=
=Getz
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 18 Mar 2021 06:38
(name . Léo Le Bouter)(address . lle-bout@zaclys.net)
878s6kpoon.fsf@gnu.org
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:

Toggle quote (20 lines)
> 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
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
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’.
L
L
Léo Le Bouter wrote on 19 Mar 2021 01:58
(name . Ludovic Courtès)(address . ludo@gnu.org)
ea674f0bd142d0f308f778b3e18205a01d895111.camel@zaclys.net
On Thu, 2021-03-18 at 14:38 +0100, Ludovic Courtès wrote:
Toggle quote (4 lines)
> I don’t think all the testing that needs to be done when grafting can
> be
> automated.

Not all but part of it?

Toggle quote (6 lines)
> 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?

Toggle quote (7 lines)
> 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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEFIvLi9gL+xax3g6RRaix6GvNEKYFAmBUZ88ACgkQRaix6GvN
EKZg/g//VpW5ff/cH8Y6Wz2x5CXlJYbLlcK4eOh6zcNMULwfbaSFx8ay+cUhv3X6
M1zj19I1elqdx8nupsFDV1l3J71PR+TddKmQD/7Q3Ke40BLva3eurr8fOBqJXepn
UbA5Da4aBbZxLppU0K0HtO6oxQjEsMnBaLfHMliFXnKeufEi8f81gqHJwjLKsv8e
SLboBw9vRrV1pakFDHNYBWznNdUHDNSteE8ql/5xPpwoV5bsECvW86yNBfczL/qV
JpCluT+zaORLomlz2X46xDWleh4dvEiRdl4l2JmySzsRbqcfX55nof1dmvKBxBQ1
wMnr82dXSzg1pCy413Ist5iNVU6HxeLNrVr/T/Y9QRpYBm+X/v/6ziGZMmDN3+6y
b55y4hq91C9z2T0/lWVdd4pmofYGEhwUs419mP2kE36AFgp19JMPMOV09WIbx6VQ
uPUouygDc8p7Q/TCPbsh8ZwJ1m4bOVAFnxm7Jij1bxx3azcOlpmHAabkhunQYcLw
7tCzbdnOKStUapflqHW+qXL++qf0xKUxnBZhai789qSNlYtxEYYtsdrnmGNRolSQ
AYgeeUdYjueZbdqZ47zb/HWDNK4Mvw5pGIDyGsAFtKmjliPKvWHGivZqa0H9Vy0p
0iSrqgJD7WRsPgTPCm4qFpP/BdoGEfascIqAKDO9f2CZe/fplX0=
=uyW9
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 19 Mar 2021 03:39
(name . Léo Le Bouter)(address . lle-bout@zaclys.net)
87sg4rfmvj.fsf@gnu.org
Hi,

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

Toggle quote (7 lines)
> 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.

Toggle quote (9 lines)
>> 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.

Toggle quote (15 lines)
>> 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?

Toggle quote (3 lines)
> 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’.
L
L
Ludovic Courtès wrote on 29 Mar 2021 23:50
control message for bug #47228
(address . control@debbugs.gnu.org)
87czvhkudy.fsf@gnu.org
tags 47228 notabug
close 47228
quit
?
Your comment

This issue is archived.

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

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