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

BoringSSL with Schannel #2634

Closed
gvanem opened this Issue Jun 5, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@gvanem
Member

gvanem commented Jun 5, 2018

Compiling libcurl with the combination -DUSE_SCHANNEL, -DHAVE_BORINGSSL and
-DUSE_OPENSSL, breaks in several places because of (from clang-cl):

In file included from vtls/openssl.c:52:
In file included from f:/MingW32/src/inet/Crypto/BoringSSL/include\openssl/ssl.h:145:
f:/MingW32/src/inet/Crypto/BoringSSL/include\openssl/base.h(275,29):  error: expected ')'
typedef struct X509_name_st X509_NAME;
                            ^
f:\ProgramFiler-x86\WindowsKits\Include\10.0.16299.0\um\wincrypt.h(2897,55):  note: expanded from macro 'X509_NAME'
#define X509_NAME                           ((LPCSTR) 7)
                                                      ^
f:/MingW32/src/inet/Crypto/BoringSSL/include\openssl/base.h(275,29):  note: to match this '('
f:\ProgramFiler-x86\WindowsKits\Include\10.0.16299.0\um\wincrypt.h(2897,45):  note: expanded from macro 'X509_NAME'
#define X509_NAME                           ((LPCSTR) 7)
                                            ^

A fix is already in ldap.c:

#ifdef HAVE_BORINGSSL
# undef X509_NAME
# undef X509_CERT_PAIR
# undef X509_EXTENSIONS
#endif

which IMHO should be moved to another place. urldata.h?

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Jun 6, 2018

Member

Yeah, clearly something like that should be done. Did you try it? I'm thinking maybe it will cause problems with vtls/openssl.c ?

Member

bagder commented Jun 6, 2018

Yeah, clearly something like that should be done. Did you try it? I'm thinking maybe it will cause problems with vtls/openssl.c ?

@gvanem

This comment has been minimized.

Show comment
Hide comment
@gvanem

gvanem Jun 7, 2018

Member

I did the #undef to vtls/schannel.h instead. Since that file usually is responsible for pulling in <wincrypt.h>. In ldap.c, it's pulled in via <winldap.h>. And ldap.c never includes any BoringSSL headers AFAICS. So I came up with this patch:

--- a/lib/ldap.c 2018-05-09 19:58:09
+++ b/ldap.c     2018-06-07 11:08:05
@@ -54,15 +54,6 @@
 # endif /* HAVE_LDAP_SSL && HAVE_LDAP_SSL_H */
 #endif

-/* These are macros in both <wincrypt.h> (in above <winldap.h>) and typedefs
- * in BoringSSL's <openssl/x509.h>
- */
-#ifdef HAVE_BORINGSSL
-# undef X509_NAME
-# undef X509_CERT_PAIR
-# undef X509_EXTENSIONS
-#endif
-
 #include "urldata.h"
 #include <curl/curl.h>
 #include "sendf.h"

--- a/lib/vtls/schannel.h 2018-05-22 08:43:49
+++ b/vtls/schannel.h     2018-06-07 10:44:05
@@ -32,6 +32,25 @@

 #include "urldata.h"

+/* <wincrypt.h> has been included via the above <schnlsp.h>.
+ * Or in case of ldap.c, it was included via <winldap.h>.
+ * And since <wincrypt.h> has this:
+ *   #define X509_NAME  ((LPCSTR) 7)
+ *
+ * And in BoringSSL's <openssl/base.h> there is:
+ *  typedef struct X509_name_st X509_NAME;
+ *  etc.
+ *
+ * this wil cause all kinds of C-preprocessing paste errors in
+ * BoringSSL's <openssl/x509.h>: So just undefine those defines here
+ * (and only here).
+ */
+#if defined(HAVE_BORINGSSL) || defined(OPENSSL_IS_BORINGSSL)
+# undef X509_NAME
+# undef X509_CERT_PAIR
+# undef X509_EXTENSIONS
+#endif
+
 extern const struct Curl_ssl Curl_ssl_schannel;

 CURLcode verify_certificate(struct connectdata *conn, int sockindex);

Tested with BoringSSL, SChannel, NgHTTP2, libssh2, RTMP, WinLDAP, WinSSPI etc.

Member

gvanem commented Jun 7, 2018

I did the #undef to vtls/schannel.h instead. Since that file usually is responsible for pulling in <wincrypt.h>. In ldap.c, it's pulled in via <winldap.h>. And ldap.c never includes any BoringSSL headers AFAICS. So I came up with this patch:

--- a/lib/ldap.c 2018-05-09 19:58:09
+++ b/ldap.c     2018-06-07 11:08:05
@@ -54,15 +54,6 @@
 # endif /* HAVE_LDAP_SSL && HAVE_LDAP_SSL_H */
 #endif

-/* These are macros in both <wincrypt.h> (in above <winldap.h>) and typedefs
- * in BoringSSL's <openssl/x509.h>
- */
-#ifdef HAVE_BORINGSSL
-# undef X509_NAME
-# undef X509_CERT_PAIR
-# undef X509_EXTENSIONS
-#endif
-
 #include "urldata.h"
 #include <curl/curl.h>
 #include "sendf.h"

--- a/lib/vtls/schannel.h 2018-05-22 08:43:49
+++ b/vtls/schannel.h     2018-06-07 10:44:05
@@ -32,6 +32,25 @@

 #include "urldata.h"

+/* <wincrypt.h> has been included via the above <schnlsp.h>.
+ * Or in case of ldap.c, it was included via <winldap.h>.
+ * And since <wincrypt.h> has this:
+ *   #define X509_NAME  ((LPCSTR) 7)
+ *
+ * And in BoringSSL's <openssl/base.h> there is:
+ *  typedef struct X509_name_st X509_NAME;
+ *  etc.
+ *
+ * this wil cause all kinds of C-preprocessing paste errors in
+ * BoringSSL's <openssl/x509.h>: So just undefine those defines here
+ * (and only here).
+ */
+#if defined(HAVE_BORINGSSL) || defined(OPENSSL_IS_BORINGSSL)
+# undef X509_NAME
+# undef X509_CERT_PAIR
+# undef X509_EXTENSIONS
+#endif
+
 extern const struct Curl_ssl Curl_ssl_schannel;

 CURLcode verify_certificate(struct connectdata *conn, int sockindex);

Tested with BoringSSL, SChannel, NgHTTP2, libssh2, RTMP, WinLDAP, WinSSPI etc.

bagder added a commit that referenced this issue Jun 8, 2018

@bagder bagder closed this in 274940d Jun 8, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Sep 6, 2018

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