Ability to disable specific auths (Basic, Bearer, Digest, etc.) and non-standard like AWS. #11490

wyattoday · 2023-07-20T14:25:19Z

In the spirit of tiny-curl deprecated, optional, and non-standard code should be able to be compiled-out.

AWS-SIG4 is an organization-specific (namely, Amazon) chunk of code that is not applicable outside of communicating with Amazon servers. It is not standardized. Personally, I think this should be disabled by default in libCurl (and maybe even curl-cli too), but a good place to start is even adding the ability to disable it.

Digest Auth is actually a standard (unlike AWS-SIG4) but has long-since become deprecated (both by standards organizations and by end-user corporations that had previously used Digest).

Lastly, Bearer, Basic, Negotiate, and Kerberos auth types are also configurably disabled. These are all safe authentication methods, but not every user of curl / libcurl will need them. Hence, reduce compiled binary size (and executed run-time checks) when they're not needed.

This PR continues that by adding 6 configure options / defines:

--disable-basic-auth
--disable-bearer-auth
--disable-digest-auth
--disable-kerberos-auth
--disable-negotiate-auth
--disable-aws

CURL_DISABLE_BASIC_AUTH
CURL_DISABLE_BEARER_AUTH
CURL_DISABLE_DIGEST_AUTH
CURL_DISABLE_KERBEROS_AUTH
CURL_DISABLE_NEGOTIATE_AUTH
CURL_DISABLE_AWS

This also removes the general CURL_DISABLE_CRYPTO_AUTH (as suggested below by @bagder ) since the specific auth-types can be disabled one-by-one.

Personally, I think both Digest & AWS should be disabled by default. But this is a good start.

We already deploy and use versions of libcurl with these auths (and AWS) disabled (and have for a few years now).

…dard like AWS.

dfandrich · 2023-07-20T15:24:57Z

--disable-crypto-auth is already there to disable them all, Basic notwithstanding.

wyattoday · 2023-07-20T15:40:10Z

--disable-crypto-auth is already there to disable them all, Basic notwithstanding.

Yeah, Basic isn't in there. And there's the case where you need NTLM but --disable-crypto-auth removes that ability.

Plus this removes AWS and give the ability for fine-grained disabling of individual auths that aren't needed.

configure.ac

bagder · 2023-07-23T14:50:44Z

--disable-basic-auth
--disable-bearer-auth
--disable-digest-auth
--disable-aws

Shouldn't this then also include ntlm and negotiate to the mix so that they all are independently controlled the same way?

@vszakats

Explicitly mark which is default on @vszakats request.

wyattoday · 2023-07-26T20:06:53Z

--disable-basic-auth
--disable-bearer-auth
--disable-digest-auth
--disable-aws

Shouldn't this then also include ntlm and negotiate to the mix so that they all are independently controlled the same way?

CURL_DISABLE_NTLM already exists, but you're right, I've added CURL_DISABLE_NEGOTIATE_AUTH.

…. This will prevent accidental use in the future.

MarcelRaad · 2023-07-27T20:01:49Z

Would it make sense to change CURL_DISABLE_CRYPTO_AUTH to only set these other defines and remove all checks for CURL_DISABLE_CRYPTO_AUTH?

Also, CMake is missing and the spellcheck job is failing. Apart from that, this PR looks good to me!

configure.ac

docs/CURL-DISABLE.md

…removal of `CURL_DISABLE_CRYPTO_AUTH`.

…PTO_AUTH`

wyattoday · 2023-09-06T14:37:13Z

@MarcelRaad

Also, CMake is missing and the spellcheck job is failing. Apart from that, this PR looks good to me!

Fixed the spelling and added the CMake changes.

@bagder

I'm all done making the requested changes. All the tests are passing (except for some failures due to the brittleness of the tests, not the code being tested).

Should be good for a final review and/or merge.

Let me know if there's anything else you want me to address.

bagder · 2023-09-07T15:45:38Z

Thanks!

Both with configure and cmake Closes curl#11490

Follow-up to e92edfb curl#11490 Closes #xxxxx

Delete leftovers of the `crypt-auth` `./configure` option and add the new ones that replaced them. Follow-up to e92edfb #11490 Reviewed-by: Daniel Stenberg Closes #12194

This curl v8.4.0 update: curl/curl@e92edfb curl/curl#11490 deleted a build option we used and replaced it with multiple new ones. autotools now also has a documented defaults for them. Update our options accordingly in cmake and autotools builds.

``` ./curl/lib/http.c:737:12: warning: unused variable 'result' [-Wunused-variable] CURLcode result = CURLE_OK; ^ ./curl/lib/http.c:1143:12: warning: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Wextra-semi-stmt] ; ^ ./curl/lib/http.c:995:18: warning: variable 'availp' set but not used [-Wunused-but-set-variable] unsigned long *availp; ^ ./curl/lib/http.c:996:16: warning: variable 'authp' set but not used [-Wunused-but-set-variable] struct auth *authp; ^ ./curl/lib/http.c:979:12: warning: unused function 'is_valid_auth_separator' [-Wunused-function] static int is_valid_auth_separator(char ch) ^ 5 warnings generated. ``` Follow-up to 0d3956b curl#11895 Follow-up to e92edfb curl#11490 Closes #xxxxx

``` ./curl/lib/http.c:979:12: warning: unused function 'is_valid_auth_separator' [-Wunused-function] static int is_valid_auth_separator(char ch) ^ 5 warnings generated. ``` Follow-up to e92edfb #11490 Closes #12227

Builds with libssh2 + `-DCURL_DISABLE_DIGEST_AUTH=ON` + `-DCURL_DISABLE_AWS=ON` in combination with either Schannel on Windows, or `-DCURL_DISABLE_NTLM=ON` on other operating systems failed while compiling due to a missing HMAC declaration. The reason is that HMAC is required by `lib/sha256.c` which publishes `Curl_sha256it()` which is required by `lib/vssh/libssh2.c` when building for libssh2 v1.8.2 (2019-05-25) or older. Make sure to compile the HMAC bits for a successful build. Both HMAC and `Curl_sha256it()` rely on the same internals, so splitting them into separate sources isn't practical. Fixes: ``` [...] In file included from ./curl/_x64-win-ucrt-cmake-llvm-bld/lib/CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c:310: ./curl/lib/sha256.c:527:42: error: array has incomplete element type 'const struct HMAC_params' 527 | const struct HMAC_params Curl_HMAC_SHA256[] = { | ^ ./curl/lib/curl_sha256.h:34:21: note: forward declaration of 'struct HMAC_params' [...] ``` Regression from e92edfb #11490 Fixes #12273 Closes #12332

Fix compiler warnings in builds with disabled auths, NTLM and SPNEGO. E.g. with `CURL_DISABLE_BASIC_AUTH` + `CURL_DISABLE_BEARER_AUTH` + `CURL_DISABLE_DIGEST_AUTH` + `CURL_DISABLE_NEGOTIATE_AUTH` + `CURL_DISABLE_NTLM` on non-Windows. ``` ./curl/lib/http.c:737:12: warning: unused variable 'result' [-Wunused-variable] CURLcode result = CURLE_OK; ^ ./curl/lib/http.c:995:18: warning: variable 'availp' set but not used [-Wunused-but-set-variable] unsigned long *availp; ^ ./curl/lib/http.c:996:16: warning: variable 'authp' set but not used [-Wunused-but-set-variable] struct auth *authp; ^ ``` Regression from e92edfb #11490 Fixes #12228 Closes #12335

The commit e92edfbef644 ("lib: add ability to disable auths individually") [1][2] in Curl 8.3.0 removes CURL_DISABLE_CRYPTO_AUTH and introduces CMake options for a number crypto protocols. The patch reflects this change in Tarantool's build infrastructure. 1. curl/curl@e92edfb 2. curl/curl#11490 Follows up tarantool#9086 NO_CHANGELOG=third_party NO_DOC=third_party NO_TEST=third_party

The commit "lib: add ability to disable auths individually" [1][2] in Curl 8.3.0 removes CURL_DISABLE_CRYPTO_AUTH and introduces CMake options for a number crypto protocols. The patch reflects this change in Tarantool's build infrastructure. 1. curl/curl@e92edfb 2. curl/curl#11490 Follows up tarantool#9086 NO_CHANGELOG=third_party NO_DOC=third_party NO_TEST=third_party

The commit "lib: add ability to disable auths individually" [1][2] in Curl 8.3.0 removes CURL_DISABLE_CRYPTO_AUTH and introduces CMake options for a number crypto protocols. The patch reflects this change in Tarantool's build infrastructure. 1. curl/curl@e92edfb 2. curl/curl#11490 Follows up #9086 NO_CHANGELOG=third_party NO_DOC=third_party NO_TEST=third_party

The commit "lib: add ability to disable auths individually" [1][2] in Curl 8.3.0 removes CURL_DISABLE_CRYPTO_AUTH and introduces CMake options for a number crypto protocols. The patch reflects this change in Tarantool's build infrastructure. 1. curl/curl@e92edfb 2. curl/curl#11490 Follows up tarantool#9086 NO_CHANGELOG=third_party NO_DOC=third_party NO_TEST=third_party (cherry picked from commit c9daf9b)

The commit "lib: add ability to disable auths individually" [1][2] in Curl 8.3.0 removes CURL_DISABLE_CRYPTO_AUTH and introduces CMake options for a number crypto protocols. The patch reflects this change in Tarantool's build infrastructure. 1. curl/curl@e92edfb 2. curl/curl#11490 Follows up #9086 NO_CHANGELOG=third_party NO_DOC=third_party NO_TEST=third_party (cherry picked from commit c9daf9b)

Add ability to disable auths (Basic, Bearer, and Digest) and non-stan…

41f183f

…dard like AWS.

Fix style errors.

ca491f6

vszakats reviewed Jul 21, 2023

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

bagder added build authentication labels Jul 23, 2023

Add CURL_DISABLE_NEGOTIATE on @bagder's request.

1064561

Explicitly mark which is default on @vszakats request.

wyattoday added 2 commits July 26, 2023 16:12

Match existing style.

972eb01

Remove some unused digest structs / definitions if Digest is disabled…

46c0779

…. This will prevent accidental use in the future.

bagder reviewed Aug 6, 2023

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

bagder reviewed Aug 15, 2023

View reviewed changes

docs/CURL-DISABLE.md Outdated Show resolved Hide resolved

wyattoday added 5 commits September 5, 2023 11:29

Merge branch 'curl:master' into disable-digest-auth

b261588

Fix spelling bot.

07c7d11

Begin removing CURL_DISABLE_CRYPTO_AUTH.

7dc47b7

Remove more CURL_DISABLE_CRYPTO_AUTH instances.

7dfffd5

More removal of the CURL_DISABLE_CRYPTO_AUTH define.

7d83a92

github-actions bot added the tests label Sep 5, 2023

wyattoday added 2 commits September 6, 2023 07:11

Fix sasl build.

e0c9f50

Add CURL_DISABLE_KERBEROS_AUTH and --disable-kerberos-auth. More …

4e1d82a

…removal of `CURL_DISABLE_CRYPTO_AUTH`.

wyattoday changed the title ~~Ability to disable auths (Basic, Bearer, and Digest) and non-standard like AWS.~~ Ability to disable specific auths (Basic, Bearer, Digest, etc.) and non-standard like AWS. Sep 6, 2023

Add CMake defines. Remove all remaining remnants of `CURL_DISABLE_CRY…

34e5942

…PTO_AUTH`

bagder approved these changes Sep 7, 2023

View reviewed changes

bagder closed this in e92edfb Sep 7, 2023

ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023

lib: add ability to disable auths individually

ac13ece

Both with configure and cmake Closes curl#11490

vszakats added a commit to vszakats/curl that referenced this pull request Oct 25, 2023

autotools: update references to deleted crypt-auth option

2deb6ed

Follow-up to e92edfb curl#11490 Closes #xxxxx

vszakats mentioned this pull request Oct 25, 2023

autotools: update references to deleted crypt-auth option #12194

Closed

This was referenced Oct 29, 2023

build: fix compiler warning with auths disabled #12227

Closed

compiler warnings in http.c when all auth methods disabled #12228

Closed

vszakats mentioned this pull request Nov 5, 2023

libssh2 + CURL_DISABLE_DIGEST_AUTH + CURL_DISABLE_AWS build error #12273

Closed

vszakats mentioned this pull request Nov 15, 2023

build: fix libssh2 + CURL_DISABLE_DIGEST_AUTH + CURL_DISABLE_AWS #12332

Closed

vszakats mentioned this pull request Nov 16, 2023

http: fix -Wunused-variable compiler warnings #12335

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to disable specific auths (Basic, Bearer, Digest, etc.) and non-standard like AWS. #11490

Ability to disable specific auths (Basic, Bearer, Digest, etc.) and non-standard like AWS. #11490

wyattoday commented Jul 20, 2023 •

edited

dfandrich commented Jul 20, 2023 via email

wyattoday commented Jul 20, 2023

bagder commented Jul 23, 2023

wyattoday commented Jul 26, 2023 •

edited

MarcelRaad commented Jul 27, 2023 •

edited

wyattoday commented Sep 6, 2023

bagder commented Sep 7, 2023

Ability to disable specific auths (Basic, Bearer, Digest, etc.) and non-standard like AWS. #11490

Ability to disable specific auths (Basic, Bearer, Digest, etc.) and non-standard like AWS. #11490

Conversation

wyattoday commented Jul 20, 2023 • edited

dfandrich commented Jul 20, 2023 via email

wyattoday commented Jul 20, 2023

bagder commented Jul 23, 2023

wyattoday commented Jul 26, 2023 • edited

MarcelRaad commented Jul 27, 2023 • edited

wyattoday commented Sep 6, 2023

bagder commented Sep 7, 2023

wyattoday commented Jul 20, 2023 •

edited

wyattoday commented Jul 26, 2023 •

edited

MarcelRaad commented Jul 27, 2023 •

edited