Use cached grpc ssl credential for ObjC library #18336
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
muxi
added
lang/ObjC
release notes: yes
include/grpc/grpc_security.h
Outdated
| @@ -191,6 +191,12 @@ typedef struct { | |||
| try to get the roots set by grpc_override_ssl_default_roots. Eventually, | |||
| if all these fail, it will try to get the roots from a well-known place on | |||
| disk (in the grpc install directory). | |||
|
|
|||
| It is a known issue that when the default root certificates are offered | |||
| with this parameter, occasionally an excessive amount of memory (~10MB) is | |||
There was a problem hiding this comment.
This is inaccurate and misleading. The root cert store is ~1MB. But gRPC opens multiple subchannels, each subchannel has its own SSL context and root store.
I would rephrase as
gRPC has implemented root certificates cache if the underlying OpenSSL library supports it. The gRPC root certificates cache is only applicable on the default root certificates. If user provides his or her own pem_root_certs when creating an SSL credential object, gRPC would not be able to cache it.
There was a problem hiding this comment.
looks good :) I'll rephrase
dapengzhang0
approved these changes
Mar 12, 2019
include/grpc/grpc_security.h
Outdated
| it. The gRPC root certificates cache is only applicable on the default | ||
| root certificates, which is used when this parameter is nullptr. If user | ||
| provides their own pem_root_certs, when creating an SSL credential object, | ||
| gRPC would not be able to cached it, and each subchannel will generate a |
jiangtaoli2016
approved these changes
Mar 12, 2019
|
Thanks much @muxi for quick fix! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gRPC ObjC library had been reading the default pem file's contents and feed them into
grpc_ssl_credential_create, caused multiple apps to crash due to excessive memory usage. This PR changes the usage ofgrpc_ssl_credential_createto use the cached credentials which is more preferable and avoid further crashes.