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.

Sign up for GitHub

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

Schannel can't disable only CURLOPT_SSL_VERIFYPEER and still verify the host name #3284

Closed
bagder opened this issue Nov 17, 2018 · 3 comments
Closed

Schannel can't disable only CURLOPT_SSL_VERIFYPEER and still verify the host name #3284

bagder opened this issue Nov 17, 2018 · 3 comments
Assignees
@jay
Labels
TLS

Comments

@bagder
Copy link
Member

bagder commented Nov 17, 2018 •
edited by jay
Loading

I did this

Martin Galvan reported on the mailing list:

I was looking at the WinSSL code which processes
CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST, and noticed that,
if CURLOPT_SSL_VERIFYPEER isn't enabled, the host name checking won't
be performed:

https://github.com/curl/curl/blob/curl-7_62_0/lib/vtls/schannel_verify.c#L537-L541
https://github.com/curl/curl/blob/curl-7_62_0/lib/vtls/schannel.c#L1115-L1119

I expected the following

If the TLS backend has the API for it, the options should be independent.

curl/libcurl version

current git master

operating system

Windows
@bagder bagder added the TLS label Nov 17, 2018
@bagder bagder changed the title winssl backend can't disable only CURLOPT_SSL_VERIFYHOST winssl backend can't disable only CURLOPT_SSL_VERIFYPEER and still verify the host name Nov 17, 2018
bagder added a commit that referenced this issue Nov 17, 2018
@bagder
schannel: allow verifhost independently of verifypeer
dca765d 
As documented and as working in other TLS backends.

Fixes #3284
@bagder bagder mentioned this issue Nov 17, 2018
Closed
bagder added a commit that referenced this issue Nov 17, 2018
@bagder
schannel: allow verifyhost independently of verifypeer
5b190ed 
As documented and as working in other TLS backends.

Reported-by: Martin Galvan
Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Fixes #3284
Closes #3285
bagder added a commit that referenced this issue Nov 17, 2018
@bagder
schannel: allow verifyhost independently of verifypeer
7ef6f3b 
As documented and as working in other TLS backends.

Reported-by: Martin Galvan
Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Fixes #3284
Closes #3285
@jay
Copy link
Member

jay commented Nov 18, 2018

I'll take a look but I thought it was already independent.

curl/lib/vtls/schannel.c

Lines 536 to 541 in 1966771

if(!conn->ssl_config.verifyhost) {
schannel_cred.dwFlags |= SCH_CRED_NO_SERVERNAME_CHECK;
infof(data, "schannel: verifyhost setting prevents Schannel from "
"comparing the supplied target name with the subject "
"names in server certificates.\n");
}

@jay
Copy link
Member

jay commented Nov 18, 2018

It fails with SEC_E_WRONG_PRINCIPAL when verifypeer is enabled, but succeeds when it's disabled. (I'm using https://cdn0.nflximg.net as test server). When verifypeer is disabled then manual verification is enabled SCH_CRED_MANUAL_CRED_VALIDATION. What I think is happening is the manual verification disables the server name check even though we don't. Seems like they could have warned us of that in the doc.

jay added a commit to jay/curl that referenced this issue Nov 19, 2018
@jay
schannel: allow verifyhost independently of verifypeer
5ad4285 
Prior to this change if the user disabled the verify peer check then no
host check was done. Empirical testing shows
SCH_CRED_MANUAL_CRED_VALIDATION, which we use when peer verification is
disabled, also disables hostname verification.

In Windows < 8 our manual host verification check (ie the check used
when CA info is specified, or peer verification is disabled, or WinCE is
the OS) for schannel continues to only check the first subject alternate
name, and not all the names, since there is no easy way supported by the
API. It looks possible to do just more work, and should be addressed
separately.

Assisted-by: Daniel Stenberg
Reported-by: Martin Galvan

Fixes curl#3284
Closes curl#3285
Closes #xxxx
jay added a commit that referenced this issue Nov 25, 2018
@jay
schannel: allow verifyhost independently of verifypeer
8494374 
Prior to this change if the user disabled the verify peer check then no
host check was done. Empirical testing shows
SCH_CRED_MANUAL_CRED_VALIDATION, which we use when peer verification is
disabled, also disables hostname verification.

In Windows < 8 our manual host verification check (ie the check used
when CA info is specified, or peer verification is disabled, or WinCE is
the OS) for schannel continues to only check the first subject alternate
name, and not all the names, since there is no easy way supported by the
API. It looks possible to do just more work, and should be addressed
separately.

Assisted-by: Daniel Stenberg
Reported-by: Martin Galvan

Fixes #3284
Closes #3285
Closes #xxxx
@bagder
Copy link
Member Author

bagder commented Jun 10, 2019

@jay any idea on how to deal with this? Should we just close or can we document this somehow somewhere?

@bagder bagder changed the title winssl backend can't disable only CURLOPT_SSL_VERIFYPEER and still verify the host name Schannel can't disable only CURLOPT_SSL_VERIFYPEER and still verify the host name Jun 10, 2019
@bagder bagder closed this as completed in 53cc6c7 Jun 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 22, 2019
jay added a commit to jay/curl that referenced this issue Dec 8, 2022
@jay
schannel: verify hostname independent of verify cert
454aca8 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: https://github.com/curl/curl/blob/curl-7_86_0/docs/KNOWN_BUGS#L304-L308
Ref: curl#3285

Fixes curl#3284
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Dec 9, 2022
@jay
schannel: verify hostname independent of verify cert
8375779 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: https://github.com/curl/curl/blob/curl-7_86_0/docs/KNOWN_BUGS#L304-L308
Ref: curl#3285

Fixes curl#3284
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Dec 9, 2022
@jay
schannel: verify hostname independent of verify cert
25c5fd0 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: https://github.com/curl/curl/blob/curl-7_86_0/docs/KNOWN_BUGS#L304-L308
Ref: curl#3285

Fixes curl#3284
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Dec 9, 2022
@jay
schannel: verify hostname independent of verify cert
0790202 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: https://github.com/curl/curl/blob/curl-7_86_0/docs/KNOWN_BUGS#L304-L308
Ref: curl#3285

Fixes curl#3284
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Aug 8, 2023
@jay
schannel: verify hostname independent of verify cert
579bca8 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: curl#3285

Fixes curl#3284
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Aug 10, 2023
@jay
schannel: verify hostname independent of verify cert
65dbc71 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: curl#3285

Fixes curl#3284
Closes #xxxx
jay added a commit that referenced this issue Aug 11, 2023
@jay
schannel: verify hostname independent of verify cert
889c071 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in #3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: #3285

Fixes #3284
Closes #10056
ptitSeb pushed a commit to wasix-org/curl that referenced this issue Sep 25, 2023
@jay @ptitSeb
schannel: verify hostname independent of verify cert
adc159c 
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in curl#3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: curl#3285

Fixes curl#3284
Closes curl#10056
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jay jay

Labels
TLS
Milestone
No milestone
2 participants
@bagder @jay