curl: add options for safe/no CA bundle search (Windows) #14582
Closed
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
2 tasks
vszakats
changed the title
|
Tested OK in a local build. |
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
5407312
to
7297995
Compare
vszakats
added
the
feature-window
vszakats
force-pushed
the
wnd-disable-searchpath
branch
2 times, most recently
from
feecb41
to
739d930
Compare
vszakats
changed the title
|
Made this option available for all platforms (was: Windows-only). I figure it might be useful to allow picking up |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
vszakats
added a commit
to curl/curl-for-win
that referenced
this pull request
Aug 29, 2024
The CA bundle is about to be shipped embedded in the curl tool starting the next release. It can be extracted with the `--dump-ca-embed` option. This makes it redundant to ship it as an external file. Esp. for Linux and macOS builds which don't pick up the external bundle like Windows does. This may be fixed in the next next release, pending: curl/curl#14582
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
1afab81
to
0c63038
Compare
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
0c63038
to
50e507c
Compare
vszakats
changed the title
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
5fd9fd4
to
3f2975c
Compare
vszakats
force-pushed
the
wnd-disable-searchpath
branch
2 times, most recently
from
465e429
to
678353c
Compare
vszakats
changed the title
vszakats
force-pushed
the
wnd-disable-searchpath
branch
2 times, most recently
from
af9e326
to
a24229d
Compare
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
a24229d
to
ee35b3f
Compare
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
ee35b3f
to
614e925
Compare
Add `CURL_WIN32_SAFE_CA_SEARCH` build-time option to restrict implicit CA bundle search to the `curl.exe` directory. The lookup method was already used to find `.curlrc` and `_curlrc` on Windows. You can enable it with: - cmake: `-DCMAKE_C_FLAGS=-DCURL_WIN32_SAFE_CA_SEARCH` - autotools: `CPPFLAGS=-DCURL_WIN32_SAFE_CA_SEARCH` Before this patch the whole `PATH` was searched for a CA bundle. The `PATH` may contain unwanted or world-writable locations, and while searching them all is convenient to pick up any CA bundle, it's not necessarily safe. Note that on Windows `PATH` also includes the current directory. confirm it enabled add verbose message when searching for CA bundle in PATH add cmake option auto-enable for UWP builds UWP builds had no CA bundle load feature at all before this patch, because the necessary code did not compile. enabled OK https://github.com/curl/curl/actions/runs/10441479612/job/28912667429?pr=14582#step:10:364 drop warning, it breaks tests autotools: add `--with-windows-safe-ca-search` option update docs drop macro no longer used in master simplify C code make Curl_win32_execpath() portable allow CURL_WIN32_SAFE_CA_SEARCH on all platforms rename CURL_WIN32_SAFE_CA_SEARCH to CURL_SAFE_CA_SEARCH make configure option platform agnostic rename CURL_SAFE_CA_SEARCH to CURL_CA_SAFE_SEARCH rename rest of names add missing curlx.h enable in autotools/cmake macOS jobs update docs (still mentioned near Windows, even though the option supports all platforms) checksrc try fix 1 move tool_argv0 to global config struct try fix 2 silence unused var ca safe search -> ca search safe add option CURL_CA_SEARCH_DISABLE to disable search fully do not enable safe CA search for UWP by default am: add option for CA search disable cmake: add option for CA search disable configure.ac: safe search option fixup am: tweak cm: tweak am: tweak make CURL_CA_SEARCH_DISABLE Windows-specific am: tweak cm: tweak am: tweak am: switch to enable/disable from with/without rename disable option with "disable" first am: fixup indent am: syntax cleanup update docs line length add to CURL-DISABLE clarify text that PATH search is unsafe test1165: scan src sources for CURL_DISABLED use Before this patch, this happened: ``` Not set by configure: CURL_DISABLE_CA_SEARCH (./src/tool_operate.c) Not set by CMakeLists.txt: CURL_DISABLE_CA_SEARCH (./src/tool_operate.c) Documented but not in configure: CURL_DISABLE_CA_SEARCH Documented but not in CMakeLists.txt: CURL_DISABLE_CA_SEARCH ``` accept CURL_DISABLE when passed COMPILE_DEFINITIONS set/init build options in project root Both options solely apply to `src`. Initially it seems like a good idea to reflect that on the build level, but it turns out it introduces too much of an exception. Sync these settings with all the existing ones: propagate them via config.h and catch them in the root CMakeLists.txt. This allows to drop extra code from test1165.pl, and also make it possible to add these options to tests/server/disable. add new options to tests/server/disabled src: dedupe feature_ssl condition, drop redundant schannel check limit safe search option to Windows It's really only safe compared to the default search. On other platforms nobody asked for this, and it's less safe than using a well-known system location. Except for portable MUSL binaries, but for those, the CA embed option seems like a better solution than looking for new disk locations. (That's also true for Windows FWIW.) add new options to tests/server/disabled fixup and limit to _WIN32 CURL_WINDOWS_APP -> CURL_WINDOWS_UWP sync with master
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
b1f49b3
to
825de9f
Compare
It's not currently used. Also it would be better to replace the `argv[0]` method with `_NSGetExecutablePath()` on macOS. Since the functionality isn't Windows-specific, I left this function in `tool_util.c` (as opposed to `tool_doswin.c`).
vszakats
force-pushed
the
wnd-disable-searchpath
branch
from
825de9f
to
a11d2ac
Compare
vszakats
added a commit
to curl/curl-for-win
that referenced
this pull request
Sep 22, 2024
vszakats
removed
the
feature-window
vszakats
added a commit
to curl/curl-for-win
that referenced
this pull request
Sep 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Add
CURL_CA_SEARCH_SAFEbuild-time option to enable CA bundle searchin the
curltool directory. The lookup method was already used to find.curlrcand_curlrc(on Windows). On Windows it overrides the unsafedefault
SearchPath()method.Enable with:
-DCURL_CA_SEARCH_SAFE=ON--enable-ca-search-safeCPPFLAGS=-DCURL_CA_SEARCH_SAFEOn Windows, before this patch the whole
PATHwas searched fora CA bundle.
PATHmay contain unwanted or world-writable locations,including the current directory. Searching them all is convenient to
pick up any CA bundle, but not secure.
The Muldersoft curl distro implements such CA search via a custom
patch for Windows:
https://github.com/lordmulder/cURL-build-win32/blob/cd652d4792c177c98b08b4309d3cac2b8dbbf9b0/patch/curl_tool_doswin.diff#L50
MSYS2/mingw-w64 distro has also been rolling a patch solving this:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-curl/0001-Make-cURL-relocatable.patch
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-curl/pathtools.c
Also add option to fully disable Windows CA search:
-DCURL_DISABLE_CA_SEARCH=ON--disable-ca-searchCPPFLAGS=-DCURL_DISABLE_CA_SEARCH.Both options are considered EXPERIMENTAL, with possible incompatible
changes or even (partial) removal in the future, depending on feedback.
An alternative, secure option is to embed the CA bundle into the binary.
Safe search can be extended to other platforms if necessary or useful,
by using
_NSGetExecutablePath()(macOS),/proc/self/exe(Linux/Cygwin), orargv[0].w/o whitespace: https://github.com/curl/curl/pull/14582/files?w=1
cacert.pem? [SKIP]Curl_execpath()?_NSGetExecutablePath()on macOS? [FUTURE]tests/server/disabled.srcto standardconfig.h. Though these only affectsrc, we need these flags intests/server/disabledtoo. This can also avoid the unpleasant tricks intest1165.pl.config.h(because it only applies to src). This makes the checker confused after test1165: check ifcurl_config.h.cmakelists allDISABLEDoptions #14754.-DCURL_DISABLE_CA_SEARCH.SearchPathmethod? [NOPE]