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] > <javascript:>> 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/questions/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/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/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] <javascript:>. >> To post to this group, send email to [email protected] >> <javascript:>. >> 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 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/72050a5c-b0a5-4e8a-ae2a-ebbc135d8b30%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
