-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Recommend OpenSSL in SECURITY.md #983
Conversation
@ejona86 PTAL |
@@ -7,10 +7,28 @@ As outlined in <a href="https://github.com/grpc/grpc/blob/master/doc/grpc-auth-s | |||
## Cipher-Suites | |||
Java 7 does not support the <a href="https://tools.ietf.org/html/draft-ietf-httpbis-http2-17#section-9.2.2">the cipher suites recommended</a> by the HTTP2 specification. To address this we suggest servers use Java 8 where possible or use an alternative JCE implementation such as <a href="https://www.bouncycastle.org/java.html">Bouncy Castle</a>. If this is not practical it is possible to use other ciphers but you need to ensure that the services you intend to call have <a href="https://github.com/grpc/grpc/issues/681">allowed out-of-spec ciphers</a> and have evaluated the security risks of doing so. On Android we recommend the use of the <a href="http://appfoundry.be/blog/2014/11/18/Google-Play-Services-Dynamic-Security-Provider/">Play Services Dynamic Security Provider</a> to ensure your application has an up-to-date OpenSSL library with the necessary ciper-suites and a reliable ALPN implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To address this we suggest servers use Java 8 where possible
That is odd language now, since we don't actually suggest using Java 8's SSL implementation.
@nmittler LGTM. I think later the document would be well-served by combining the Cipher+ALPN discussions and just split Android and non-Android, but that will be more substantial. |
1b380d2
to
fe56a2a
Compare
fe56a2a
to
1a2c428
Compare
@ejona86 PTAL |
### Requirements for using OpenSSL | ||
|
||
1. Currently only supported by the Netty transport (via netty-tcnative). | ||
2. [OpenSSL](https://www.openssl.org/) version >= 1.0.2 for ALPN support, or version >= 1.0.0g for NPN. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did you get the version 1.0.0g? From looking at OpenSSL changelog, it looks like it should be 1.0.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I was just googling ... should have used the changelog instead. Done.
@nmittler, a comment about the OpenSSL version, but otherwise LGTM! Much easier to read! |
9bd23f6
to
40c4559
Compare
@nmittler LGTM. Yay! No more need for antrun. |
@zhangkun83 you may be interested for the gradle plugin config. |
87d169a
to
5465ca1
Compare
buildscript { | ||
repositories { | ||
mavenCentral() | ||
mavenLocal() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mavenCentral() is enough. The user wouldn't need mavenLocal().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@zhangkun83 PTAL |
c0c95dd
to
ff50f52
Compare
ff50f52
to
be2ada6
Compare
@nmittler LGTM |
cherry-picked as 4ff4314 |
Fixes #547