Assigning to a boost::asio::ssl::context while asynchronous operations are using it
I cannot find any definitive documentation on whether it's safe to move-assign to an ssl-context while it is used for asynchronous operations, assuming of course the application is single-threaded. In my tests I could not trigger any failures, but that may be pure luck (or not lucky at all depending on point of view).
move-assignment from what to where ?
The ssl context is context which tuned for incoming connections (if
its server side).
You can to have any number of ssl contexts for it (with ordered logic
of that work),
but I have just the one.
Without a code is hard to suggest what you doing, but the ssl context
is not for moving,
I have been storing it in server class, initialize it on start_server
method, and put it into session_start
for make connection with handshake etc (the ssl context forwarded for
each connection as reference).
If you need more detailed answer, you could to give more details of your case.
2019-09-25 20:32 GMT+05:00, Martijn Otto via Boost-users
I cannot find any definitive documentation on whether it's safe to move-assign to an ssl-context while it is used for asynchronous operations, assuming of course the application is single-threaded.
In my tests I could not trigger any failures, but that may be pure luck (or not lucky at all depending on point of view).
On Thu, 2019-09-26 at 04:27 +0500, Dmitrij V via Boost-users wrote:
move-assignment from what to where ?
The ssl context is context which tuned for incoming connections (if its server side). You can to have any number of ssl contexts for it (with ordered logic of that work), but I have just the one.
Without a code is hard to suggest what you doing, but the ssl context is not for moving, I have been storing it in server class, initialize it on start_server method, and put it into session_start for make connection with handshake etc (the ssl context forwarded for each connection as reference).
If you need more detailed answer, you could to give more details of your case.
2019-09-25 20:32 GMT+05:00, Martijn Otto via Boost-users
: I cannot find any definitive documentation on whether it's safe to move-assign to an ssl-context while it is used for asynchronous operations, assuming of course the application is single-threaded.
In my tests I could not trigger any failures, but that may be pure luck (or not lucky at all depending on point of view).
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
OK, I'll do my best to explain it as clearly as I can. In my main function I define an instance of an ssl context. This is then provided by lvalue reference to a 'server' class, which uses it to do a TLS handshake on each incoming connection. Now I want to be able to load a new certificate without restarting the process (minimizing downtime). For this I have implemented a signal handler listening to SIGUSR1. This handler constructs a new ssl context and then move-assigns it to the ssl context variable in the main function. Since the server class has a reference to this same object, it's also going to see the changes performed from the signal handler. The code is fully async, so there are no race-conditions in the classic sense of the word. The server may, however, be somewhere in the middle of performing a TLS handshake. I know that _after_ the handshake is complete, the ssl context is no longer relevant, but I can imagine this could break if the ssl context is move-assigned to when an asynchronous tls handshake is in progress. As I said, I could not manage to get this to fail in my tests, but conceptually I can imagine this is wrong. I'd like to be sure about this. If what I am doing is indeed not correct, I will need to use a shared_ptr so the handshakes can complete with the old context. Since this introduces overhead I'd like to do this only if really necessary.
Yes, you have no problem for now just by pure luck. In the destructor of the context freeing all related to SSL resources. How you can avoid the problems in future: as an one of soltions, in your loop thread (if you have it, like me for example): 1) io_.stop(); 2) while (io_.poll()) {} 3) HERE your code to assign the context (perhaps you will need to rehandshake alived sessions) 4) io_.restart(); 5) acceptor_.async_accept(...); PS: In my past project I had very likely approach - restart server without stopping the process app. I had reject all my beginnings and forced clients to reconnect by losed the connections (there are many bytes would needs to store in shared memory (related to session data)) -- regards
I have wrote: ...without stopping the process app... Sorry for my mistake: I had target: replacing the image of app with execve call (with needs to save session's data in shared memory) - that would was likely smart updater :)...
On Fri, 2019-09-27 at 06:30 +0500, Dmitrij V via Boost-users wrote:
Yes, you have no problem for now just by pure luck. In the destructor of the context freeing all related to SSL resources.
How you can avoid the problems in future:
as an one of soltions, in your loop thread (if you have it, like me for example):
1) io_.stop(); 2) while (io_.poll()) {} 3) HERE your code to assign the context (perhaps you will need to rehandshake alived sessions) 4) io_.restart(); 5) acceptor_.async_accept(...);
PS: In my past project I had very likely approach - restart server without stopping the process app. I had reject all my beginnings and forced clients to reconnect by losed the connections (there are many bytes would needs to store in shared memory (related to session data))
-- regards _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
It makes sense for the destructor to indeed clean up resources. That's what it's for. However, looking at the OpenSSL documentation, there are three relevant functions: SSL_CTX_new(), creating an SSL_CTX instance with refcount 1 SSL_CTX_up_ref(), increasing the refcount SSL_CTX_free(), decreasing the refcount, freeing when reaching 0 Now, given that OpenSSL uses a reference-counted system for their TLS contexts, it would make sense if boost::asio were to increment the reference count when starting the handshake, and decrement it when this is done, which could easily make what I'm doing safe. However, looking at the source, it appears that's not what it's doing. I believe it wouldn't be much work to rectify this. In ssl/stream.hpp, we are already taking the underlying SSL_CTX object as a member and store it in core_. I think it'd be trivial to add an SSL_CTX_up_ref() to the constructor and an SSL_CTX_free() to the destructor. If we do this, we avoid potential problems, like the context being freed while the stream is active. I could make a pull-request to implement these changes, if so desired. Does this have any change of getting merged?
Martijn Otto wrote:
I could make a pull-request to implement these changes, if so desired.
+1, changes + documentation on its, please :)
Does this have any change of getting merged?
Oh, that is not in my authority, but please, send the PR into https://github.com/chriskohlhoff/asio too -- regards
On Sat, 2019-09-28 at 07:31 +0500, Dmitrij V via Boost-users wrote:
Martijn Otto wrote:
I could make a pull-request to implement these changes, if so desired.
+1, changes + documentation on its, please :)
Does this have any change of getting merged?
Oh, that is not in my authority, but please, send the PR into https://github.com/chriskohlhoff/asio too
-- regards _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
After reading through the source code some more, it seems that no changes are actually necessary, since my initial code was already safe. I'll explain why. The ssl::stream constructor, which does get a reference to the ssl::context and extracts the underlying SSL_CTX but does not keep this in a member. Instead, it forwards it to its core_ member, which is an instance of detail::stream_core. detail::stream_core then forwards it to its engine_ member, which is an instance of detail::engine. detail::engine then uses it to call SSL_new. The documentation for SSL_new does not explicitly mention it, but looking through the code I can see it call SSL_CTX_up_ref(), which increases the reference count on the underlying SSL_CTX, which means that even if the ssl_context calls SSL_CTX_free(), the context will remain valid if there is an ssl::stream using it.
Dear Martijn Otto,
Please, if it is not hard for you: make PR with documentation the
behaviour for the context.
--
regards
2019-09-29 22:11 GMT+05:00, Martijn Otto via Boost-users
On Sat, 2019-09-28 at 07:31 +0500, Dmitrij V via Boost-users wrote:
Martijn Otto wrote:
I could make a pull-request to implement these changes, if so desired.
+1, changes + documentation on its, please :)
Does this have any change of getting merged?
Oh, that is not in my authority, but please, send the PR into https://github.com/chriskohlhoff/asio too
-- regards _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
After reading through the source code some more, it seems that no changes are actually necessary, since my initial code was already safe. I'll explain why.
The ssl::stream constructor, which does get a reference to the ssl::context and extracts the underlying SSL_CTX but does not keep this in a member. Instead, it forwards it to its core_ member, which is an instance of detail::stream_core. detail::stream_core then forwards it to its engine_ member, which is an instance of detail::engine.
detail::engine then uses it to call SSL_new. The documentation for SSL_new does not explicitly mention it, but looking through the code I can see it call SSL_CTX_up_ref(), which increases the reference count on the underlying SSL_CTX, which means that even if the ssl_context calls SSL_CTX_free(), the context will remain valid if there is an ssl::stream using it.
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
On Wed, Sep 25, 2019 at 11:53 PM Martijn Otto via Boost-users
If what I am doing is indeed not correct, I will need to use a shared_ptr so the handshakes can complete with the old context. Since this introduces overhead I'd like to do this only if really necessary.
If you are using SSL then you should not be concerned about the overhead of using shared_ptr, it is not statistically significant compared to the kernel I/O and SSL operations. Regards
On 9/26/19 8:52 AM, Martijn Otto via Boost-users wrote:
Now I want to be able to load a new certificate without restarting the process (minimizing downtime). For this I have implemented a signal handler listening to SIGUSR1. This handler constructs a new ssl context and then move-assigns it to the ssl context variable in the main function.
Please notice that only syscalls marked as async-signal-safe may be used within a signal handler. Memory allocation is not async-signal-safe.
On Sun, 2019-09-29 at 12:57 +0200, Bjorn Reese via Boost-users wrote:
On 9/26/19 8:52 AM, Martijn Otto via Boost-users wrote:
Now I want to be able to load a new certificate without restarting the process (minimizing downtime). For this I have implemented a signal handler listening to SIGUSR1. This handler constructs a new ssl context and then move-assigns it to the ssl context variable in the main function.
Please notice that only syscalls marked as async-signal-safe may be used within a signal handler. Memory allocation is not async-signal-safe. _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
That's interesting and good to know. I cannot find it anywhere in the documentation for signal_set ( https://www.boost.org/doc/libs/1_71_0/doc/html/boost_asio/reference/signal_s... ). My understanding has always been that this issue does not occur using asio, since it will post() to the executor when a signal is received, so the possibly unsafe code is not executed in the signal handler the kernel sees. Have I misunderstood this?
participants (4)
-
Bjorn Reese
-
Dmitrij V
-
Martijn Otto
-
Vinnie Falco