loopbio / opencv-feedstock

A conda-smithy repository for opencv.
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

vulnerable hdf5 vs not updated h5py #17

Open sdvillal opened 5 years ago

sdvillal commented 5 years ago

Latest hdf5 release (1.10.3) fixes some CVEs: https://www.hdfgroup.org/2018/08/hdf5-1-10-3-release-newsletter-164/

At the same time, h5py tests do not pass with hdf5 1.10.3 yet: https://github.com/h5py/h5py/issues/1088

Also hdf5 1.10.x has a terrible ABI compatibility history: https://abi-laboratory.pro/?view=timeline&l=hdf5

We have witnessed a dance in hdf5 pinning in conda-forge: first update to 1.10.3, then downgrade to 1.10.2 to accommodate for h5py, then reapply the update but pin hdf5 in h5py to 1.10.2. https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/125

The final decision has been to go ahead and apply the security patches, so most dependencies in conda-forge, and our opencv builds, are already compiling against the safer version.

But the solution for h5py is not great, they pin it to 1.10.2: https://github.com/conda-forge/h5py-feedstock/pull/36

This makes h5py from conda-forge simply not installable together with anything compiling against latest official conda-forge pinning. This includes our opencv build. I would personally not risk installing an unpinned h5py version, while defaults is more conservative and seems to be pinning hdf5 to 1.8.x. across the whole stack.

The solution proposed by the maintainers of h5py (patch the tests so that the build passes until they cut a new release) seem a bit risky to me, but it would be possible that we try that ourselves. The best solution would be of course for an h5py release to happen, but we cannot really know when will that happen. So we likely should pin down h5py explicitly in our opencv builds, which is quite undesirable on itself: we would force shipping environments with known vulnerabilities (if going for 1.10.2, we could go for 1.8 as done by defaults) and would also become out of sync with the conda-forge libraries building new builds against 1.10.3.

Also it is clear that defaults should start using conda-forge pinning (as it seems to be the idea). Otherwise incompatibilities between core libraries, like hdf5, will keep creeping into conda-forge+defaults environments.

sdvillal commented 5 years ago

@ap-- @nzjrs FYI. I still do not know what should we try first, but I'm inclined to go for the h5py "do not test this" patch first.

nzjrs commented 5 years ago

I'm sorry I don't understand what

1) we can do 2) what we should do

From your message It seems we can't do anything re #1, so question #2 is moot?

sdvillal commented 5 years ago

I'm now building a temporary version of opencv pinning down hdf5 to the less safe hdf5 version, so at least we can install it alongside conda-forge h5py. I will look into actually fixing h5py, but if it turns out it is harder or unsafer than just disabling a couple of tests, then we will live with the temporary version for a while - until h5py is fixed and released upstream.

nzjrs commented 5 years ago

OK, thanks for that. I'd like to install a CI job/pipeline that just takes our base conda-environment and installs it every few days, just so we proactively catch when this sort of thing happens.

sdvillal commented 5 years ago

We probably should use "locked" environments for safety.

That CI job you mention should test that all constraints in the environment file are met by the recreated environment - should not be very hard but at the same time it is not trivial, that was one of my planned features for "pen". That conda, instead of shouting out loud that our environment is not solvable, goes ahead and ignores most of our constraints should also be considered an ugly bug. Yesterday I played a bit with conda 4.6 beta and it seems it does scream and fail to create our environment - although strangely it complains about blas and not hdf5, which makes me nervous until I dig a bit deeper.

nzjrs commented 5 years ago

Yeah, we should, and I think such a CI job is a necessary step to getting to that point.

But for now we don't need perfect, I think in the CI test we can just grep for the output of conda list inside the created environment. That will catch most of the bugs.

And as for conda 4.6, I have zero faith in it as serious software. I don't want to touch anything less than a .5 release near production. I hope the amount of pain we eat means we are at least one of the more complex conda users.

tacaswell commented 5 years ago

Any help understanding the cause of the change in the exceptions types would greatly expedite the process of getting a new h5py release.