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

OpenSSL version range #16

Closed
lgrahl opened this issue Oct 21, 2016 · 6 comments
Closed

OpenSSL version range #16

lgrahl opened this issue Oct 21, 2016 · 6 comments

Comments

@lgrahl
Copy link
Contributor

lgrahl commented Oct 21, 2016

Obviously, OpenSSL 1.1.0 support is currently being implemented. But what is the minimum OpenSSL version we should support?

This is vital information for PRs such as #15 and an upcoming PR to set Diffie-Hellman parameters. I need to know which functions I can use.

@alfredh
Copy link
Contributor

alfredh commented Oct 21, 2016

we should support openssl 0.9.8 to 1.1.0, and libressl

@lgrahl
Copy link
Contributor Author

lgrahl commented Oct 21, 2016

Thanks for the quick reply. I think it would make sense adding this information to https://github.com/creytiv/re/blob/master/docs/README

@lgrahl
Copy link
Contributor Author

lgrahl commented Oct 21, 2016

As a follow-up question: Would you guys prefer OpenSSL version checks or is checking that a functions is defined fine for you as well? For example:

#if defined(SSL_CTX_set_ecdh_auto)
    ...
#else
    ...
#endif

or

#if OPENSSL_VERSION_NUMBER >= 0x1000200fL
    ...
#else
    ...
#endif

The reason why I'm asking is that I'm not sure if libressl mimics the OPENSSL_VERSION_NUMBER. And furthermore, it seems much easier to just check if the function is defined.

Move along, nothing to see here! 😰

@czarkoff
Copy link
Contributor

I am not a project member, so take my word with some grain of salt, but:

  1. Macro tests should be preferred over version checks, whenever available. Who knows what modifications people/distributions may apply to their OpenSSL packages? Some may even specifically mock OpenSSL versions for some arcane reasons.
  2. Preprocessor does not see function definitions, so test #if defined(function) should always evaluate to false. Try the following to see for yourself:
cat > t.c << EOF
#include <stdio.h>

int
main(void)
{
    printf("#ifdef printf: ");
#ifdef printf
    printf("true\n");
#else
    printf("false\n");
#endif

    printf("#if defined(printf): ");
#if defined(printf)
    printf("true\n");
#else
    printf("false\n");
#endif

    return 0;
}
EOF
cc -o t t.c
./t

@lgrahl
Copy link
Contributor Author

lgrahl commented Oct 21, 2016

Oh, you're right... I'm a bit embarrassed about that. The check above only works because SSL_CTX_set_ecdh_auto is indeed a macro, at least in OpenSSL. If I understand you correctly, checking for existence would normally be done during configuration which re doesn't have... so, although I agree with you on your first point... back to version checking for now?

@alfredh
Copy link
Contributor

alfredh commented Nov 2, 2016

can we close this now?

if a macro is available to show if a feature is present or not, it is good to use it.
otherwise using a version check is fine too.

@lgrahl lgrahl closed this as completed Nov 2, 2016
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

No branches or pull requests

3 participants