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

Update openssl-src to 300? #1645

Closed
link2xt opened this issue May 27, 2022 · 19 comments · Fixed by #1925
Closed

Update openssl-src to 300? #1645

link2xt opened this issue May 27, 2022 · 19 comments · Fixed by #1925

Comments

@link2xt
Copy link

link2xt commented May 27, 2022

Currently openssl-sys crate references openssl-src version 111. Is there any plan to update to OpenSSL 3.0?

@sfackler
Copy link
Owner

OpenSSL 3 unfortunately has some pretty severe performance regressions. The vendored feature will use 1.1.1 until the upstream situation improves: #1578.

@amousset
Copy link
Contributor

We will likely need to soon switch to openssl 3 anyway as 1.1 will reach EOL (11th September 2023).

@sfackler
Copy link
Owner

Yeah, it's probably time. OpenSSL 3.1.0 seems to have at least partially resolved the performance regressions.

@Sytten
Copy link

Sytten commented May 15, 2023

I would be very interested in keeping a branch of openssl-sys on openssl 1.1
Some of us need to interact/maintain legacy systems that won't go off because the EOL was decided on a certain date

@amousset
Copy link
Contributor

amousset commented May 15, 2023

This issue is only about the version embedded when using the vendored feature (which should always use a supported version IMO), not the general compatibility of the crate.

@Sytten
Copy link

Sytten commented May 15, 2023

Then I might suggest keeping a feature vendored-1.1 or something like that for a little while. I might be in the wrong but we did have issues with our Tauri application when not vendoring because it was expecting openssl 1.1 and only openssl 3 was installed (ubuntu 22.04). We support ubuntu 20.04 which is only EOL in 2025 and it doesn't have an official port of openssl 3.

@daprilik
Copy link

daprilik commented May 15, 2023

For folks interested in using vendored openssl 3.0 today, I've figured out a somewhat hacky approach that allows patching openssl-src in the crate-graph without running into cargo [patch] semver issues.

Create a new crate called openssl-src-hack, with the following Cargo.toml and src/lib.rs:

[package]
name = "openssl-src"
version = "111.25.2+1.1.1t" # !! MUST MATCH VERSION IN `Cargo.lock` !!

[dependencies]
openssl-src = "300.1.0+3.1.0"
pub use openssl_src::*;

Then, patch this crate in via [patch] in your project's Cargo.toml

[patch.crates-io]
"openssl-src" = { path = "./openssl-src-hack" }

It ain't pretty... but it does indeed work, as while the openssl-src versions are ostensibly semver incompatible with one another (i.e: you shouldn't be able to patch a version = "111.25.2+1.1.1t" with a version = "300.1.0+3.1.0"), the actual crate-level API is totally compatible, allowing the patch to go off without a hitch.

@sfackler
Copy link
Owner

@Sytten If you're using the vendored feature then the version of OpenSSL on the host doesn't matter.

@Sytten
Copy link

Sytten commented May 18, 2023

Agreed thats what I am using, I would just like to retain vendored 1.1.1 maybe under a different feature?

@sfackler
Copy link
Owner

If you want to continue using an EOL version of OpenSSL you can build it yourself.

@Sytten
Copy link

Sytten commented May 18, 2023

I guess that's fair, I just thought that adding a feature vendored-1.1 would really not be a big maintenance burden and would simplify the life of some people. The current vendor setup is very nice to use and builds seamlessly as part of the larger build, something you would abruptly lose. Happy to contribute the 5 lines of code that this represents if you change your mind.

@micolous
Copy link
Contributor

micolous commented Jun 13, 2023

I guess that's fair, I just thought that adding a feature vendored-1.1 would really not be a big maintenance burden and would simplify the life of some people.

Your proposal (as-written) violates feature unification principle that all features are additive: if the openssl-sys/vendored feature normally pulled in OpenSSL v3, any dependency which tried to pull openssl-sys/vendored-1.1 would change the behaviour of openssl-sys in a non-additive way, and thus also the openssl crate. This causes a problem if any dependency requires OpenSSL v3 (either by API differences, or because of bugs in older versions).

Currently, the only way a package can enforce minimum OpenSSL versions is by failing to build when it encounters them. Keeping such build scripts around is a significant maintenance burden for those package authors... and that's a best-case scenario: the package developers may not even test against old OpenSSL, and just have users encountering bugs in their software!

For a simple solution for you: you should pin a specific (old) version of the openssl crate and also not use vendored. By using vendored, you're missing out on any backported patches for OpenSSL v1.1 from your OS/distribution vendor under an extended support agreement, which go beyond what the OpenSSL project (will soon cease) providing, and (should you need it) be built in a way which meets particular cryptographic certifications or regulatory mandates.

@Sytten
Copy link

Sytten commented Jun 13, 2023

Your proposal (as-written) violates feature unification principle that all features are additive

This is not an argument, most popular crates added features like uuid-1 and uuid-0.9 during the transition period to ease the developer experience.

Currently, the only way a package can enforce minimum OpenSSL versions is by failing to build when it encounters them

Its not the job a package to decide which version is used IMO. You cannot say that and say in the next paragraph that the OS should provide the openssl version. Those arguments are contrary to each other. If the binding is present you can use it and it's the responsibility of the developer to make sure they are present at runtime/link time.

For a simple solution for you: you should pin a specific (old) version of the openssl crate and also not use vendored

Pinning will at some point be impossible when sub dependency move on and require an higher version of openssl-sys. Broad statement about why static linking is bad are unhelpful and I explained my particular use case in the past. End users don't care about that, they care that their software starts, no matter if they are on Ubuntu 20.04 or Ubuntu 22.04. Turns out APT is a mess and if you link against openssl 3 you have problems on 20.04 and if you link against 1.1 you have problems on 22.04.

I am not letting this go because I know it will cause problems if this transition is not handled correctly. I am not trying to be pedantic for the sake of it.

@sfackler
Copy link
Owner

Are you planning on paying $50,000 a year to the OpenSSL foundation for extended support of 1.1.1? Adding a vendored-1.1 feature for 3 months and then removing is not a reasonable option.

@Skepfyr
Copy link
Collaborator

Skepfyr commented Jun 13, 2023

@Sytten I may be wrong but I think there's a misunderstanding here, vendored is not the same as static linking, you can set the OPENSSL_STATIC environment variable at build time to statically link against a provided version of OpenSSL. vendored is there to provide a simple default for people who don't care, hence the pushback for adding a vendored-1.1 feature, if you care that much then just build it against your preferred version of OpenSSL. We clearly don't want to make it easy to link against an EOL version of OpenSSL so don't want a vendored-1.1 long term. Granted this will make building against a static openssl-1.1 harder, but it's definitely won't be impossible.

Additionally, I suspect we will keep testing openssl-sys against OpenSSL 1.1.1 for a long time, definitely while it's still shipped with major distros, so you don't need to worry about losing support entirely.

@Sytten
Copy link

Sytten commented Jun 13, 2023

@Skepfyr Yeah that is probably acceptable. I just replied on the other comment because I thought the arguments where invalid for the why and I was triggered 😅

I would add some documentation on how to do that (probably not that hard with openssl-src pinned and a build script) in the readme and do a clear breaking change message. At least there is this thread, but still.

@micolous
Copy link
Contributor

micolous commented Jun 14, 2023

@Sytten I'm going to rearrange your points here, because I think it'll help.

[...] Broad statement about why static linking is bad are unhelpful and I explained my particular use case in the past. End users don't care about that, they care that their software starts, no matter if they are on Ubuntu 20.04 or Ubuntu 22.04. [...] I am not letting this go because I know it will cause problems if this transition is not handled correctly. I am not trying to be pedantic for the sake of it.

I understand loud and clear that you deeply care about your end-users' experience, and there was never any doubt in my mind of that. It was absolutely not my intention to solicit an emotional response.

The reason I'm making this commentary I am is because I also care about my users' experience – both end-users and developers. I don't want them to discover their OpenSSL is broken by my library crashing at runtime, or have something in their dependency chain pulling in openssl/vendored-1.1. Ideally, I want the person building the software to realise what they built won't work before they send it to anyone else!

[...] Most popular crates added features like uuid-1 and uuid-0.9 during the transition period to ease the developer experience.

That's something different entirely – it allows packages to build with equivalent functionality on both versions of uuid, because the major version change means the types are treated differently. There's also overhead in supporting that, because you need to account for API differences between those libraries.

I didn't think it needed to be said, but to demonstrate I'm being consistent, I would argue changing which major OpenSSL version is vendored is also a SemVer-incompatible change... but I can sympathise with why quick action was taken. 😄

Its not the job a package to decide which version is used IMO.

It is when they're impacted by bugs in OpenSSL which make them unusable: for me, webauthn-authenticator-rs needs openssl/openssl#12826. The broken routine means my program crashes at runtime without that patch. I can't change how OpenSSL is used in order to work around it without implementing a cryptographic protocol incorrectly.

In order to protect my users' experience, I explicitly check to see if the feature I want is usable, and if it is not, then I tell them to upgrade to OpenSSL v3.0.0 or later. This also means that any vendored feature which points at OpenSSL 1.1 (be it the current one, or a future openssl-1.1) breaks my software.

It's easiest for me to say "use OpenSSL v3.0.0 or later", as they can look at their distribution's package repositories and figure out what they need to upgrade to. If that patch eventually gets backported to OpenSSL v1.1, then that may be an option... but I don't see this coming any time soon that will probably never happen, as OpenSSL only applies security fixes in the last year of support. Everyone else is migrating to OpenSSL v3.x.

Once a binary is built with OpenSSL:

  • If it's dynamically linked, it's against libcrypto.so.3 and libssl.so.3. Systems which only have OpenSSL 1.1 have libcrypto.so.1.1 and libssl.so.1.1 instead.
  • If it's statically linked, then it doesn't matter what the system version is.

I think that what I've done meets your expectations:

it's the responsibility of the developer to make sure they are present at runtime/link time.

If I allowed linking against versions of OpenSSL that which were provably broken, then I would be eschewing that responsibility.

Likewise, there are others reporting issues because of things that need features in OpenSSL v3.

Turns out APT is a mess and if you link against openssl 3 you have problems on 20.04 and if you link against 1.1 you have problems on 22.04.

I think @Skepfyr is correct here – this shouldn't matter if you're vendoring and have statically linked them, and you should be able to freely use either OpenSSL version on either distribution.

OPENSSL_STATIC will explicitly override whether openssl-sys uses static or dynamic libraries, openssl-src builds OpenSSL statically, and openssl-sys will default to using dynamic libraries if both are present.

I was unable to reproduce the symptoms you saw (dependencies on system OpenSSL when vendoring) with the openssl crate tests on a Linux machine with OpenSSL 3:

$ openssl version
OpenSSL 3.0.8 7 Feb 2023 (Library: OpenSSL 3.0.8 7 Feb 2023)

$ git clone https://github.com/sfackler/rust-openssl.git
$ cd rust-openssl

## Build vendored version
$ cargo test -p openssl --features vendored
   Compiling openssl-sys v0.9.88 (~/rust-openssl/openssl-sys)
   Compiling openssl-macros v0.1.1 (~/rust-openssl/openssl-macros)
   Compiling openssl v0.10.54 (~/rust-openssl/openssl)
    Finished test [unoptimized + debuginfo] target(s) in 35.97s
     Running unittests src/lib.rs (target/debug/deps/openssl-986641eebe96ea17)
[...] tests pass

## Check dynamic linkages
$ ldd target/debug/deps/openssl-986641eebe96ea17
	linux-vdso.so.1 (0x00007ffc4ffac000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f0fbcb5f000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f0fbc519000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f0fbc31e000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f0fbcb96000)
## Verify presence of OpenSSL in binary
$ strings target/debug/deps/openssl-986641eebe96ea17 | grep -E "OpenSSL.+202"
OpenSSL 1.1.1u  30 May 2023

## Build system linked version
$ cargo test -p openssl
   Compiling openssl-sys v0.9.88 (~/rust-openssl/openssl-sys)
   Compiling openssl v0.10.54 (~/rust-openssl/openssl)
    Finished test [unoptimized + debuginfo] target(s) in 6.31s
     Running unittests src/lib.rs (target/debug/deps/openssl-987f335285143245)
[...] tests pass

## Check dynamic linkages, uses system OpenSSL
$ ldd target/debug/deps/openssl-987f335285143245
	linux-vdso.so.1 (0x00007ffee91d9000)
	libssl.so.3 => /lib64/glibc-hwcaps/x86-64-v3/libssl.so.3.0.8 (0x00007fa479960000)
	libcrypto.so.3 => /lib64/glibc-hwcaps/x86-64-v3/libcrypto.so.3.0.8 (0x00007fa479400000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fa479d1c000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fa479879000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fa479205000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa479d53000)
	libz.so.1 => /lib64/glibc-hwcaps/x86-64-v3/libz.so.1.2.13 (0x00007fa479d00000)

## Verify absence of OpenSSL in binary
$ strings target/debug/deps/openssl-987f335285143245 | grep -E "OpenSSL.+202"
[no hits]

## Verify that would find the OpenSSL version string
$ strings /lib64/glibc-hwcaps/x86-64-v3/libcrypto.so.3 | grep -E "OpenSSL.+202"
OpenSSL 3.0.8 7 Feb 2023

You can verify how your own binaries are built the same way, and the file names will show you whether you're linked against OpenSSL v1.1 or v3. If you're able to reproduce the issue and trace it down to a problem in the openssl-* crates, I'd suggest you open another bug for this, so you can get migration to OpenSSL v3 unblocked.

On a side note, I'm particularly cautious about vendoring after dealing with clients who were compromised as a result of enterprise application vendors bundling years-old versions of software with well-known security vulnerabilities – even on the latest version of that package configured according to the vendor's recommendations. It is extremely difficult to trace those things down.

Generally speaking, application developers have a responsibility to ensure that they are patching any bundled dependencies (whether that be dynamic or static libraries, or Rust package dependencies) in a timely manner. This is especially important in the Tauri ecosystem, which requires careful enforcement of boundaries between web and local content, so the best strategy is defence in depth.

@wez
Copy link

wez commented Jul 9, 2023

What's preventing #1925 from being merged and released?

@amousset
Copy link
Contributor

amousset commented Jul 9, 2023

Investigating the failing tests and fixing what's broken, mainly. I'll try to have a look at it soon.

wez added a commit to wez/wezterm that referenced this issue Sep 4, 2023
wez added a commit to wez/libssh-rs that referenced this issue Oct 4, 2023
BlackDex added a commit to BlackDex/rust-openssl that referenced this issue Nov 14, 2023
Since sfackler#1925 building static binaries fails because of a hardcoded `dylib`.
Removing the `dylib` but still let rust search for the dependency it
will still link it.

This should still keep sfackler#1645 fixed but also fixes sfackler#2043.

I have tested this by building Vaultwarden for both dynamically linked
debian based image, and a statically linked musl/alpine based image.
But are using OpenSSL v3.x.x.
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 a pull request may close this issue.

8 participants