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

Add Windows header for CRYPT_E_REVOKED macro #411

Closed
wants to merge 1 commit into from
Closed

Add Windows header for CRYPT_E_REVOKED macro #411

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Sep 4, 2015

Starting with curl 7.44.0 [1], compiling source file lib/strerror.c
with mingw and SSPI enabled, may fail with error:

strerror.c: In function 'Curl_sspi_strerror':
strerror.c:827:10: error: 'CRYPT_E_REVOKED' undeclared (first use in this function)
     case CRYPT_E_REVOKED:
          ^

This happens when using mingw distros that ship with Windows headers
via the w32api package. Such distro is the canonical one, which is
also offered by certain [2] CI platforms. w32api developers implemented
[3] this macro strictly per MSDN [4], solely adding it to wincrypt.h,
This made it incompatible with original Windows SDK headers which define
it in winerror.h. Other mingw distros (f.e. [5]) that use the WINE
headers, work fine.

One solution is to comply with MSDN by #include-ing <wincrypt.h>
via lib/curl_setup.h.

This patch also fixes a TAB indentation.

[1] de74e85
[2] http://www.appveyor.com/docs/installed-software#mingw-msys-cygwin
[3] https://sourceforge.net/u/cstrauss/w32api/ci/3dd1f4b7cb6ab8da80e4c2bbcdb629c8c43b0d6b/
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/aa377167.aspx
[5] https://sourceforge.net/projects/mingw-w64

Starting with curl 7.44.0 [1], compiling source file `lib/strerror.c`
with mingw and SSPI enabled, may fails with error:

```
strerror.c: In function 'Curl_sspi_strerror':
strerror.c:827:10: error: 'CRYPT_E_REVOKED' undeclared (first use in this function)
     case CRYPT_E_REVOKED:
          ^
```

This happens when using mingw distros that ship with Windows headers
via the `w32api` package. Such distro is the canonical one, which is
also used by certain [2] CI platforms. `w32api` developers implemented
[3] this macro strictly per MSDN [4], solely adding it to `wincrypt.h`,
This made it incompatible with official Windows headers which define
it in `winerror.h`. Other mingw distros (f.e. [5]) that use the WINE
headers, work fine.

One solution is to comply with MSDN by #include-ing `<wincrypt.h>`
via `lib/curl_setup.h`.

This patch also fixes a TAB indentation.

[1] de74e85
[2] http://www.appveyor.com/docs/installed-software#mingw-msys-cygwin
[3] https://sourceforge.net/u/cstrauss/w32api/ci/3dd1f4b7cb6ab8da80e4c2bbcdb629c8c43b0d6b/
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/aa377167.aspx
[5] https://sourceforge.net/projects/mingw-w64
@vszakats
Copy link
Member Author

vszakats commented Sep 4, 2015

Not sure why the CI failure, it doesn't seem to be related to this patch, given that it compiles strerror.c fine.

Anyhow an extra guard of defined(WIN32) may be added, though I'd assume USE_WINDOWS_SSPI is never defined on non-Windows platforms.

Or, for extra safety a HAVE_WINCRYPT_H could be defined and used, but (maybe besides certain versions of experimental Tiny C compiler and possibly other very old ones), this header should be present. My experience with autoconf tools is non-existent unfortunately.

@bagder
Copy link
Member

bagder commented Sep 6, 2015

Any objections or comments from @jay or @mback2k ?

@vszakats
Copy link
Member Author

vszakats commented Sep 8, 2015

Here's the error in action on AppVeyor CI:
https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.84#L2061

Here's a successful build run that uses the patch:
https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.74#L2147
with resulting binaries here:
https://bintray.com/vszakats/generic/curl/7.44.0/view

@jay
Copy link
Member

jay commented Sep 9, 2015

It may be better to just define CRYPT_E_REVOKED ourselves. It is the only CRYPT_ code that schannel will return afaict. @vszakats can you try
https://github.com/bagder/curl/compare/master...jay:fix_missing_crypt_e_revoked?expand=1

@vszakats
Copy link
Member Author

vszakats commented Sep 9, 2015

If such solution is okay in curl and it's used already, it's definitely much simpler, in fact this was how I fixed it first, but went the "official" way instead.

Build is okay with the new patch:
https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.86#L2069

jay added a commit that referenced this pull request Sep 10, 2015
@jay
Copy link
Member

jay commented Sep 10, 2015

Thanks, landed in df6a4d3.

@jay jay closed this Sep 10, 2015
@vszakats vszakats deleted the patch-3 branch September 10, 2015 07:41
@vszakats
Copy link
Member Author

Thank you @jay!

jgsogo pushed a commit to jgsogo/curl that referenced this pull request Oct 19, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants