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

libressl 2.7 adds openbsd 1.1 API #319

Closed
gperciva opened this issue Aug 31, 2018 · 11 comments
Closed

libressl 2.7 adds openbsd 1.1 API #319

gperciva opened this issue Aug 31, 2018 · 11 comments

Comments

@gperciva
Copy link
Member

LibreSSL 2.7.0 (2018-03-21) adds the openssl 1.1 API:
https://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.7.0-relnotes.txt

This required a bunch of programs to update their OPENSSL_VERSION_NUMBER handling, such as libarchive:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=226853

As far as I know, libressl still supports the OpenSSL 1.0.1 API, so we don't need to change anything yet. But at some point, we should probably switch over.
(in lib/crypto/crypto_compat.c, we assume that any libressl is using the 1.0.1g API)

#ifdef LIBRESSL_VERSION_NUMBER
#undef OPENSSL_VERSION_NUMBER
#define OPENSSL_VERSION_NUMBER 0x1000107fL
#endif
@botovq
Copy link

botovq commented Nov 22, 2021

It wold be nice if this could be reconsidered. In LibreSSL 3.5 many types will be made opaque, including the RSA type. This will break the build of tarsnap. Users on OpenBSD-current will be affected by this before the end of the year. From the compat code that LibreSSL currently reaches, only the call to ERR_remove_thread_state(NULL) in crypto_compat_free() is still needed.

I was going to make a PR that removes the compat handling for LibreSSL (since no-one should be using a version prior to 2.7.0 anymore), but then I noticed the ERR_remove_thread_state() in the master branch... I'm not sure how you would prefer to handle that.

@gperciva
Copy link
Member Author

Thanks, I didn't know about the LibreSSL 3.5 issue. Yes, I need to move this up on my priority list.

On a related note, the OpenSSL 3.0 release on 2021-09-07 warns that the "low level" APIs are deprecated, so I've made a few forays into updating our code but nothing serious. I guess this is a "nice" new topic to delve into on a Monday morning!

(As for "no-one should be using libressl prior to 2.7.0"... we're very reluctant to break working code at tarsnap. What if somebody has an openbsd 5.9 system?)

For the record, I'll probably start the crypto compat updates with
Tarsnap/libcperciva#413

@gperciva
Copy link
Member Author

Fix appears in #508.

@botovq, does my commentary in #508 (comment) sound reasonable? I found that I had to call EVP_cleanup() with LibreSSL 2.7.0+ in order to clean up all memory.

@botovq
Copy link

botovq commented Dec 1, 2021 via email

@gperciva
Copy link
Member Author

gperciva commented Dec 1, 2021

There is definitely a lot of room for improvement in this regard, it's not near the top of our lists right now, though.

Fair enough. Thanks for checking that PR!

@gperciva gperciva closed this as completed Dec 1, 2021
@botovq
Copy link

botovq commented Sep 13, 2022

LibreSSL 3.6 will ship with OPENSSL_cleanup() which should help minimize the pain with using valgrind. We do not, however, install it as an atexit() function, applications wanting to use it need to do that themselves.

@gperciva
Copy link
Member Author

@botovq thanks! I've added #553 to remind me to use OPENSSL_cleanup() once libressl 3.6 is out.

@gperciva
Copy link
Member Author

gperciva commented Nov 22, 2022

@botovq: thanks for the recommendation about OPENSSL_cleanup()!

  1. is this officially documented anywhere? In libressl-3.6.1.tar.gz, I see

    $ grep OPENSSL_cleanup * -R
    ChangeLog:	  - Provide OPENSSL_cleanup().
    crypto/crypto_init.c:OPENSSL_cleanup(void)
    crypto/crypto.sym:OPENSSL_cleanup
    include/openssl/crypto.h:void OPENSSL_cleanup(void);
    

    but nothing in the man/ directory.

  2. my testing with valgrind 3.20.0 on FreeBSD 12.3 suggests that we need to call CRYPTO_cleanup_all_ex_data() as well. Is that expected?

    I mean, it's not the end of the world if we have to use:

    #if LIBRESSL_VERSION_NUMBER >= 0x3060000fL
            OPENSSL_cleanup();
            CRYPTO_cleanup_all_ex_data();
    #else   
    

    but that's not what I was expecting.

    (I'm quite willing to believe that I don't have a firm grasp of the difference between the OPENSSL_ and CRYPTO_ layers in libressl and openssl, though!)

@gperciva
Copy link
Member Author

@botovq to add to my confusion about not understanding the layers -- and for the benefit of anybody skimming this issue -- here's the implementation of OPENSSL_cleanup() in libressl-3.6.1.tar.gz:

void
OPENSSL_cleanup(void)
{
        /* This currently calls init... */
        ERR_free_strings();

        ENGINE_cleanup();
        EVP_cleanup();
        x509_issuer_cache_free();

        crypto_init_cleaned_up = 1;
}

so it's not like the OPENSSL_cleanup() exclusively cleans up OPENSSL_foo functions; it's doing ERR_ and ENGINE_ and EVP_ as well.

@botovq
Copy link

botovq commented Nov 22, 2022

Thanks for the report, that's our mistake, not yours.

  1. No, it's not yet documented, unfortunately.
  2. That's a bug, we'll look into fixing it.
  3. Yes, it should clean up everything that was allocated during the library's runtime, not only OPENSSL_foo things (of which there are very few).

@gperciva
Copy link
Member Author

Note to self: this was added in openbsd/src@3e826b6, and is present in libressl 3.7.1.

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

No branches or pull requests

2 participants