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

"schannel: failed to retrieve ALPN result" when running on Windows 7 #840

Closed
tiwoc opened this Issue May 30, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@tiwoc

tiwoc commented May 30, 2016

I did this

We built libcurl with Visual Studio 2015 on Windows Server 2012 R2 (based on Windows 8.1) and linked it statically to an app that downloads a file via HTTP/1.1. This app was then executed on Windows 7. It failed with this error when trying to download this file:

schannel: failed to retrieve ALPN result

(see also #724, where ALPN support for schannel has been introduced)

Suspected root cause

The error occurs when libcurl calls QueryContextAttributes in schannel.c. This function has a minimum supported OS version of Windows 8.1 or Server 2012 R2. It is not guaranteed to work on older versions, like Windows 7 (where it fails when called with SECPKG_ATTR_APPLICATION_PROTOCOL).

I expected the following

The download should have succeeded.

curl/libcurl version

7.49.1 (all is well with 7.48.0)

operating system

  • libcurl was built on Windows Server 2012 R2
  • the build was used on Windows 7

@bagder bagder added the SSL/TLS label May 30, 2016

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder May 30, 2016

Member

I figure this means we shouldn't try to use ALPN on anything before 8.1?

Have you tried to add a check that can switch off conn->bits.tls_enable_alpn if the windows version isn't recent enough?

Member

bagder commented May 30, 2016

I figure this means we shouldn't try to use ALPN on anything before 8.1?

Have you tried to add a check that can switch off conn->bits.tls_enable_alpn if the windows version isn't recent enough?

@tiwoc

This comment has been minimized.

Show comment
Hide comment
@tiwoc

tiwoc May 31, 2016

According to MS, ALPN has been introduced with Windows 8.1. It is not available in Windows 7, so it should be disabled on these systems.

We have not tried to do this depending on the Windows version. What we're trying right now is to set CURLOPT_SSL_ENABLE_ALPN to 0. We'll report the outcome.

tiwoc commented May 31, 2016

According to MS, ALPN has been introduced with Windows 8.1. It is not available in Windows 7, so it should be disabled on these systems.

We have not tried to do this depending on the Windows version. What we're trying right now is to set CURLOPT_SSL_ENABLE_ALPN to 0. We'll report the outcome.

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder May 31, 2016

Member

we're trying right now is to set CURLOPT_SSL_ENABLE_ALPN to 0

Yeah, that sounds like a functional work-around. Another one would be to rebuild libcurl with a TLS backend that supports ALPN on win 7 as well, but if you can live without APLN I guess your approach is the easier one.

We should still get a fix for this done.

Member

bagder commented May 31, 2016

we're trying right now is to set CURLOPT_SSL_ENABLE_ALPN to 0

Yeah, that sounds like a functional work-around. Another one would be to rebuild libcurl with a TLS backend that supports ALPN on win 7 as well, but if you can live without APLN I guess your approach is the easier one.

We should still get a fix for this done.

@webmaster128

This comment has been minimized.

Show comment
Hide comment
@webmaster128

webmaster128 May 31, 2016

Contributor

Confirm everything above. I've got a minimal test case running that shows the following result

Operating system unpatched patched
Windows Server 2012 R2 ok ok
Windows 7 fail ok

where patched means adding the following in our application code:

#ifdef _WIN32
    // Disable ALPN which is not supported on Windows < 8.1
    // https://msdn.microsoft.com/en-us/library/windows/desktop/aa379340%28v=vs.85%29.aspx
    curlEasy_->add<CURLOPT_SSL_ENABLE_ALPN>(0);
#endif

all builds are done using MSVS 2015 on Windows Server 2012 R2.


Given we have a nice workaround now, the question is weather or not this compile time feature check is sufficient. This causes the default behavior that code compiled on Windows 8+ does not run on Windows 7 (and below).

Contributor

webmaster128 commented May 31, 2016

Confirm everything above. I've got a minimal test case running that shows the following result

Operating system unpatched patched
Windows Server 2012 R2 ok ok
Windows 7 fail ok

where patched means adding the following in our application code:

#ifdef _WIN32
    // Disable ALPN which is not supported on Windows < 8.1
    // https://msdn.microsoft.com/en-us/library/windows/desktop/aa379340%28v=vs.85%29.aspx
    curlEasy_->add<CURLOPT_SSL_ENABLE_ALPN>(0);
#endif

all builds are done using MSVS 2015 on Windows Server 2012 R2.


Given we have a nice workaround now, the question is weather or not this compile time feature check is sufficient. This causes the default behavior that code compiled on Windows 8+ does not run on Windows 7 (and below).

@tiwoc

This comment has been minimized.

Show comment
Hide comment
@tiwoc

tiwoc May 31, 2016

I'd like to make clear that we use Windows Server 2012 R2, which is based on Windows 8.1 and which is the oldest version that supports ALPN. Windows Server 2012 (without R2) is based on Windows 8 and does not support ALPN.

tiwoc commented May 31, 2016

I'd like to make clear that we use Windows Server 2012 R2, which is based on Windows 8.1 and which is the oldest version that supports ALPN. Windows Server 2012 (without R2) is based on Windows 8 and does not support ALPN.

@tiwoc

This comment has been minimized.

Show comment
Hide comment
@tiwoc

tiwoc May 31, 2016

Given we have a nice workaround now, the question is weather or not this compile time feature check is sufficient.

It is not. There should be a runtime check in the schannel TLS code that disables ALPN if it is running on Windows < 8.1. I could have a shot at implementing this in the next few days, but I'm not sure that it will be today or tomorrow. @bagder Do you intend to release a bug fix version when this is done?

tiwoc commented May 31, 2016

Given we have a nice workaround now, the question is weather or not this compile time feature check is sufficient.

It is not. There should be a runtime check in the schannel TLS code that disables ALPN if it is running on Windows < 8.1. I could have a shot at implementing this in the next few days, but I'm not sure that it will be today or tomorrow. @bagder Do you intend to release a bug fix version when this is done?

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder May 31, 2016

Member

Do you intend to release a bug fix version when this is done?

Sure, once we have a bug fix it'll get merged and should be included in the pending next release.

Member

bagder commented May 31, 2016

Do you intend to release a bug fix version when this is done?

Sure, once we have a bug fix it'll get merged and should be included in the pending next release.

tiwoc added a commit to tiwoc/curl that referenced this issue May 31, 2016

schannel: Disable ALPN on Windows < 8.1
Calling QueryContextAttributes with SECPKG_ATTR_APPLICATION_PROTOCOL
fails on Windows < 8.1, so we need to disable ALPN on these OS versions.

Fixes #840

tiwoc added a commit to tiwoc/curl that referenced this issue Jun 1, 2016

schannel: Disable ALPN on Windows < 8.1
Calling QueryContextAttributes with SECPKG_ATTR_APPLICATION_PROTOCOL
fails on Windows < 8.1, so we need to disable ALPN on these OS versions.

Fixes #840
@JDepooter

This comment has been minimized.

Show comment
Hide comment
@JDepooter

JDepooter Jun 1, 2016

Contributor

My apologies for the regression here. I was fairly certain I had tested this on Windows 7 when I implemented #724, but apparently did not.

Contributor

JDepooter commented Jun 1, 2016

My apologies for the regression here. I was fairly certain I had tested this on Windows 7 when I implemented #724, but apparently did not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.