Skip to content
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

(PA-6131) Bump openssl version #807

Closed
wants to merge 1 commit into from

Conversation

amitkarsale
Copy link
Contributor

Bump open-ssl version from 3.1.12 to 3.1.13 which address following vulnerabilities

NVD - CVE-2023-5678

NVD - CVE-2023-6129

CVE-2023-6237

NVD - CVE-2024-0727

@amitkarsale amitkarsale requested review from a team as code owners March 26, 2024 05:01
@amitkarsale amitkarsale force-pushed the PA-6131 branch 2 times, most recently from 966df28 to 68c4951 Compare March 26, 2024 06:01
@amitkarsale amitkarsale marked this pull request as draft March 27, 2024 11:47
@cthorn42
Copy link
Collaborator

Do we have one vanagon generic builder job that tests this PR and includes all platforms for runtime main?

@amitkarsale
Copy link
Contributor Author

amitkarsale commented Apr 1, 2024

@cthorn42 , are these platforms sufficient to consider its verified for main or do we need to add some more platforms?

https://jenkins-platform.delivery.puppetlabs.net/view/vanagon-generic-builder/job/platform_vanagon-generic-builder_vanagon-packaging_generic-builder/2769/

@amitkarsale amitkarsale marked this pull request as ready for review April 1, 2024 09:22
@joshcooper
Copy link
Contributor

@amitkarsale can you rebase the PR instead of git pull? For example:

git reset --hard be0e6f12056b0e660d0cc2eac235e2ca7a961e75
git rebase --onto upstream/master HEAD^
git push -f origin HEAD

The reason is merge commits clutter up the git history and make things hard to follow later.

@joshcooper
Copy link
Contributor

are these platforms sufficient to consider its verified for main or do we need to add some more platforms?

I commented in slack but posting here for visibility.

If you're making a change that only affects a few platforms, then it's fine to just test those platforms in your PR. To confirm that's the case use this rake task. The agent-runtime-main is correct for this PR, but if you were testing 7.x, you'd want to specify agent-runtime-7.x.

bundle exec rake vanagon:component_diff -- -P agent-runtime-main -p all --from upstream/master --to HEAD

If you're bumping a component that contains C/C++ code (like ruby, curl, openssl), then it's better to test all of the platforms from the respective pipeline. For example, in this PR, you're bumping openssl 3.0.13 which is only used in puppet8, so I'd test against all of the BUILD_TARGETS from the main pipeline

Finally, if it's a rubygems bump and that gem doesn't contain native extensions like https://github.com/puppetlabs/puppet-runtime/blob/master/configs/components/rubygem-semantic_puppet.rb, then it's fine to just build on one platform like el-7-x86_64

@joshcooper
Copy link
Contributor

Given the code complete deadline, I rebased your PR and triggered generic builder on all agent-runtime-main platforms: https://jenkins-platform.delivery.puppetlabs.net/view/vanagon-generic-builder/job/platform_vanagon-generic-builder_vanagon-packaging_generic-builder/2776/

@joshcooper
Copy link
Contributor

Generic builder failed.

osx14 may be transient

15:42:17 checking for a BSD-compatible install... /usr/bin/install -c
15:42:18 checking whether build environment is sane... configure: error: newly created file is older than distributed files!
15:42:18 Check your system clock

windows looks like a real failure

15:35:36 cd openssl-3.0.13 && \
15:35:36  ./Configure --prefix=C:/ProgramFiles64Folder/PuppetLabs/Puppet/puppet --libdir=lib --openssldir=C:/ProgramFiles64Folder/PuppetLabs/Puppet/puppet/ssl shared no-gost mingw64 no-camellia no-md2 no-ssl3 no-ssl3-method no-dtls1-method no-dtls1_2-method no-aria no-rc5 no-mdc2 no-whirlpool no-legacy no-md4 no-dtls no-dtls1 no-idea no-seed no-weak-ssl-ciphers -DOPENSSL_NO_HEARTBEATS
15:35:36 Configuring OpenSSL version 3.0.13 for target mingw64
15:35:36 Using os-specific seed configuration
15:35:44 
15:35:44 Failure!  Makefile wasn't produced.

When using 3.0.12 on Windows:

11:44:21 cd openssl-3.0.12 && \
11:44:21  ./Configure --prefix=C:/ProgramFiles64Folder/PuppetLabs/Puppet/puppet --libdir=lib --openssldir=C:/ProgramFiles64Folder/PuppetLabs/Puppet/puppet/ssl shared no-gost mingw64 no-camellia no-md2 no-ssl3 no-ssl3-method no-dtls1-method no-dtls1_2-method no-aria no-rc5 no-mdc2 no-whirlpool no-legacy no-md4 no-dtls no-dtls1 no-idea no-seed no-weak-ssl-ciphers -DOPENSSL_NO_HEARTBEATS
11:44:21 Configuring OpenSSL version 3.0.12 for target mingw64
11:44:21 Using os-specific seed configuration
11:44:32 Created configdata.pm
11:44:32 Running configdata.pm
11:44:36 Created Makefile.in
11:44:36 Created Makefile
11:44:36 Created include/openssl/configuration.h

use Cwd qw/realpath/;
-
+
+ mkdir $dir unless -d $dir;
Copy link
Contributor

Choose a reason for hiding this comment

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

This works around the issue, but it seems less than ideal to create a directory as a side effect of calling absolutedir. It looks like this issue has been fixed upstream already and backported to the 3.0.x branch openssl/openssl@7b3eda5 I'm thinking we should pull in that patch instead.

@joshcooper
Copy link
Contributor

@amitkarsale given the time schedule, I've pulled your commit into a new PR #814 with the upstream patch, and am testing now.

@joshcooper joshcooper closed this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants