Skip to content

Fixed issue where rhel >8 packages would not have correct openssl dependency version #1570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

craigcomstock
Copy link
Contributor

@craigcomstock craigcomstock commented Jan 16, 2025

We build against systems with the latest available dependencies such as OpenSSL.
libcurl builds against this OpenSSL version which can have API changes that will break functionality if used on a non-updated system.

Ticket: ENT-12587
Changelog: title

Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me otherwise. One thing that worries me, though, is that I knew the fixed 3.0.0/3.0.1 would stop working at some point, but I expected our tests to catch this. How come that is not the case?

@craigcomstock craigcomstock force-pushed the ENT-12587/master branch 2 times, most recently from 80167af to 9ff7f5e Compare January 22, 2025 21:01
Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's worth complicating the commit message, but this change means we require some API level, not a specific version of openssl. The commit message says hard dependency on the exact version and that's not the case. Any version that provides the required API level is fine (i.e. basically also any newer version). I'd reword the commit message so that in case someone goes searching for this commit, they don't get the impression that if there's a CVE in OpenSSL, they will be stuck on the some version because of CFEngine.

@craigcomstock
Copy link
Contributor Author

Not sure if it's worth complicating the commit message, but this change means we require some API level, not a specific version of openssl. The commit message says hard dependency on the exact version and that's not the case. Any version that provides the required API level is fine (i.e. basically also any newer version). I'd reword the commit message so that in case someone goes searching for this commit, they don't get the impression that if there's a CVE in OpenSSL, they will be stuck on the some version because of CFEngine.

Agreed. They will be required at a minimum VERSION of OpenSSL nothing to do with an API requirement except that yes, the version we build with is the API version we build against.

Happy to change the commit message to reflect this.

@craigcomstock
Copy link
Contributor Author

@cf-bottom jenkins with exotics no tests (I will test manually installing on non-updated rhel-8/9)

@cf-bottom
Copy link

Alright, I triggered a build:

Build Status

(with exotics) [NO TESTS]

Jenkins: https://ci.cfengine.com/job/pr-pipeline/12092/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-12092/

@craigcomstock
Copy link
Contributor Author

had a typo, retry: @cf-bottom jenkins with exotics no tests

@cf-bottom
Copy link

Sure, I triggered a build:

Build Status

(with exotics) [NO TESTS]

Jenkins: https://ci.cfengine.com/job/pr-pipeline/12093/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-12093/

@vpodzime
Copy link
Contributor

vpodzime commented May 5, 2025

They will be required at a minimum VERSION of OpenSSL nothing to do with an API requirement except that yes, the version we build with is the API version we build against.

TL;DR

It has to do with API level requirements, really.

Just to elaborate on what I meant

The provides returned by rpm -q --provides are based on symbol names/versions in the libraries' ELF files. So even if we build with openssl-3.5.0 the highest symbol name/version in the libssl and libcrypto libraries will determine the provides and thus our requires. If we wanted to go with a finer-grained approach, we would check the libraries and binaries we build and determine the highest symbol/name version based on what we actually link with. Then, even when building with openssl-3.5.0 we could easily require 3.2.0 or higher or something similar as long as we and our dependencies didn't use any newer symbols.

Thanks for adjusting the commit message!

@craigcomstock
Copy link
Contributor Author

They will be required at a minimum VERSION of OpenSSL nothing to do with an API requirement except that yes, the version we build with is the API version we build against.

TL;DR

It has to do with API level requirements, really.

Just to elaborate on what I meant

The provides returned by rpm -q --provides are based on symbol names/versions in the libraries' ELF files. So even if we build with openssl-3.5.0 the highest symbol name/version in the libssl and libcrypto libraries will determine the provides and thus our requires. If we wanted to go with a finer-grained approach, we would check the libraries and binaries we build and determine the highest symbol/name version based on what we actually link with. Then, even when building with openssl-3.5.0 we could easily require 3.2.0 or higher or something similar as long as we and our dependencies didn't use any newer symbols.

Thanks for adjusting the commit message!

Right. Reviewing the output of -q --provides openssl-libs and checking man rpm which says

       --provides
              List capabilities this package provides.

I see that doesn't provide ANY inkling of what information it is giving so didn't indicate to me any of what you are mentioning. Bad docs for sure I'd say.

So. As I understand it from you this -q --provides openssl-libs examines symbols in the libraries and returns essentially the Version Definitions from the ELF/shared-object (visible with objdump -p /usr/lib64/libcrypto.so for example). I don't quite understand what these are but will trust that they are linked dependencies between libcrypto.so and libssl.so.

Correct, so it is an API version expressed in a version which we then use in Requires.

I will amend the comment again to be more clear.

…endency version

We build against systems with the latest available dependencies such as OpenSSL.
We examine the highest "Version Definition" in the OpenSSL libraries which gives us the highest "API" and then use that as a requirement in our rpm package spec files.
This should ensure that when packages are installed with yum/dnf any required OpenSSL package upgrades will be performed or the installation will fail.

Ticket: ENT-12587
Changelog: title
@craigcomstock
Copy link
Contributor Author

Thanks @vpodzime and @larsewi . I will test the packages when they are ready and if OK I will merge and fixup the cherry picks with these commits.

@vpodzime
Copy link
Contributor

vpodzime commented May 6, 2025

As I understand it from you this -q --provides openssl-libs examines symbols in the libraries and returns

Not exactly. When you do -q --provides, it only gives you whatever is stored in the RPM DB (or an RPM package, if you point it to a file). The symbol examination happens when an RPM is built and populates the metadata. Similarly, when an RPM is built, dependencies on dynamic libraries are determined based on the ELF files in the package and their NEEDED info. Which is what we disable because we ship our dependencies ourselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants