Basic TLS Encrypted ClientHello (ECH) support (updated) #1340
Conversation
This introduces a new Exception so that clients can respond only to the ECH retry request without having to parse SSLHandshakeExceptions in general. This exception should probably be implemented in boringssl or native_crypto.cc.
OpenJDK's JNDI API and Android DnsResolver API both provide support for raw DNS queries. These must be parsed to be useful, so this includes Android's DnsPacket to parse the raw DNS answer. Original source: https://android.googlesource.com/platform/frameworks/libs/net/+/de5905fe0407a1f5e115423d56c948ee2400683d/common/framework/com/android/net/module/util/DnsPacket.java
https://docs.gradle.org/current/userguide/gradle_wrapper.html#sec:verification https://gradle.org/release-checksums/ ./gradlew wrapper --gradle-distribution all --gradle-version 6.5 \ --gradle-distribution-sha256-sum \ c9910513d0eed63cd8f5c7fec4cb4a05731144770104a0871234a4edc3ba3cef
…ter_google_fix
…master_google_fix_merge
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| /** | ||
| * | ||
| * @param socket the socket | ||
| * @param enabled whether ECH GREASE is enabled or not |
There was a problem hiding this comment.
Good call, I fixed this and also added descriptions for the other new methods in Conscrypt.java.
|
Hi @mnbogner! Thank you for your patience in getting this PR reviewed. We are still working on the exact design as part of the platform, but there are some parts of this PR that can land, so let me help you get that happening! I know some of these changes are coming from @eighthave, but would it be possible to merge some of these commits (potentially adding a Signed-Off-By footer to capture the original author)? In this first iteration, it is probably better to only focus on the client-side of ECH (this reduces the scope of the change, knowing that we can always implement the server parts later on). The native layer (exposing BoringSSL API to Conscrypt) is probably the first commit that should land. That is exposing:
This would probably update native_crypto.cc as well as NativeCrypto.java. Updating parts of NativeCryptoTest.java would also be welcomed. The next commit would be the plumbing (e.g., getter/setter) through the different Socket classes:
Once that’s done, we can focus on the retry case and how it should be handled. It is likely we don’t want to do any DNS parsing within Conscrypt. We are exploring the options at the platform level. From the Conscrypt side, we will probably define an ECHConfigList class (similar to the ECHParameters class, but without the grease attribute), which will be exposed externally and populated for us (we are still discussing the options for that class, so no need to include it in the pull request). On the test side, some of these end-to-end tests might be better executed in Android CI infrastructure. Unit tests with mocked credentials are welcomed though. Please let me know if that makes sense. And feel free to tag me (@tweksteen) and @bjiang7 on future changes. Thanks! |
Thanks for the feedback. I've been working on other things so it might take a little while to get back into this frame of reference. It sounds like what you're asking for is to split up these changes and create several new PRs, the first one dealing with the native layer, and so on. If that's the case, I assume there's nothing more to be done with this PR? |
|
Yes, that's correct. It will make reviews easier and faster. No worries at all, thanks for letting us know. |
4 participants
This is an update to #1044 which includes both additional changes made by @eighthave and the updates made to conscrypt since the original pull request was created.
Original description:
This is the first stage of implementing Encrypted ClientHello (ECH) in Conscrypt #730. It provides the APIs required for clients to make TLS connections using ECH. This implements enough of the server-side to provide ECH in the test suite using ECH Key and Configs generated by boringssl. This should be enough to let libs like OkHTTP fully implement ECH square/okhttp#6539