ECH experimental

[sftcd]

This is an (as-promised, on the mailing list) early pull request for adding HTTPS RR an ECH support to cURL, that has had so far minimal testing when using OpenSSL or wolfSSL as the TLS provider, but does seem to work. (There's a known outstanding issue with the wolfSSL build.) The goal for now is to get some early but detailed review as to whether the direction of the current code changes looks good. It may well be that this PR evolves into something that can be merged, or it may require a chunk more work first, either is OK.

[bagder]

Some of the CI test failures happen because docs/options-in-versions and docs/libcurl/symbols-in-versions need updating

[sftcd]

Some of the CI test failures happen because docs/options-in-versions and docs/libcurl/symbols-in-versions need updating

I'll push a version with the mostly trivial changes indicated above. (I hit the "resolve" button on those that seem most trivial:-) I'll then rebase to try make the CI situation better and push another.

Thanks for all the good comments meanwhile!

[sftcd]

So now that I'm clearing up some of the CI chaff, an interesting question arises - if we're not doing ECH, then when, if ever, should we make a DNS query for an HTTTPS RR? Probably, for now, never, but that'll likely change if/as support for other uses for HTTPS RRs is added.

[sftcd]

The current DoH code seems to setup a new TLS session with the DoH-server for each DoH query. That probably makes no difference if the client is only making one request for one A record, but if we're now sometimes requesting A, AAAA and HTTPS RRs, it may be worth refactoring that code to use just one TLS session. Could do that but probably better as a separate PR sometime. Would need to understand though if it were possible/desirable to preserve the TLS session with the DoH-server across the runtime of an application using the library, which'd be more complex, but sometimes much more efficient.

[sftcd]

On 29/09/2023 17:13, Daniel Stenberg wrote: While you were working on this, we changed how these references should be formatted and they are now verified to follow this format by test 1173. Now they should be: ~~~ .BR CURLOPT_ECH_CONFIG (3), .BR CURLOPT_ECH_STATUS (3)

Aha! Thanks for that - it was doing my head in trying to figure why the local tests differed from the CI builds:-) I'll process the your other comments shortly, but mostly they look like good things to do. Cheers, S.

[sftcd]

This is out dated, see further down for latest.

So the latest commit fb2b2cd re-does the command line args as follows:

• --ech <config> - the config value can be one of:
  • false says to not attempt ECH
  • true says to attempt ECH, if possible
  • grease if attempting ECH is not possible, then send a GREASE'd ECH extension
  • hard hard-fail the connection if ECH cannot be attempted
  • a base64 encoded ECHConfigList, rather than one accessed from the DNS
• --echpublic over-ride the public_name from an ECHConfigList

I think that's along the lines suggested above.

Still needs some testing and happy to tweak more.

[sftcd]

Given there's a preference for fewer options, the latest push reduces the command line to one option only as in:

• --ech <config> - the config value can be one of:
  • false says to not attempt ECH
  • true says to attempt ECH, if possible
  • grease if attempting ECH is not possible, then send a GREASE ECH extension
  • hard hard-fail the connection if ECH cannot be attempted
  • ecl:<b64value> a base64 encoded ECHConfigList, rather than one accessed from the DNS
  • pn:<name> over-ride the public_name from an ECHConfigList

[vszakats]

I hope it's not too late, but I made this patch for CMake earlier:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 96a1aeb21..cf188437e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -732,6 +732,28 @@ if(USE_MSH3)
   list(APPEND CURL_LIBS ${MSH3_LIBRARIES})
 endif()
 
+option(USE_HTTPSRR "Enable HTTPS RR support for ECH (experimental)" OFF)
+option(USE_ECH "Enable ECH support" OFF)
+if(USE_ECH)
+  if(USE_OPENSSL OR USE_WOLFSSL)
+    # Be sure that the OpenSSL/wolfSSL library actually supports ECH.
+    if(NOT DEFINED HAVE_OPENSSL_ECH)
+      if(USE_OPENSSL AND HAVE_BORINGSSL)
+        openssl_check_symbol_exists(SSL_set1_ech_config_list "openssl/ssl.h" HAVE_OPENSSL_ECH)
+      elseif(USE_OPENSSL)
+        openssl_check_symbol_exists(SSL_ech_set1_echconfig "openssl/ssl.h" HAVE_OPENSSL_ECH)
+      elseif(USE_WOLFSSL)
+        openssl_check_symbol_exists(wolfSSL_CTX_GenerateEchConfig "wolfssl/options.h;wolfssl/ssl.h" HAVE_OPENSSL_ECH)
+      endif()
+    endif()
+    if(NOT HAVE_OPENSSL_ECH)
+      message(FATAL_ERROR "ECH support missing in OpenSSL/BoringSSL/wolfSSL")
+    endif()
+  else()
+    message(FATAL_ERROR "ECH requires OpenSSL, BoringSSL or wolfSSL")
+  endif()
+endif()
+
 if(NOT CURL_DISABLE_SRP AND (HAVE_GNUTLS_SRP OR HAVE_OPENSSL_SRP))
   set(USE_TLS_SRP 1)
 endif()
@@ -1563,6 +1585,7 @@ _add_if("TLS-SRP"       USE_TLS_SRP)
 # TODO option --with-nghttp2 tests for nghttp2 lib and nghttp2/nghttp2.h header
 _add_if("HTTP2"         USE_NGHTTP2)
 _add_if("HTTP3"         USE_NGTCP2 OR USE_QUICHE)
+_add_if("ECH"           USE_ECH)
 _add_if("MultiSSL"      CURL_WITH_MULTI_SSL)
 # TODO wolfSSL only support this from v5.0.0 onwards
 _add_if("HTTPS-proxy"   SSL_ENABLED AND (USE_OPENSSL OR USE_GNUTLS
diff --git a/lib/curl_config.h.cmake b/lib/curl_config.h.cmake
index 0bfb45796..b38a62359 100644
--- a/lib/curl_config.h.cmake
+++ b/lib/curl_config.h.cmake
@@ -814,3 +814,9 @@ ${SIZEOF_TIME_T_CODE}
 
 /* Define to 1 to enable TLS-SRP support. */
 #cmakedefine USE_TLS_SRP 1
+
+/* Define to 1 to force HTTPS RR support for ECH. */
+#cmakedefine USE_HTTPSRR 1
+
+/* Define to 1 if ECH support is available. */
+#cmakedefine USE_ECH 1

Short of an ECH-enabled TLS library, it's only lightly tested.

[sftcd]

I hope it's not too late, but I made this patch for CMake earlier:

Short of an ECH-enabled TLS library, it's only lightly tested.

Oh thanks, I'll compare it with what I just did.

BTW - does the cmake build work cleanly with the master branch? I saw some issues with that. (My flailing about notes are here:-)

[vszakats]

For local test builds, cmake works (for me) with master, just by omitting USE_MANUAL. In this case the build creates a dummy tool_hugehelp.c automatically, and curl doesn't have a manual embedded.

But, enabling it can be tricky with out of tree builds.

To prevent rebuilding the manual I use -DCMAKE_C_FLAGS=-DUSE_MANUAL=1 (instead of -DUSE_MANUAL=ON).
Then supply a pre-built or dummy tool_hugehelp.c copy myself, e.g. with cp -p src/tool_hugehelp.c.cvs build/src/tool_hugehelp.c:

https://github.com/curl/curl-for-win/blob/06edf3f3b6d81c1efc419a5933a93fe6ffa2816c/curl-cmake.sh#L352-L359

[sftcd]

I hope it's not too late, but I made this patch for CMake earlier:

Thanks again @vszakats. Since yours was cleaner than mine, I've now gone more or less with your suggested patch.

[vvb2060]

What is the meaning of --ech false, why not delete --ech?

[sftcd]

On 18/10/2023 16:47, 南宫雪珊 wrote: What is the meaning of `--ech false`, why not delete `--ech`?

In my local setup I have "ech=TRUE" in ``$HOME/.curlrc`` so this provides a way to turn that off from the command line, if needed. S.

[vvb2060]

curl/docs/cmdline-opts/ech.d

Line 10 in 9742dba

Multi: single

What if i want hard + base64 ECHConfigList?
My thoughts: --ech true/false/hard/grease[:string]
The string is optional, if it has a dot then it is public_name, otherwise it is base64 ECHConfigList

[sftcd]

curl/docs/cmdline-opts/ech.d

Line 10 in 9742dba

Multi: single

To be honest, I'm not sure what "Multi: single" means, I probably just copied from some other .d file.

What if i want hard + base64 ECHConfigList?

At present supplying --ech hard --ech ecl:<b64value> does that. Basically it'll use the "last" true/false/hard/grease value, then "last" public_name and the "last" base64-encoded ECHConfigList (if the latter two are supplied).

My thoughts: --ech true/false/hard/grease[:string] The string is optional, if it has a dot then it is public_name, otherwise it is base64 ECHConfigList

Not sure why that'd be better? Happy to change to something that is better of course but would like to understand why first. Switching [:string] based on the presence/absence of a dot also seems a bit brittle and who knows there might be a use-case for --ech pn:localhost or something else that isn't a FQDN.

[sftcd]

Thanks for the comments btw - I take it from those that, at minimum, more text is needed in ech.d to clarify the above. I'll add something in a while.

[bagder]

A bunch of comments of mine. Most of them are plain opinions so do take them as proposals, not strict requirements.

[bagder]

I think we can merge this as a first take once these minor nits are fixed. This is still experimental so we do not guarantee functionality or ABI/API compatibility, but getting the first version in git will allow for more and easier tests by a larger community.

One thing that needs fixing at some point is the configure/cmake checks: for multissl to work with ech, we need to allow per-TLS ECH ability, not a global boolean.

BTW, as do not think LD_LIBRARY_PATH is very nice to use, I prefer this build like this:

LDFLAGS="-Wl,-rpath,$HOME/build-openssl-ech/lib64" \
./configure  --with-ssl=$HOME/build-openssl-ech  --enable-ech --enable-httpsrr

[sftcd]

I think we can merge this as a first take once these minor nits are fixed. This is still experimental so we do not guarantee functionality or ABI/API compatibility, but getting the first version in git will allow for more and easier tests by a larger community.

Great, will fix the nits above in a sec.

One thing that needs fixing at some point is the configure/cmake checks: for multissl to work with ech, we need to allow per-TLS ECH ability, not a global boolean.

Good point. Happy to do it (now or later) - got an example of some other feature that's "per-TLS"?

BTW, as do not think LD_LIBRARY_PATH is very nice to use, I prefer this build like this:

LDFLAGS="-Wl,-rpath,$HOME/build-openssl-ech/lib64" \
./configure  --with-ssl=$HOME/build-openssl-ech  --enable-ech --enable-httpsrr

Yep, playing with that now - the above doesn't work for me right now for some reason though, but presumably that's something local so hopefully will figure it out today. (configure is failing not finding the right openssl with ECH support somehow;-) Once I fix that locally, I'll update the ECH.md instructions for building accordingly. Think that's working now, doing some testing...

[bagder]

Thanks a lot for your hard and patient work on this @sftcd . We can now continue polishing this in separate PRs.

[sftcd]

Thanks a lot for your hard and patient work on this @sftcd . We can now continue polishing this in separate PRs.

Excellent! Thanks for being so receptive for experimental stuff like this. Happy to help as useful in future too of course.