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

Projects
None yet
3 participants
@p
Copy link

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented May 23, 2018

@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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented May 23, 2018

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

This comment has been minimized.

Copy link
Member

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

configure: add basic test of --with-ssl prefix
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

This comment has been minimized.

Copy link
Contributor

commented May 23, 2018

That'll work splendidly, @bagder

@p

This comment has been minimized.

Copy link
Author

commented May 23, 2018

Seems reasonable, thanks!

@bagder

This comment has been minimized.

Copy link
Member

commented May 23, 2018

Thanks both of you!

@bagder bagder closed this 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.
You can’t perform that action at this time.