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 OpenSSL #229

Closed
briansmith opened this issue Nov 11, 2018 · 26 comments · Fixed by #322
Closed

Remove OpenSSL #229

briansmith opened this issue Nov 11, 2018 · 26 comments · Fixed by #322

Comments

@briansmith
Copy link
Contributor

I propose that all the OpenSSL functionality be removed from cross. I noticed a lot of the issues in the cross issue tracker are related to cross trying to maintain OpenSSL; all of these issues would just go away if OpenSSL support were removed. And, the experience of using cross for applications that do not use OpenSSL would be improved.

It's not clear why cross includes OpenSSL for some (but not all) targets in the first place. Making OpenSSl easier to use seems out of scope for a tool like this. The rust-openssl crate already has support for building a vendored copy of OpenSSL if it isn't available. If that mechanism and other ways of installing OpenSSL don't work well, that's really OpenSSL's and/or rust-openssl's problem.

In any case, I would like to use cross specifically for applications that often make some effort to avoid depending on OpenSSL. Unfortunately, cross's OpenSSL stuff gets in the way here. For example, does a Rustls application, specifically one that uses the webpki-roots crate for root certificates, work correctly if OpenSSL isn't installed and/or was never installed?

(FWIW, I am the creator of ring and webpki.)

@therealprof
Copy link
Contributor

Big agreement from my side here.

@TotalKrill
Copy link

I actually used this because it meant i did not have to compile a version of openssl myself. So would be sad to see it go. It is quite easy to remove yourself though, so a suggestion would be to create a docker image without it and upload and use that instead in the Cross.toml file.

@SirVer
Copy link

SirVer commented Feb 24, 2020

I just ran into issues because OpenSSL was removed from cross. Every single of my tools I use cross (and trust) for are using OpenSSL and since this happens somewhere deep down in my dependencies, I have little options of not using OpenSSL. Removing it means that cross has lost all value to me overnight :(.

While I fully acknowledge that the maintenance burden is on the maintainers of this repository and I can see that OpenSSL is a pita to maintain, the users of https://github.com/japaric/trust and many cross users that just want to do a web request in their code will have a lot more to deal with now. I hope this move will be reconsidered going forward.

@CryZe
Copy link
Contributor

CryZe commented Feb 24, 2020

Yeah this is likely going to cause significant problems for me as well (I switched to rustls for most targets, but rustls only works on "tier 1" platforms).

@surban
Copy link

surban commented Feb 24, 2020

Is it possible to revert to an older Docker image that still includes OpenSSL?

Edit:
cargo install --version 0.1.16 cross

@Emilgardis
Copy link
Member

@surban to use another image, use following config: https://github.com/rust-embedded/cross/blob/master/README.md#custom-docker-images

@SirVer
Copy link

SirVer commented Feb 25, 2020

Reverting to an older version worked for me too for the moment, but of course comes with the downside of bit rot over time. The convenience of cross for my CI pipeline is a fantastic selling point for rust within my organizations.

@briansmith
Copy link
Contributor Author

I'm glad this change was made because this makes it much easier for library crate developers to use cross to get an environment more like what's typical for their users. One of the biggest issues I had using cross in the past was that cross's configuration is different from what is typical in non-cross environments. As soon as a user couldn't use cross for whatever reason, they couldn't build my stuff that (accidentally) depended on OpenSSL.

@cschneid
Copy link

This suddenly broke my CI pipeline. I use trust, which in turn uses cross. I am not directly using openssl, but it is a transient dependency.

I'm not very familiar with how cross works internally, and only sorta know docker. Is it expected that I need to create a new docker file for every target I build for, which also installs openssl?

I have a short term fix of pinning to v0.1.16 as suggested above. I edited the dynamic lookup of the latest tag in trust's ci/install.sh to be hard coded to local tag="v0.1.16"

@SirVer
Copy link

SirVer commented Feb 25, 2020

@briansmith Your point is very interesting to me. It seems that cross has two major use cases, one is for crate developers, one is for cross compilers. The former probably want to have full control over the containers used, the latter mostly just want working compiles. And for the latter, OpenSSL is a very common (and very painful) dependency - I'd argue it is the only C dependency that basically all rust programs depend upon in their default configuration.

I'd argue that the cross compiler use case should be optimized for, because it probably represents the (mostly silent) majority of users and because the other use case (crate developers) is likely more savvy and interested to having full control over their environment, aka building their own cross docker base images.

Of course this is just my 2c and since I am just a user and not an active contributor I understand that maintainers need to make tradeoffs to make maintenance manageable. I still hope this change can be rolled back somehow.

@brainstorm
Copy link
Contributor

brainstorm commented Feb 26, 2020

I have an ongoing PR where I introduce support for curl-openssl on musl:

rust-bio/rust-htslib#184 (comment)

After reading this issue, I guess that my best shot is going for this project/container instead (or pinning cross to 0.1.16):

https://github.com/emk/rust-musl-builder

I also hope this removal gets somehow reconsidered :-S

@nickbabcock
Copy link

I think the quickest solution for those facing the breaking change is to explicitly depend upon a statically compiled openssl, as not every library is compatible with rustls (eg: rust-postgres).

[dependencies]
openssl = { version = '0.10', features = ["vendored"] }

We can take it a step farther for those who want to provide end users the most flexibility. It's possible for an application to have three ways to be built for a linux target:

  • Dynamically link to openssl using native-tls (the default)
  • Statically link openssl
  • Statically link rustls

The trick is to add an opt-in feature (eg: vendored-openssl) that statically compiles openssl into the binary. Below is a partial Cargo.toml snippet where we include openssl as an optional feature alongside optional rustls.

[dependencies]
openssl = { version = '0.10', optional = true }

[features]
default = ["reqwest/default-tls", "trust-dns-resolver/dns-over-native-tls"]
vendored-openssl = ["openssl/vendored"]
rustls = ["reqwest/rustls-tls", "trust-dns-resolver/dns-over-rustls"]

So now we can create the following builds

# dynamically linked openssl
cargo build

# statically link in openssl (still dynamically linked to libc)
cargo build --features vendored-openssl

# compile with rustls (but still dynamically linked to glibc)
cargo build --no-default-features --features rustls

# static build with openssl
cross build --target x86_64-unknown-linux-musl  --features vendored-openssl

# static build with rustls
cross build --target x86_64-unknown-linux-musl --no-default-features --features rustls

There's a bit of a precedence of writing cargo configs that vendor openssl, as that is what tools like wasm-pack and vector use to distribute their executables. This means that all executables for non-macos and non-windows systems (as they should rely on native-tls functionality) will include a crypto library (openssl or rustls) that is statically linked.

Due note that there is a slight downside to this approach in that it may seem wonky to select both features:

cargo build --features vendored-openssl --features rustls

But this problem seems minor.

One may need to trim their supported platforms (I had to remove netbsd as a target as I couldn't get openssl-src to compile for it). This is not a deal breaker for me.

If anyone needs to see a project employing this strategy for inspiration: https://github.com/nickbabcock/dness/tree/2daf29a6ebf1a4182369489f3638348c38cfd228

@therealprof
Copy link
Contributor

Doesn't a static OpenSSL preclude from shipping binaries built with it? Otherwise it's an insane risk that someone keeps on using a very old OpenSSL version with known security risks for a long time without even realising that. Especially with the notorious OpenSSL all red lights should turn on...

If we have such a feature we probably should add a big fat warning in case people want to distribute such binaries.

@nickbabcock
Copy link

Doesn't a static OpenSSL preclude from shipping binaries built with it?

IANAL, but I don't think that's the case.

Otherwise it's an insane risk that someone keeps on using a very old OpenSSL version with known security risks for a long time without even realising that. Especially with the notorious OpenSSL all red lights should turn on...

Yeah it's the old dynamically vs statically linked argument. The solution I gave allows one compiling from source the ability to choose their preference.

@clintfred
Copy link

Our CI pipeline will also break when we upgrade to cross 0.2.0. We discovered this issue when a developer recently installed cross and builds that were working in CI and working on other devs machines wouldn't work for him.

I guess I don't know if openssl should be in the docker image, but there are going to be lots of people with this problem, so we should have migration instructions for them. From this library maintainers perspective, I wish openssl was still in the docker images.

jaemk added a commit to jaemk/self_update that referenced this issue Jun 2, 2020
lutostag pushed a commit to lutostag/pincers that referenced this issue Aug 10, 2020
Cross upstream removed ssl libraries, using temporary workaround for now
as outlined in:
cross-rs/cross#229 (comment)
@Xuanwo
Copy link

Xuanwo commented Dec 11, 2021

Hi, I came up with this problem recently and found out that we are running into the wrong way. The problem is simple and easy to fix like described in this PR: datafuselabs/databend#3371 (comment)

RUN dpkg --add-architecture arm64 && \
    apt-get update && \
    apt-get install --assume-yes libssl-dev libssl-dev:arm64 zlib1g-dev zlib1g-dev:arm64

I think we need to add openssl support back so that we don't need to do this workaround anymore.

I'm willing to start a PR if maintainers think it's OK to go.

@Emilgardis
Copy link
Member

Emilgardis commented Dec 11, 2021

Its a hard issue, adding it back to how it was is not a solution.

If anything, I'm ok with making specific "official" cross images for certain targets which would include it. See also #580

@Mohsen7s
Copy link

Mohsen7s commented Feb 3, 2022

As @briansmith stated : issues that depend on using OpenSSL would just go away if OpenSSL support were removed
Removing OpenSSL as a solution to that its stupid as removing your internet WAN to solve all the problems that exist in internet.
There are plenty of C code libraries which are used by rust bridges and its impossible to change the C code to use alternative rustls. And OpenSSL its not a library or a tool made by just another developer. Its still a WORLDWIDE solution to a lot of projects. Removing OpenSSL and asking to go with custom docker images completely break the cross-rs main goal as described “Zero setup” cross compilation.
All I can do its to vote bring it back.

@reitermarkus
Copy link
Member

@Mohsen7s, “issues” in this context specifically meant issues in this issue tracker, not issues in general. Removing OpenSSL leads to fewer issues being opened, i.e. making the project easier to maintain.

All I can do its to vote bring it back.

You can certainly start a discussion on what the best way forward should be and to find people who will do the work once there is consensus.

@surban
Copy link

surban commented Feb 7, 2022

Perhaps a more general solution would be to add "features" to the base Docker image that can be selected like Cargo features? This would save users that need native libraries from customizing Docker images, while preserving a clean base image for everyone else.

@Emilgardis
Copy link
Member

Perhaps a more general solution would be to add "features" to the base Docker image that can be selected like Cargo features? This would save users that need native libraries from customizing Docker images, while preserving a clean base image for everyone else.

This is what I hope we can implement after we've switched to GHCR.

@briansmith
Copy link
Contributor Author

There's nothing special about OpenSSL. There are lots of Rust crates that have external library dependencies; rust-openssl is just one of many. rust-openssl implemented a mechanism to install openssl when it isn't installed, at least for certain targets. I'm not sure what else is needed.

Further, this change will ultimately benefit rust-openssl users as cross having an old version of openssl installed would do nothing to help people who need OpenSSL 3.0, which will become increasingly common.

I suggest working with the rust-openssl project to better improve the experience.

@detly
Copy link

detly commented May 2, 2022

I have spent hours trying to get a statically-linked-with-musl, openssl-enabled Rust binary to build. Some of that time was due to #649, which I've added info to. But now I am stuck on building openssl. In this related issue, there's a remark:

if you're cross-compiling to MUSL, you'll probably need an OpenSSL cross-compiled for MUSL as well. If you're not cross-compiling, pkg-config will handle all of this configuration for you when using the normal Ubuntu OpenSSL install.

Is it actually possible to use cross to achieve a musl-linked openssl-enabled rust binary? If so, how?

spoorn added a commit to spoorn/RedditNotifier that referenced this issue Aug 29, 2022
Add devtool with build target
Add openssl as vendored dependency for cross-rs/cross#229
@Alexhuszagh
Copy link
Contributor

The rationale for this is now extensively explained on the wiki.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.