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

--with-ssl=/nonexistent/path ignores specified path and builds against another SSL implementation #2580

Closed
p opened this issue May 17, 2018 · 8 comments

Comments

@p
Copy link
Contributor

p commented May 17, 2018

I did this

Attempted to build libcurl against libressl by specifying a path thusly:

./configure --with-ssl=/path/to/libressl ...

However, I neglected to build libressl prior to doing this hence /path/to/libressl didn't exist.

I expected the following

An error telling me I gave a nonexistent path to --with-ssl.

Ideally if the directory exists but doesn't contain openssl headers/libraries this should also be an error but maybe this is asking too much.

Actual result

libcurl built itself against system openssl which is an entirely different ssl implementation without any errors.

Further background

There are two mentions of /path/to/libressl in configure output:

configure: PKG_CONFIG_LIBDIR will be set to "/path/to/libressl/lib/pkgconfig"
configure: Added /path/to/libressl/lib to CURL_LIBRARY_PATH

So, my specified path is simply added (prepended I imagine) to search directories.

curl/libcurl version

7.60.0 from source/release tarball

operating system

Ubuntu Trusty VM

Note

This issue may well affect other --with-* options, I added a generic guard for directory existence to my build process now.

@bagder
Copy link
Member

bagder commented May 17, 2018

I'm pretty sure this happens because the test for the ssl library still works because it finds a system one installed and uses that (in one of the default paths). So while a check for the dir existing would catch your specific case if you truly point to a missing directory, it would still work like this if you point to an empty directory...

@PerMalmberg
Copy link
Contributor

@p You're not alone - we had a spelling error in the path; we ended up building against a very dated openssl. It took us a while to figure out where it went wrong.

@bagder
Copy link
Member

bagder commented May 23, 2018

I can't think of any easy ways to detect this. The linker has a default path that might make it find another library if the given path isn't working. To detect that, we have to add some custom logic that verifies the given path to make sure it actually contains a (working) openssl installation and that does feel very error-prone.

Or do you have any ideas on how this process could be improved?

@PerMalmberg
Copy link
Contributor

Well, do we need to know it is a working installation in the specified directory? Wouldn't it be enough that we find at least the "entry header file" in the directory? If the user points to a broken installation, that'll likely become obvious at a later stage when it doesn't compile or otherwise breaks.

Other than that, I can only think of of documenting the possible pitfall.

@bagder
Copy link
Member

bagder commented May 23, 2018

I like that. Simple and easy. Basically logic like this:

if ! test -f "$givenpath/include/openssl/ssl.h"; then
   echo "probably the wrong path, run away"
fi

I'll put together a PR for it...

bagder added a commit that referenced this issue May 23, 2018
When given a prefix, the $PREFIX_OPENSSL/lib/openssl.pc or
$PREFIX_OPENSSL/include/openssl/ssl.h files must be present or cause an
error. Helps users detect when giving configure the wrong path.

Reported-by: Oleg Pudeyev
Assisted-by: Per Malmberg
Fixes #2580
@PerMalmberg
Copy link
Contributor

That'll work splendidly, @bagder

@p
Copy link
Contributor Author

p commented May 23, 2018

Seems reasonable, thanks!

@bagder
Copy link
Member

bagder commented May 23, 2018

Thanks both of you!

@bagder bagder closed this as completed in d353af0 May 23, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants