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

openssl: Fix compilation on Windows when ngtcp2 is enabled #5606

Closed
wants to merge 1 commit into from

Conversation

@jblazquez
Copy link
Contributor

jblazquez commented Jun 24, 2020

The wincrypt.h Windows header defines a number of preprocessor macros that conflict with identically named OpenSSL types. For example, it defines the following macros:

#define X509_NAME                           ((LPCSTR) 7)
#define OCSP_REQUEST                        ((LPCSTR) 66)
#define OCSP_RESPONSE                       ((LPCSTR) 67)

Which conflict with these OpenSSL types and cause compile errors:

typedef struct X509_name_st X509_NAME;
typedef struct ocsp_request_st OCSP_REQUEST;
typedef struct ocsp_response_st OCSP_RESPONSE;

OpenSSL headers already try to avoid these conflicts by undefining these Windows macros. However, that requires that wincrypt.h is included before any OpenSSL header.

In curl's vtls/openssl.c file, there is at least one configuration where an OpenSSL header is included before wincrypt.h. If ngtcp2 is enabled, for example, then the urldata.h include ends up including OpenSSL headers transitively like this:

Note: including file: urldata.h
Note: including file:  quic.h
Note: including file:   vquic/ngtcp2.h
Note: including file:    openssl/ssl.h

That include happens here.

In order to solve these conflicts and fix the compile issues when ngtcp2 is enabled, we need to move the wincrypt.h include as early as possible.

The `wincrypt.h` Windows header defines a number of preprocessor macros that conflict with identically named OpenSSL types. For example, it defines the following macros:

```
#define X509_NAME                           ((LPCSTR) 7)
#define OCSP_REQUEST                        ((LPCSTR) 66)
#define OCSP_RESPONSE                       ((LPCSTR) 67)
```

Which conflict with these OpenSSL types and cause compile errors:

```
typedef struct X509_name_st X509_NAME;
typedef struct ocsp_request_st OCSP_REQUEST;
typedef struct ocsp_response_st OCSP_RESPONSE;
```

OpenSSL headers already try to avoid these conflicts by [undefining these Windows macros](https://github.com/openssl/openssl/blob/15dfa0/include/openssl/types.h#L71-L78). However, that requires that `wincrypt.h` is included _before_ any OpenSSL header.

In curl's `vtls/openssl.c` file, there is at least one configuration where an OpenSSL header is included before `wincrypt.h`. If `ngtcp2` is enabled, for example, then the `urldata.h` include ends up including OpenSSL headers transitively like this:

```
Note: including file: urldata.h
Note: including file:  quic.h
Note: including file:   vquic/ngtcp2.h
Note: including file:    openssl/ssl.h
```

That include happens [here](https://github.com/curl/curl/blob/14c17a/lib/vquic/ngtcp2.h#L32).

In order to solve these conflicts and fix the compile issues when `ngtcp2` is enabled, we need to move the `wincrypt.h` include as early as possible.
@jay
Copy link
Member

jay commented Jun 25, 2020

Thanks

@jblazquez jblazquez deleted the jblazquez:patch-1 branch Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.