Your original remark said - there is still a race here with other thread(s) un-initializing OpenSSL (*talking about un-initializing*) Now you are saying - that is not what I meant by my remark -- SSL-library-init functions are not threadsafe, afaik. (*talking about initializing*)
My response was for the former. The original problem I had was that grpc would internally trounce my settings and I can not tell everyone else in my setting to wait for some unknown time when grpc would trounce those settings in some internal thread that I don't know about and only use OpenSSL after it. Now, it's an easier problem that I just need to make sure the OpenSSL is ready to use before I call grpc_init. As grpc does not do any shutdown on OpenSSL, I can ensure that my application is only cleaning it up after I am done with grpc. As for your latest remark that SSL-library-init functions are not threadsafe, they could also be wrapped inside the callback check. I have not done any research on them being not thread-safe so if you have reference literature on it, that'd be good to see. Still as the calls are idempotent, this thread synchronization is an easy problem. On Tue, Apr 24, 2018 at 5:45 PM, Michael Kilburn <[email protected]> wrote: > Well, there is no official way to fully uninitialize OpenSSL. Also, that > is not what I meant by my remark -- SSL-library-init functions are not > threadsafe, afaik. I.e. calling them from different threads at the same > time is unsafe. And it is not easy to guarantee that this won't happen if > grpc is doing it under the covers. > > On Mon, Apr 23, 2018 at 10:43 PM, Arpit Baldeva <[email protected]> > wrote: > >> Grpc does not un-initialize OpenSSL. If you have other thread that is >> un-initing it, you can easily coordinate that after grpc shutdown. The >> patch is only ensuring that it is not overriding application settings. >> While the cleanest solution is to provide an explicit flag, the solution in >> the patch works as long as the user knows what’s happening behind the >> scenes. >> >> Sent from my iPhone >> >> On Apr 23, 2018, at 8:07 PM, 'Jiangtao Li' via grpc.io < >> [email protected]> wrote: >> >> Initializing OpenSSL outside gRPC is not practical in many situations. >> Our internal expert says the race condition only exist for OpenSSL 1.0.x >> and 0.9.x. We are on best effort to support older versions of OpenSSL, >> while our main support is BoringSSL and OpenSSL 1.1. >> >> Thanks, >> Jiangtao >> >> >> On Mon, Apr 23, 2018 at 7:30 PM CM <[email protected]> wrote: >> >>> This change didn't really fix the problem -- there is still a race here >>> with other thread(s) un-initializing OpenSSL. Old problem, the only correct >>> solution is to leave OpenSSL initialization to user. >>> >>> On Monday, April 23, 2018 at 4:18:42 PM UTC-5, Jiangtao Li wrote: >>>> >>>> Good to know. Once the patch approved and merged. It will be in next >>>> grpc release. >>>> >>>> >>>> Thanks, >>>> Jiangtao >>>> >>>> >>>> On Mon, Apr 23, 2018 at 2:10 PM Arpit Baldeva <[email protected]> >>>> wrote: >>>> >>>>> Thanks for the patch. It did fix the issue! >>>>> >>>>> What release version are you targeting? >>>>> >>>>> Thanks. >>>>> >>>>> On Friday, April 20, 2018 at 2:12:05 PM UTC-7, [email protected] >>>>> wrote: >>>>>> >>>>>> Arpit, >>>>>> >>>>>> Could you please patch https://github.com/grpc/grpc/pull/15143 and >>>>>> see whether it solves your problem. >>>>>> >>>>>> On Friday, April 20, 2018 at 9:30:34 AM UTC-7, Arpit Baldeva wrote: >>>>>>> >>>>>>> I am on 1.0.2k so yeah it is a problem on that version. >>>>>>> >>>>>>> I think the simplest fix is what I mentioned in last email - grpc >>>>>>> init_openssl implementation can check if locking callback already >>>>>>> exists by >>>>>>> calling CRYPTO_get_id_callback and if so, skip putting it's own locks. >>>>>>> The >>>>>>> application can ensure that it has the right initialization in place >>>>>>> before >>>>>>> doing anything with grpc. >>>>>>> >>>>>>> Thanks. >>>>>>> >>>>>>> >>>>>>> On Thursday, April 19, 2018 at 10:58:21 AM UTC-7, Jiangtao Li wrote: >>>>>>>> >>>>>>>> Which version of OpenSSL are you using? Or you are using BoringSSL? >>>>>>>> OpenSSL 1.1 or BoringSSL does not have such problems on OpenSSL init. >>>>>>>> >>>>>>>> For OpenSSL 1.0x, it is a valid concern. Let me check what is the >>>>>>>> best way to resolve this issue (pass a compiler flag, environment >>>>>>>> variable, >>>>>>>> or some API changes). >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Jiangtao >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Apr 18, 2018 at 7:34 PM Arpit Baldeva <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> To explain further, a library such as grpc intializing global >>>>>>>>> settings of another library is not a good practice. Looking at >>>>>>>>> libcurl ( >>>>>>>>> https://curl.haxx.se/libcurl/c/curl_global_init.html) , >>>>>>>>> PostgreSQL(https://www.postgresql.org/docs/9.1/static/libpq- >>>>>>>>> ssl.html ) (and may be there are more libraries), these take an >>>>>>>>> initialization flag that to decide whether OpenSSL should be >>>>>>>>> initialized >>>>>>>>> internally or not. >>>>>>>>> >>>>>>>>> Actually, looking at https://stackoverflow.com/ques >>>>>>>>> tions/21874152/openssl-thread-safety-callback-function- >>>>>>>>> registration-with-both-direct-call-and-i , the simplest solution >>>>>>>>> here is for grpc to check if locking callback already exists by >>>>>>>>> calling >>>>>>>>> CRYPTO_get_id_callback and if so, skip putting it's own locks. Rest >>>>>>>>> of the >>>>>>>>> init functions in OpenSSL are idempotent and hence can be called >>>>>>>>> multiple >>>>>>>>> times. And looks like grpc is not cleaning up OpenSSL so we are fine >>>>>>>>> there >>>>>>>>> too (looks like clean up calls are getting no-op in future - >>>>>>>>> https://stackoverflow.com/questions/35802643/will-ignoring- >>>>>>>>> to-call-openssl-evp-cleanup-result-in-serious-flaws-or-memory-leak >>>>>>>>> ) . >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wednesday, April 18, 2018 at 4:09:01 PM UTC-7, Arpit Baldeva >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> Again, I am not sure why you are focusing on the race condition. >>>>>>>>>> Race condition is not the problem. Fact that grpc decides to put some >>>>>>>>>> global callbacks on the OpenSSL is the problem. It should not do >>>>>>>>>> that. Why >>>>>>>>>> can't this be made optional via a compile time flag or some run time >>>>>>>>>> initialization parameter? >>>>>>>>>> >>>>>>>>>> On Wednesday, April 18, 2018 at 3:52:47 PM UTC-7, Jiangtao Li >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> I don't see a convenient way to do in gRPC. SSL init can be >>>>>>>>>>> called multiple times, as long as it is not called in the same >>>>>>>>>>> time. It >>>>>>>>>>> seems that the best way to address is adding mutex in your >>>>>>>>>>> application to >>>>>>>>>>> make sure SSL init is not called simultaneously. >>>>>>>>>>> >>>>>>>>>>> On Wed, Apr 18, 2018 at 3:28 PM Arpit Baldeva <[email protected]> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Yes, there are two parallel threads that do this at the same >>>>>>>>>>>> time. What I was noticing is that at application shutdown, my >>>>>>>>>>>> application >>>>>>>>>>>> complained about the lock I handed over to OpenSSL not being >>>>>>>>>>>> unlocked. >>>>>>>>>>>> Looking into it, I realized that during initialization, OpenSSL >>>>>>>>>>>> grabbed a >>>>>>>>>>>> lock from my callback, during this time, grpc replaced the locks >>>>>>>>>>>> and >>>>>>>>>>>> OpenSSL unlock call got re-routed. But again, this is just a side >>>>>>>>>>>> effect. >>>>>>>>>>>> The real problem is simply the fact that grpc is setting some >>>>>>>>>>>> global >>>>>>>>>>>> callbacks on OpenSSL that should best be left to the application. I >>>>>>>>>>>> understand that this is probably done to make it easy for the grpc >>>>>>>>>>>> integrators but there are applications such as mine where there >>>>>>>>>>>> are other >>>>>>>>>>>> usage of OpenSSL and I'd rather manage the global settings myself >>>>>>>>>>>> than rely >>>>>>>>>>>> on grpc to do it. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Wednesday, April 18, 2018 at 3:05:05 PM UTC-7, >>>>>>>>>>>> [email protected] wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Could you describe the race condition in details? >>>>>>>>>>>>> >>>>>>>>>>>>> In gRPC ssl_transport_security, init_openssl() is only called >>>>>>>>>>>>> when ssl transport security is needed. In your application, you >>>>>>>>>>>>> are >>>>>>>>>>>>> calling SSL_library_init() in a separate thread in parallel? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Wednesday, April 18, 2018 at 2:46:28 PM UTC-7, Arpit >>>>>>>>>>>>> Baldeva wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> I am using grpc-1.10.0 and it has that code. Looking at the >>>>>>>>>>>>>> latest master, it still has that code - >>>>>>>>>>>>>> https://github.com/grpc/grpc/blob/master/src/core/tsi/ssl_ >>>>>>>>>>>>>> transport_security.cc - see the init_openssl function. >>>>>>>>>>>>>> >>>>>>>>>>>>>> The problem is not just that grpc_init initialized the >>>>>>>>>>>>>> OpenSSL. The problem is really that grpc is initializing OpenSSL >>>>>>>>>>>>>> internally >>>>>>>>>>>>>> and it can trounce the application settings. Grpc should let >>>>>>>>>>>>>> application >>>>>>>>>>>>>> choose if they want it to handle the OpenSSL initialization or >>>>>>>>>>>>>> not. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Wednesday, April 18, 2018 at 12:25:58 PM UTC-7, >>>>>>>>>>>>>> [email protected] wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi Arpit, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> grpc_init initializes OpenSSL for a short period (~2 days) >>>>>>>>>>>>>>> and the code was later removed. Do you still the problem, if >>>>>>>>>>>>>>> you fetch the >>>>>>>>>>>>>>> latest master? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Monday, April 16, 2018 at 2:22:32 PM UTC-7, Arpit Baldeva >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I recently pinned down a sporadic race condition in my >>>>>>>>>>>>>>>> application due to grpc intializing OpenSSL internally. The >>>>>>>>>>>>>>>> problem is that >>>>>>>>>>>>>>>> OpenSSL has some global callbacks that grpc is trying to >>>>>>>>>>>>>>>> initialize on it's >>>>>>>>>>>>>>>> own without the authorization of the application. The problem >>>>>>>>>>>>>>>> is in the >>>>>>>>>>>>>>>> init_openssl call in ssl_transport_security.cc which >>>>>>>>>>>>>>>> trounces similar calls from the application. The situation is >>>>>>>>>>>>>>>> made >>>>>>>>>>>>>>>> worse(race condition) if some application threads already have >>>>>>>>>>>>>>>> locks >>>>>>>>>>>>>>>> acquired via previous calls and then grpc changes the stuff >>>>>>>>>>>>>>>> underneath >>>>>>>>>>>>>>>> before the locks are released (hence the race condition). >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> My application has usage of OpenSSL in a wider context than >>>>>>>>>>>>>>>> just grpc. I get the point that grpc is wrapping this up to >>>>>>>>>>>>>>>> make it easier >>>>>>>>>>>>>>>> for standing up an application with grpc but I think such type >>>>>>>>>>>>>>>> of things >>>>>>>>>>>>>>>> should be accompanied by a compile/run time option supplied at >>>>>>>>>>>>>>>> grpc_init >>>>>>>>>>>>>>>> that let's application decide the right owner. I am fine with >>>>>>>>>>>>>>>> the option >>>>>>>>>>>>>>>> defaulting to grpc initializing the OpenSSL internally. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thoughts? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thanks. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> -- >>>>>>>>>>>> You received this message because you are subscribed to a topic >>>>>>>>>>>> in the Google Groups "grpc.io" group. >>>>>>>>>>>> To unsubscribe from this topic, visit >>>>>>>>>>>> https://groups.google.com/d/topic/grpc-io/Z48vpXRnJaw/unsubs >>>>>>>>>>>> cribe. >>>>>>>>>>>> To unsubscribe from this group and all its topics, send an >>>>>>>>>>>> email to [email protected]. >>>>>>>>>>>> To post to this group, send email to [email protected]. >>>>>>>>>>>> Visit this group at https://groups.google.com/group/grpc-io. >>>>>>>>>>>> To view this discussion on the web visit >>>>>>>>>>>> https://groups.google.com/d/msgid/grpc-io/3df13436-d771-4450 >>>>>>>>>>>> -85c3-d2683b08e976%40googlegroups.com >>>>>>>>>>>> <https://groups.google.com/d/msgid/grpc-io/3df13436-d771-4450-85c3-d2683b08e976%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>>>> . >>>>>>>>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>> You received this message because you are subscribed to a topic in >>>>>>>>> the Google Groups "grpc.io" group. >>>>>>>>> To unsubscribe from this topic, visit >>>>>>>>> https://groups.google.com/d/topic/grpc-io/Z48vpXRnJaw/unsubscribe. >>>>>>>>> To unsubscribe from this group and all its topics, send an email >>>>>>>>> to [email protected]. >>>>>>>>> To post to this group, send email to [email protected]. >>>>>>>>> Visit this group at https://groups.google.com/group/grpc-io. >>>>>>>>> To view this discussion on the web visit >>>>>>>>> https://groups.google.com/d/msgid/grpc-io/8821745f-9b23-4e3e >>>>>>>>> -b4cf-b88bce513b4b%40googlegroups.com >>>>>>>>> <https://groups.google.com/d/msgid/grpc-io/8821745f-9b23-4e3e-b4cf-b88bce513b4b%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>> . >>>>>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>>>>> >>>>>>>> -- >>>>> You received this message because you are subscribed to a topic in the >>>>> Google Groups "grpc.io" group. >>>>> To unsubscribe from this topic, visit https://groups.google.com/d/to >>>>> pic/grpc-io/Z48vpXRnJaw/unsubscribe. >>>>> To unsubscribe from this group and all its topics, send an email to >>>>> [email protected]. >>>>> To post to this group, send email to [email protected]. >>>>> Visit this group at https://groups.google.com/group/grpc-io. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/d/msgid/grpc-io/d178eff0-8225-4a0b >>>>> -b63f-ac15afe425e3%40googlegroups.com >>>>> <https://groups.google.com/d/msgid/grpc-io/d178eff0-8225-4a0b-b63f-ac15afe425e3%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> -- >>> You received this message because you are subscribed to a topic in the >>> Google Groups "grpc.io" group. >>> To unsubscribe from this topic, visit https://groups.google.com/d/to >>> pic/grpc-io/Z48vpXRnJaw/unsubscribe. >>> To unsubscribe from this group and all its topics, send an email to >>> [email protected]. >>> To post to this group, send email to [email protected]. >>> Visit this group at https://groups.google.com/group/grpc-io. >>> To view this discussion on the web visit https://groups.google.com/d/ms >>> gid/grpc-io/221ffdb6-aa2c-457d-8772-32ace46173dc%40googlegroups.com >>> <https://groups.google.com/d/msgid/grpc-io/221ffdb6-aa2c-457d-8772-32ace46173dc%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "grpc.io" group. >> To unsubscribe from this topic, visit https://groups.google.com/d/to >> pic/grpc-io/Z48vpXRnJaw/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> [email protected]. >> To post to this group, send email to [email protected]. >> Visit this group at https://groups.google.com/group/grpc-io. >> To view this discussion on the web visit https://groups.google.com/d/ms >> gid/grpc-io/CACcm8_i7f6HdGG3G72p_FN2UqwyUzaDyDVKGE3oNP1Xbi9G >> SwA%40mail.gmail.com >> <https://groups.google.com/d/msgid/grpc-io/CACcm8_i7f6HdGG3G72p_FN2UqwyUzaDyDVKGE3oNP1Xbi9GSwA%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> For more options, visit https://groups.google.com/d/optout. >> >> > > > -- > Sincerely yours, > Michael. > -- You received this message because you are subscribed to the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/grpc-io. To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CAL2p%3D0kcYUXPd-Qce1bOLBC6P%3DvfQK2kaYn7j7hD4AFivzp%3Dnw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
