-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
openssl: add 3.0.8 + add GitHub as mirror #15802
openssl: add 3.0.8 + add GitHub as mirror #15802
Conversation
I detected other pull requests that are modifying openssl/3.x.x recipe:
This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This comment has been minimized.
This comment has been minimized.
What's the status on this? Is anybody actively looking into it? Much appreciated! |
You can checkout this bot prince-chrismc/conan-center-index-pending-review#1 which will give you a rundown of all the PRs and the bottom has graphs about how long they are taking |
Ill need to check why this failed again from my MacBook 😕 |
Is this something that happened recently? Can you give me a cross-reference please? This PR doesn't modify 3.0.5 at all, so it is weird. |
Any movement on this? We're waiting on this to release our product so we can incorporate the latest OpenSSL security fixes... |
This comment has been minimized.
This comment has been minimized.
cc4b8a0
to
be988da
Compare
Random thing that came up with a search: Maybe the openssl config files is making problems while loading the legacy stuff nodejs/node#43184 Env var The question remains what changed in the CI environment that surfaced this error? It is really bad that critical updates like these are blocked by unrelated bugs. Maybe we should disable the tests with the legacy providers to get this merged first. |
This comment has been minimized.
This comment has been minimized.
I am following this issue closely as we are waiting on this PR being merged to make a new release of our software to respond to the disclosed openssl vulnerabilities. I could not agree more with your statement about how damaging it is from a security standpoint. Hope this issue will be resolved soon. Thanks for your work ! |
We are actually already skipping tests for OpenSSL 1.x on macOS armv8 shared
|
yes but for another reason (and this skip_test in 1.x.x branch should be removed because CMake has been upgraded on Macos agents). |
This comment has been minimized.
This comment has been minimized.
Do that plus add some links for reviewers and it can go in :) |
It's a suggestion which would make sense given the error and OpenSSL documentation regarding env var and loading mechanism of modules. I let OpenSSL experts here give their opinion and provide the fix. |
Can this be replicated locally? I tried it on my Mac with
Both |
@prince-chrismc @SpaceIm anything that can be done to get this merged faster? We're holding off an entire release because of this. Much appreciated! |
@gegles please contribute the fix 🙏 that will ve the fastest way :) |
I have not been able to reproduce locally and I do not have access to the full CI logs. What is the best way to contribute? Also, I diffed the 3.0.7 and 3.0.8 release tags and this changes looks like it might be relevant:
|
You can access the logs from EDIT: I suggested docker images but that wont work for macos nodes |
|
I can't reproduce this locally either. I'm not sure about those environment variables, but why would they be suddenly required? And why only Windows and macOS? |
Hi @Croydon - we are currently investigating this. Could you please add the following to the recipe so that we can verify the build configuration? I've changed the PR to draft while we continue to investigate this diff --git a/recipes/openssl/3.x.x/conanfile.py b/recipes/openssl/3.x.x/conanfile.py
index 05de257a5..2a7b19e88 100644
--- a/recipes/openssl/3.x.x/conanfile.py
+++ b/recipes/openssl/3.x.x/conanfile.py
@@ -610,6 +610,7 @@ class OpenSSLConan(ConanFile):
self._create_targets()
with self._make_context():
self._make()
+ self.run("perl source_subfolder/configdata.pm --dump")
@property
def _win_bash(self) ☝️ Please disregard this, see below. |
There are two "loadable" modules in the package, irrespective of whether the recipe is built as
The issue is that OpenSSL will look in the directory provided at install time - this is why we cannot reproduce this locally, because those directories will exist on everyone's local machine as part of Conan create. An easier way to test this is to clear the binaries from the Conan cache, and run:
and let Conan fetch the OpenSSL binaries from Conan Center - we should now see the same error. Inspecting the process, I can see it tries to look in a path inside Adding this to the diff --git a/recipes/openssl/3.x.x/conanfile.py b/recipes/openssl/3.x.x/conanfile.py
index 05de257a5..34311f48a 100644
--- a/recipes/openssl/3.x.x/conanfile.py
+++ b/recipes/openssl/3.x.x/conanfile.py
@@ -756,3 +756,9 @@ class OpenSSLConan(ConanFile):
self.cpp_info.components["crypto"].names["cmake_find_package_multi"] = "Crypto"
self.cpp_info.components["ssl"].names["cmake_find_package"] = "SSL"
self.cpp_info.components["ssl"].names["cmake_find_package_multi"] = "SSL"
+
+ openssl_modules_dir = os.path.join(self.package_folder, "lib", "ossl-modules")
+ self.runenv_info.define_path("OPENSSL_MODULES", openssl_modules_dir)
+
+ # For legacy 1.x downstream consumers, remove once recipe is 2.0 only:
+ self.env_info.OPENSSL_MODULES = openssl_modules_dir This is a rough patch that should fix the issue - I'd advise to also add some comments in the recipe clarifying the need for this. THis env-var is documented in: https://www.openssl.org/docs/man3.0/man7/openssl-env.html Thanks!
@Croydon - simple coincidence. The paths are absolute to the package folder inside the Conan cache, and may have just coincided when |
Signed-off-by: Uilian Ries <[email protected]>
b8f80e8
@@ -610,6 +610,7 @@ def build(self): | |||
self._create_targets() | |||
with self._make_context(): | |||
self._make() | |||
self.run("perl source_subfolder/configdata.pm --dump") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's code to dump OpenSSL configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check this out: https://github.com/openssl/openssl/blob/master/Configure#L32-L48
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was debug code from us testing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the results are posted i will remove it :)
self.run("perl source_subfolder/configdata.pm --dump") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be useful for troubleshooting in future, e.g. debugging bugs, it's worth keeping
self.runenv_info.define_path("OPENSSL_MODULES", openssl_modules_dir) | ||
|
||
# For legacy 1.x downstream consumers, remove once recipe is 2.0 only: | ||
self.env_info.OPENSSL_MODULES = openssl_modules_dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we set more variables listed here? https://www.openssl.org/docs/man3.0/man7/openssl-env.html
Thanks everyone for investigating this. The CI's main job failed without a reported error. I'm going to restart the CI |
Conan v1 pipeline ✔️All green in build 12 (
Conan v2 pipeline (informative, not required for merge) ❌
The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future. See details:Failure in build 11 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
https://www.openssl.org/news/openssl-3.0-notes.html
For 1.1.1t see #15860