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

Add SSL_CTX_set1_cert_store #71

Closed
wahern opened this issue Oct 20, 2016 · 3 comments
Closed

Add SSL_CTX_set1_cert_store #71

wahern opened this issue Oct 20, 2016 · 3 comments

Comments

@wahern
Copy link

wahern commented Oct 20, 2016

SSL_CTX_set_cert_store does not increment the reference count of the store object. This a trap for the unwary as OpenSSL has been inconsistent in this regard. Some "set_foo" routines will increment the reference, others won't, and my habit has always been to examine the implementations. Newer APIs use the set0_foo and set1_foo, which is a much nicer contract that allows me to relax a little more.

I've been using a fake SSL_CTX_set1_cert_store macro in my projects for a couple of years now which only incremented the reference if SSL_CTX_set_cert_store failed to. (I was afraid somebody might "fix" the routine by incrementing the reference count, causing a leak in my wrapper.) But now that OpenSSL objects are going opaque I can't test the reference count anymore.

OpenSSL is also adding SSL_CTX_set1_cert_store:
openssl/openssl#1755
openssl/openssl#1734

@daurnimator
Copy link

upstream openssl has merged now (in openssl/openssl@b50052d)

@busterb busterb closed this as completed Jun 4, 2017
@daurnimator
Copy link

@busterb was this fixed?

@busterb
Copy link

busterb commented Jun 4, 2017

oh, pardon, I misread the link

@busterb busterb reopened this Jun 4, 2017
nak3 added a commit to nak3/openbsd that referenced this issue Jul 27, 2024
As reported at libressl#71, current
users must call X509_STORE_up_ref() when they use
SSL_CTX_set_cert_store().

This patch adds SSL_CTX_set1_cert_store(), which updates the reference
count as implied by "set1".
@botovq botovq closed this as completed in 795f6cc Aug 3, 2024
bob-beck pushed a commit to openbsd/src that referenced this issue Aug 3, 2024
SSL_CTX_set_cert_store() should have been called SSL_CTX_set0_cert_store()
since it takes ownership of the store argument. Apparently a few people ran
into the issue of not bumping the refcount themselves, leading to use after
frees about 10 years ago. This is a quite rarely used API and there are no
misuses in the ports tree, but since someone did the work of writing a diff,
we can still add it.

Needless to say that SSL_CTX_get_cert_store() obviously has the exact same
issue and nobody seems to have thought of adding a get0 or get1 version to
match...

Fixes libressl/openbsd#71
From Kenjiro Nakayama
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants