search

awslabs / aws-crt-python

Python bindings for the AWS Common Runtime
Apache License 2.0
87 stars 43 forks source link

Change from engines to providers in OpenSSL #583

Closed major closed 1 month ago

major commented 2 months ago

Describe the feature

Per the OpenSSL wiki, developers are encouraged to use the new provider APIs instead of engines. Could this change be made within aws-crt-python?

Use Case

This change is required for upcoming OpenSSL changes in Fedora, CentOS, and RHEL distributions.

Proposed Solution

No response

Other Information

No response

Acknowledgements

DmitriyMusatkin commented 2 months ago

If im reading the Openssl wiki correctly, this applies only to methods modifying engine state or custom methods.

All of crt openssl usage comes from https://github.com/awslabs/aws-c-cal and we currently do not touch *_meth_new at all and have a single place in the code that does anything with the engine (passes null for the engine ptr). So i think we should be good for this and the is no immediate work needed.

github-actions[bot] commented 1 month ago

Greetings! It looks like this issue hasn’t been active in longer than a week. We encourage you to check if this is still an issue in the latest release. Because it has been longer than a week since the last update on this, and in the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or add an upvote to prevent automatic closure, or if the issue is already closed, please feel free to open a new one.

neverpanic commented 1 month ago

This package still fails to build against a copy of OpenSSL built with no-engine and thus without ENGINE support, because the the s2n submodule contains https://github.com/aws/s2n-tls/blob/main/utils/s2n_random.c#L50, which unconditionally includes openssl/engine.h, which does not exist. This include should be guarded by #ifndef OPENSSL_NO_ENGINE and the relevant code around https://github.com/aws/s2n-tls/blob/main/utils/s2n_random.c#L556-L576 and other locations should be adjusted to transparently deal with this.

graebm commented 1 month ago

Cool, whenever S2N fixes this, we'll automatically pick it up. We regularly pull in their latest tagged release