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

Remove support for openssl older than 1.1.1 #1397

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

Conversation

jonesmz
Copy link
Contributor

@jonesmz jonesmz commented Jan 22, 2024

Openssl 1.1.1 is end-of-life in September 2023.
This PR removes support for versions of openssl OLDER than 1.1.1
1.1.1 should still be usable after this change is merged.

I don't see any value in supporting 1.1.1, but didn't see a reason to purge support for 1.1.1 when there are so few checks for >= 3.0.

Note that this does also remove CI support for Ubuntu 16.04. The official version of OpenSSL from Ubuntu for this release is listed here: https://launchpad.net/ubuntu/+source/openssl as 1.0.2g

Since no newer releases of coturn will be backported by Canonical to Ubuntu 16.04, anyone using Coturn on this operating system will have to download and compile it themselves. They may build their own version of OpenSSL if they somehow cannot upgrade to a newer version of Ubuntu.

My position is that these users should prefer to upgrade to a newer operating system than worry about chasing newer releases of Coturn.

@jonesmz jonesmz force-pushed the openssl-1.1.1-eol branch 2 times, most recently from 6f1eb2b to 5e66652 Compare January 22, 2024 18:42
@jonesmz
Copy link
Contributor Author

jonesmz commented Jan 22, 2024

The action

Docker CI / test (debian, i386) (pull_request) 

failed, but didn't in my own repo.

I suspect this test is flaky.

Can a maintainer re-trigger it?

@jonesmz jonesmz force-pushed the openssl-1.1.1-eol branch 2 times, most recently from 5e66652 to ec31a96 Compare January 24, 2024 17:31
@eakraly
Copy link
Collaborator

eakraly commented Jan 28, 2024

Thank you for the contribution @jonesmz !

I feel like this needs some discussion. As someone who rewrote a bunch of ssl related code recently I can totally relate. But also, we have no idea if people are still building coturn for that older version.

What I suggest is that we keep this PR until we tag a new version (4.6.3) which would be last one to support openssl <1.1.1 and then merge this PR (for next version 4.7.0).

What do you think?

@jonesmz
Copy link
Contributor Author

jonesmz commented Jan 28, 2024

I feel like this needs some discussion. As someone who rewrote a bunch of ssl related code recently I can totally relate. But also, we have no idea if people are still building coturn for that older version.

I mean ultimately it's not my call, so whatever you think is best.

I'm not sure that the criteria should involve whether people still use the old version of OpenSSL. Regardless of any other thing involved in a support decision, ultimately the deciding factor is whether the people who maintain the project are willing to continue supporting a version, not whether anyone is using the old version.

OpenSSL <1.1.1 have a much different API than 1.1.1 and newer, (as evidenced by by this patch). The amount of extra work to support this is noticeable. My team at work has put in a disproportionate amount of time, compared to the total, stumbling over the details of the old OpenSSL code, and my team's time is a finite resource, so I want to eliminate the bottleneck as soon as practical.

My work also has contracts that forbid us from deploying versions of OpenSSL that are out of support (so OpenSSL 1.1.1 and older), so my boss will give me flack if I put any time into continuing to support it. I don't have to go purge it, but I'm not excited about having to explain why I'm submitting PR's that adjust OpenSSL 1.0.1 code, even if it's a tiny amount of time relative to the rest of the changes. I wont get in trouble or anything, just eyebrow raises.

As for the philosophical questions regarding "should support" or "should use" and so on, I think it's pretty unlikely that someone is simultaneously:

  1. Unable to upgrade their operating system to a version that comes with a newer OpenSSL
  2. Unable to build a newer version of OpenSSL for their OS for coturn to use
  3. Building coturn themselves instead of using a pre-built package
  4. Intending to upgrade to newer releases of coturn
  5. Not intending to carefully review the patches between different versions of coturn when they upgrade.

You say in this comment on another PR ( OpenSSL ) that Ubuntu 16.04 is kept in the C.I. specifically for testing against older OpenSSL versions.

But the tests (the few that coturn has) don't all run on ubuntu 16.04. Most of them do, but not everything.

../bin/turnserver: error while loading shared libraries: libmicrohttpd.so.12: cannot open shared object file: No such file or directory

I think the benefits of dropping OpenSSL <1.1.1 outweigh the negatives by a large amount, personally.

What I suggest is that we keep this PR until we tag a new version (4.6.3) which would be last one to support openssl <1.1.1 and then merge this PR (for next version 4.7.0).

What do you think?

If that's how you want to do it then that's fine.

If you want to do that, I would prefer that 4.6.3 be tagged soon (next week or two, if that's doable?), so that I can make new PRs that build off of this one soon.

I recommend:

  1. Drop anything that's end-of-life, such as OpenSSL <1.1.1
  2. Drop support support for any operating system that isn't part of the CI. I care about AmazonLinux2 for my work so I will ensure that it's supported in the CI until it's end-of-life (June 30, 2025) or my work switches to AmazonLinux2023.
  3. Support only the latest release of any supported operating system, or at most the latest two. This would be, for example, Ubuntu 20.04 and 22.04, then drop 20.04 when the github runner is upgraded to 24.04.
  4. For OSes that the github CI doesn't accommodate, I don't have a good recommendation other than requesting that interested parties provide hosted runners to support their OS of preference.

But just removing OpenSSL <1.1.1 is sufficient for my needs regardless of my recommendations.

@Neustradamus
Copy link

To follow

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.

None yet

3 participants