No way to reliably opt out of statically linking libcurl

[Shnatsel]

Following up in a new issue since #316 was erroneously closed, and I cannot reopen it

Right now curl-rust will silently fall back to statically linking the bundled version of libcurl, even if static-curl feature is not enabled.

This present a security issue: if you have configured the build to dynamically link to libcurl, it is reasonable to assume that updating the system-wide libcurl is sufficient to mitigate any outstanding CVEs. But in reality curl-rust may have silently fallen back to a bundled libcurl, which is still vulnerable.

There needs to be a way to opt out of statically linked libcurl entirely, so that if shared libcurl is not present the compilation would fail.

Also, the behavior should be clear from feature names; right now the fact that removing static-curl feature does not actually disable static linking of the bundled version is a major footgun from the security standpoint.

[ebarnard]

I've also experienced an issue with this feature. I am using curl to download files over FTP and was building the software on a new machine.

The downloads all failed with Protocol "ftp" not supported or disabled in libcurl.

It took me a long time to figure out that curl-rust was falling back to its bundled version which explicitly disables FTP.

@alexcrichton It seems from #316 that you are not keen on changing the default fallback but would you be happy with adding a no-static-curl-fallback feature?

[sagebind]

I think this would have to be implemented as a separate feature based on my understanding of how features in Cargo work. Features are additive; opting out of a feature is never sufficient to guarantee that something is disabled, because another dependency in the full tree may have requested the feature. Absence of a request is not the same as a request for absence.

You'd need a second feature in order to explicilty "disable" something. I'd be interested in prior art for this sort of "disabling feature" in other crates to see what it might look like and whether it has been successful.

[alexcrichton]

Honestly I'm unfortunately entirely lost in all the ways people want to link libcurl. Add this on top of https://github.com/alexcrichton/curl-rust/pull/319 and I just can't keep straight who wants what, and everything always seems like a minor addition over what's already there. I would prefer to have a section of documentation "how to link libcurl" which tries to cover all this, but without that I don't want to add any more bells and whistles because I feel like it's complicated enough as-is.

[ebarnard]

My issue is really that the bundled libcurl enables fewer protocols than those found in most Linux distros' package managers e.g. FTP.

This makes the behavior when building on a system without the libcurl-dev (or similar) package installed confusing. The crate will build but various calls will return protocol not supported, while curl --version will show the protocol is enabled. Maybe this is obvious to most people but it wasn't to me.

Were this project willing to make a breaking change I would want to put all protocols behind feature flags (with http and https enabled by default) and enable only those where the corresponding feature flag is enabled (via curl_easy_setopt(.., CURLINFO_PROTOCOL, ..)). This would give identical behavior when using both the bundled version and the system installed one. I would make curl::init check that the library being used supports all the protocols selected by feature flags and have it return an error detailing the protocols not supported. I realise that this can be done manually at the moment.

[sagebind]

I can't read Alex's mind, but I imagine that the curl bindings were created just for HTTP access, and no one really needed anything else from libcurl, so it was better to keep all the other unnecessary features disabled. Straight from the docs:

This crate contains bindings for an HTTP/HTTPS client which is powered by libcurl, the same library behind the curl command line tool.

So other protocols weren't really in scope. Now that this crate is getting more users that might need libcurl for other things, maybe we should re-evaluate the scope of the crate.

Were this project willing to make a breaking change I would want to put all protocols behind feature flags (with http and https enabled by default) and enable only those where the corresponding feature flag is enabled

I agree, this would be a great design to consider.

This makes the behavior when building on a system without the libcurl-dev (or similar) package installed confusing. The crate will build but various calls will return protocol not supported, while curl --version will show the protocol is enabled. Maybe this is obvious to most people but it wasn't to me.

Maybe better documentation on how and when static linking is performed might have helped here to avoid the confusion you encountered?

[alexcrichton]

Yes this was originally only for HTTP so that's all I bothered to implement, but adding support for more protocols behind features sounds great to me!

[roqvist]

I would like to use these bindings on Windows. Windows does not come with a Curl library out-of-the-box (although Windows 10 actually has the executable).

When I add the crate to my project, a bundled version is used. The bundled version for Windows is missing the NTLM feature that I'd like to use, so I compiled my own libcurl from source and tried linking it with a build.rs script but I can't get my project to prioritize my custom built version before the bundled version - is there any documentation on how to do this on Windows?

Edit: I figured out through one of the build scripts that the crate uses vcpkg to identify whether libcurl is installed or not on Windows, so I installed vcpkg and through it added libcurl. Cleaned the project and rebuilt, still falling back to static bundled. 😞

C:\>vcpkg_cli probe -l dll curl
Found library curl
Include paths:
  C:\Src\Github\vcpkg\installed\x64-windows\include
Library paths:
  C:\Src\Github\vcpkg\installed\x64-windows\lib
Runtime Library paths:
  C:\Src\Github\vcpkg\installed\x64-windows\bin
Cargo metadata:
  cargo:rustc-link-search=native=C:\Src\Github\vcpkg\installed\x64-windows\lib
  cargo:rustc-link-search=native=C:\Src\Github\vcpkg\installed\x64-windows\bin
  cargo:rustc-link-lib=libcurl
  cargo:rustc-link-lib=zlib
Found DLLs:
  C:\Src\Github\vcpkg\installed\x64-windows\bin\libcurl.dll
  C:\Src\Github\vcpkg\installed\x64-windows\bin\zlib1.dll
Found libs:
  C:\Src\Github\vcpkg\installed\x64-windows\lib\libcurl.lib
  C:\Src\Github\vcpkg\installed\x64-windows\lib\zlib.lib
Libraries linking names:
  libcurl
  zlib

C:\>vcpkg list
curl:x64-windows                                   7.68.0-1         A library for transferring data with URLs
curl:x86-windows                                   7.68.0-1         A library for transferring data with URLs
curl[non-http]:x64-windows                                          Enables protocols beyond HTTP/HTTPS/HTTP2
curl[non-http]:x86-windows                                          Enables protocols beyond HTTP/HTTPS/HTTP2
curl[ssl]:x64-windows                                               Default SSL backend
curl[ssl]:x86-windows                                               Default SSL backend
curl[winssl]:x64-windows                                            SSL support (Secure Channel / "WinSSL")
curl[winssl]:x86-windows                                            SSL support (Secure Channel / "WinSSL")
zlib:x64-windows                                   1.2.11-6         A compression library
zlib:x86-windows                                   1.2.11-6         A compression library

[dbregman]

@alexcrichton @roqvist

I discovered that in order for a vcpkg version of curl to be picked up by the current build script, the environment variable VCPKGRS_DYNAMIC must be set. I didn't yet figure out how to set this from cargo, but setting it system wide did the trick for me. Perhaps this variable should be set by the build script and/or configured through features.

[thomcc]

I would like to use these bindings on Windows. Windows does not come with a Curl library out-of-the-box (although Windows 10 actually has the executable).

What rusqlite (well libsqlite3-sys) does is have a separate feature you can turn on for "use vendored but only if on windows".

Honestly the build feature bloat growth over time is a huge hassle. I don't blame @alexcrichton for wanting to avoid it, it gets very complex for crates that have widely used native libraries.

I wish there were a better way to do it, features feels wrong for something like this... The settings are target-specific and also really only the business of the person publishing the application mostly. (I've even considered adding one final feature that would let users pass an environment variable containing a toml. file with explicit settings rather than use more features but this is probably just overboard).

All that said I did come here to find out if curl had an equivalent "use vendored but only if on windows" feature -- and it looks like the default settings behave that way (...which is what this bug is complaining about).

[sagebind]

The right way to toggle static builds on different targets for downstream crates will probably be with using target-specific feature sets, which is currently in development: https://github.com/rust-lang/cargo/issues/7914. This doesn't affect what curl-sys's default behavior is, but changing the default behavior is a difficult sell without a clear advantage for everyone.