Skip to content
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

Jump to bottom

Test 1013 fails due to differences in protocol list #1262

Closed
jay opened this issue Feb 14, 2017 · 3 comments
Closed

Test 1013 fails due to differences in protocol list #1262

jay opened this issue Feb 14, 2017 · 3 comments
Assignees
@MarcelRaad
Labels
build Windows Windows-specific

Comments

@jay
Copy link
Member

jay commented Feb 14, 2017 •
edited

I did this

Built a curl tool debug build using mingw32 msys:

make distclean
./buildconf
CFLAGS=-DNGHTTP2_STATICLIB \
CPPFLAGS=-DMEMDEBUG_LOG_SYNC \
LDFLAGS=-static \
PKG_CONFIG="pkg-config --static" \
./configure \
  --disable-shared \
  --disable-threaded-resolver \
  --enable-static \
  --enable-debug \
  --enable-sspi \
  --prefix=c:/mingw/msys/1.0/local \
  --with-ssl=c:/mingw/msys/1.0/local \
  --with-nghttp2=c:/mingw/msys/1.0/local \
  >config.out 2>&1
make V=1 curl_LDFLAGS=-all-static >make.out 2>&1

Of note is --enable-sspi.

Test 1013 failed

test 1013...[Compare curl --version with curl-config --protocols]
Mismatch in protocols lists:
curl:        dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smtp smtps telnet tftp
curl-config: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
 postcheck FAILED

Of note is that smb and smbs are not listed protocols in curl --version.

Analysis

configure.ac uses this logic to decide whether to put smb in the curl-config --protocols protocol list:

if test "x$CURL_DISABLE_SMB" != "x1" \
    -a "x$CURL_DISABLE_CRYPTO_AUTH" != "x1" \
    -a \( "x$OPENSSL_ENABLED" = "x1" -o "x$USE_WINDOWS_SSPI" = "x1" \
      -o "x$GNUTLS_ENABLED" = "x1" -o "x$MBEDTLS_ENABLED" = "x1" \
      -o "x$NSS_ENABLED" = "x1" -o "x$DARWINSSL_ENABLED" = "x1" \); then
  SUPPORT_PROTOCOLS="$SUPPORT_PROTOCOLS SMB"
  if test "x$SSL_ENABLED" = "x1"; then
    SUPPORT_PROTOCOLS="$SUPPORT_PROTOCOLS SMBS"
  fi
fi

Of note is OPENSSL_ENABLED and USE_WINDOWS_SSPI variables are true so SMB and SMBS are in the curl-config --protocols protocol list.

lib/version.c uses this logic to decide whether to put smb in the curl --version protocol list:

#if !defined(CURL_DISABLE_SMB) && defined(USE_NTLM) && \
   (CURL_SIZEOF_CURL_OFF_T > 4) && \
   (!defined(USE_WINDOWS_SSPI) || defined(USE_WIN32_CRYPTO))
  "smb",
#  ifdef USE_SSL
  "smbs",
#  endif
#endif

Of note is (!defined(USE_WINDOWS_SSPI) || defined(USE_WIN32_CRYPTO)) which is also the logic lib/smb.c is guaraded with. For this curl --version case, which does not include smb, I've got USE_WINDOWS_SSPI defined in my lib/curl_config.h but not USE_WIN32_CRYPTO, and as a result means smb won't be enabled.

USE_WIN32_CRYPTO is only defined in lib/config-win32.h (which is not used here since I'm using autotools generated config) and with this logic:

#if !defined(USE_OPENSSL) && !defined(USE_NSS)
#define USE_WIN32_CRYPTO
#endif

I'm not sure why only config-win32 defines USE_WIN32_CRYPTO and only under those circumstances, and what is correct here.

curl/libcurl version

master e1187c4 2017-02-11

curl 7.53.0-DEV (i686-pc-mingw32) libcurl/7.53.0-DEV OpenSSL/1.0.2k zlib/1.2.11 nghttp2/1.19.0
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smtp smtps telnet tftp
Features: Debug TrackMemory Largefile SSPI Kerberos SPNEGO NTLM SSL libz TLS-SRP HTTP2 HTTPS-proxy

operating system

Windows 7 x64 Enterprise
@jay jay added the build label Feb 14, 2017
@bagder
Copy link
Member

bagder commented Feb 14, 2017

I'm not sure why only config-win32 defines USE_WIN32_CRYPTO and only under those circumstances, and what is correct here.

It seems wrong on at least three ways:

  1. We want to be able to explicitly select using wincrypto, not just implicitly like this by not selecting openssl or NSS.

  2. Win builds could also use other crypto backends (like wolfssl).

  3. we want to enable wincrypto in configure builds too

@jay jay changed the title Test 1031 fails due to differences in protocol list Test 1013 fails due to differences in protocol list Feb 22, 2017
@stale stale bot added the stale label Aug 22, 2017
@stale
Copy link

stale bot commented Aug 22, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this as completed Sep 5, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
@MarcelRaad MarcelRaad mentioned this issue Oct 5, 2018
Closed
@MarcelRaad MarcelRaad mentioned this issue Jun 11, 2019
Closed
@MarcelRaad MarcelRaad reopened this Aug 2, 2020
@stale stale bot removed the stale label Aug 2, 2020
@MarcelRaad
Copy link
Member

MarcelRaad commented Aug 2, 2020 •
edited

I've been bitten by this issue often enough to finally start working on it. Assigning to myself so that I don't forget it once again.

I think the root cause is that the definition of USE_NTLM is not correct as it's not enough to actually build with NTLM support (or was not correct at that time maybe), requiring all those additional !defined(USE_WINDOWS_SSPI) || defined(USE_WIN32_CRYPTO) wherever NTLM is used.
Correction: it actually only affects curl_ntlm_core as used by the SMB code because it needs a backend that supports DES, while ntlm_sspi.c does not. So USE_NTLM is correct nowadays, only the condition guarding curl_ntlm_core is not as it wrongly excludes "SSPI + SSL backend but without USE_WIN32_CRYPTO".

@MarcelRaad MarcelRaad added the Windows Windows-specific label Aug 2, 2020
@curl curl unlocked this conversation Aug 2, 2020
MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Aug 3, 2020
@MarcelRaad
ntlm: fix condition for curl_ntlm_core usage
a3faffd 
`USE_WINDOWS_SSPI` without `USE_WIN32_CRYPTO` but with any other DES
backend is fine, but was excluded before.

This also fixes test 1013 as the condition for SMB support in
configure.ac didn't match the condition in the source code. Now it
does.

Fixes curl#1262
Closes
This was referenced Aug 3, 2020
Closed
Closed
MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Aug 13, 2020
@MarcelRaad
ntlm: fix condition for curl_ntlm_core usage
f2dacc2 
`USE_WINDOWS_SSPI` without `USE_WIN32_CRYPTO` but with any other DES
backend is fine, but was excluded before.

This also fixes test 1013 as the condition for SMB support in
configure.ac didn't match the condition in the source code. Now it
does.

Fixes curl#1262
Closes curl#5771
MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Aug 13, 2020
@MarcelRaad
ntlm: fix condition for curl_ntlm_core usage
d2bc34b 
`USE_WINDOWS_SSPI` without `USE_WIN32_CRYPTO` but with any other DES
backend is fine, but was excluded before.

This also fixes test 1013 as the condition for SMB support in
configure.ac didn't match the condition in the source code. Now it
does.

Fixes curl#1262
Closes curl#5771
MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Aug 14, 2020
@MarcelRaad
ntlm: fix condition for curl_ntlm_core usage
6bcb4b5 
`USE_WINDOWS_SSPI` without `USE_WIN32_CRYPTO` but with any other DES
backend is fine, but was excluded before.

This also fixes test 1013 as the condition for SMB support in
configure.ac didn't match the condition in the source code. Now it
does.

Fixes curl#1262
Closes curl#5771
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MarcelRaad MarcelRaad

Labels
build Windows Windows-specific
Milestone
No milestone
4 participants
@bagder @jay @captain-caveman2k @MarcelRaad