Revisiting ABI_VERSION handling policy

[Felix Fietkau]

On 2021-02-09 23:56, Jo-Philipp Wich wrote:
> Hi,
> 
>> The last stable update didn't change SONAME, but it included changes to
>> the same data structure that triggered the bug that I ran into. It could
>> very likely have introduced very similar subtle breakage.
> 
> this is a clear upstream bug then. Might make sense to look into how other
> distributions deal with that.
> 
>> Looking at the abi-laboratory timeline, there have been many incremental
>> wolfssl releases that did not change the SONAME, but introduced
>> potentially ABI incompatible changes. In fact, it seems to me that there
>> are more breaking than non-breaking updates.
> 
> That doesn't inspire confidence into wolfSSL at all then. Maybe we should
> reconsider using it by default. If upstream is not able to deliver a stable
> ABI for a crypto library, I'd say it isn't really fit for our purposes.
> 
>> If we were to peg the ABI_VERSION to the package version, we could still
>> deliver security updates by backporting relevant patches and review
>> potential ABI isuses of those patches.
> 
> If we at the same time ensure that the library SONAME is properly modified to
> always match ABI_VERSION and that the packaged shared object filename matches
> SONAME then it is fine for me. Merely bumping ABI_VERSION while continue
> shipping the same lib*.so.123 would not work as it'd lead to the previously
> mentioned file conflicts (e.g. libfoobar1.1 (ABI_VERSION:=1.1) and
> libfoobar1.2 (ABI_VERSION:=1.2) both shipping libfoobar.so.123 as incorrectly
> specified by upstream)
It looks to me like we already have the above problem in most of those
packages where you set the ABI_VERSION explicitly. Quite a few of our
packages intentionally have no stable ABI.

I think adjusting soname a good idea. I will look into automating it
using something like this: https://github.com/NixOS/patchelf

>>> [...]
>> The breakage that I was seeing has nothing to do with exported symbols.
>> The layout of a data structure changed. I'm not sure how to detect that
>> from readelf/objdump output.
> 
> Okay, I missed that. Still, not changing exported data structures while
> keeping the current SONAME should be common sense for any library maintainer.
> 
> The ABI_VERSION mechanism as it is right now relies on upstream being sane
> obviously. I wrongly assumed that wolfSSL does proper ABI hygiene. Apparently
> it doesn't so no wonder it breaks down in subtle ways.
> 
> As for the ability to detect it - it should be possible to reconstruct
> effective binary struct layout from DWARF debug info. So given debug builds of
> both the old and the new library versions, the output of a utility such as
> dwarfdump can be compared.
I'm pretty sure this will catch changes to non-exposed structs as well,
possibly with enough false positives to render it completely useless.

For now I think it's much safer to simply assume every version update to
be ABI-breaking unless it has explicitly been proven to be compatible.

>>> [...]
>> How do we address this in a proper way that isn't a potential foot-gun
>> on every version upgrade?
> 
> Apart from investing time on our end to paper over versioning deficiencies, we
> should talk to upstream, raise awareness for proper SONAME versioning. Also
> look how Debian deals with it (if at all).
Sounds good. While some core upstream libraries might be able to do a
good job on maintaining a stable ABI, I don't think we can rely on this
being the case for most of them, regardless of how much awareness we raise.

To sum it up, here's my current plan for resolving the immediate issues:
1. implement automatic soname patching to include ABI_VERSION
2. add config based suffix to wolfssl ABI_VERSION
3. switch back packages to automatic ABI_VERSION

When we want to deliver security updates or critical bugfixes, they
should either come in patch form (which won't affect ABI_VERSION), or
should be verified to be fully ABI compatible and override ABI_VERSION
for just that one upgrade with a comment to remove it on the next major
upgrade.

If we need to fix ABI_VERSION to a particular value for an incremental
update, we should only ever do it in the release branch and only after a
thorough review.

Do you agree?

- Felix