Yubico / libfido2

Provides library functionality for FIDO2, including communication with a device over USB or NFC.
Other
590 stars 152 forks source link

Avoids openssl name clashes on Windows #673

Closed James-ZHANG closed 1 year ago

James-ZHANG commented 1 year ago

See https://github.com/openssl/openssl/issues/1093 for the motivation.

LDVG commented 1 year ago

Sorry, I'm not sure I follow the problem as outlined by the linked issue. I was under the impression that these #undef directives are included in OpenSSL's own header files (e.g. ossl_typ.h in 1.1.1 stable). Do you have an example of an environment in which our build fails?

James-ZHANG commented 1 year ago

I built libfido2 on Cygwin, and Cygwin's gcc does not define _WIN32: https://cygwin.com/pipermail/cygwin-developers/2012-July/010732.html.

Maybe the proper fix is to upstream the __CYGWIN__ check to OpenSSL, then?

LDVG commented 1 year ago

Thank you for the clarification. I'm still unsure exactly how this problem materializes in your build.

From my understanding, these symbols are defined in wincrypt.h which is included by windows.h unless WIN32_LEAN_AND_MEAN is defined. libfido2 defines WIN32_LEAN_AND_MEAN for Cygwin builds so there should not be any name clashes:

https://github.com/Yubico/libfido2/blob/fe5aa1d2ee1e698345b56feef9325cd44c45d9c2/CMakeLists.txt#L60-L66

Am I missing something?

James-ZHANG commented 1 year ago

Ah, that's exactly what caused my build to fail: we use a constrained build environment where we do not have cmake in our Cygwin and thus have to use a custom Makefile to build libfido2. Naturally, we missed this part in the upstream.

After adding -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=0x0600 in our Makefile my build works.

Closing this PR. Thanks for your pointer :).