Skip to content

ssl: Fix compilation with OpenSSL 1.x with md4 disabled #14218

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

Closed
wants to merge 1 commit into from

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Jul 18, 2024

If OpenSSL 1.x is used, and it is configured with md4 disabled, OPENSSL_NO_MD4 is defined in opensslconf.h, but this header was not included before checking for this define.

Later in md4.c, openssl/md4.h is included, and it includes that header indirectly, leading to inconsistency within md4.c.

Since the md4.h branch was taken, wincrypt.h (or others) is not included, and later below the USE_WIN32_CRYPTO branch is taken, but the types are not defined.

@orgads
Copy link
Contributor Author

orgads commented Jul 18, 2024

Is the failing test known to be flaky? What exactly failed there?

@vszakats
Copy link
Member

vszakats commented Jul 18, 2024

Yes, all Windows native tests are flaky.

@orgads
Copy link
Contributor Author

orgads commented Jul 18, 2024

Yes, all native tests are flaky.

So should I rerun it? How?

@MarcelRaad
Copy link
Member

So should I rerun it? How?

You can click on the job that failed, and then on "re-run failed jobs" in the menu at the top. I just did that, and enabled debug logging as there was no output at all for the tests, only the failure mark.

Copy link
Member

@MarcelRaad MarcelRaad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this is done in an #else branch for other backends.

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, using an #else there as mentioned by @MarcelRaad makes sense.

If OpenSSL 1.x is used, and it is configured with md4 disabled,
OPENSSL_NO_MD4 is defined in opensslconf.h, but this header was
not included before checking for this define.

Later in md4.c, openssl/md4.h is included, and it includes that
header indirectly, leading to inconsistency within md4.c.

Since the md4.h branch was taken, wincrypt.h (or others) is not
included, and later below the USE_WIN32_CRYPTO branch is taken,
but the types are not defined.
@orgads orgads force-pushed the openssl-1-no-md4 branch from 590c097 to cf00615 Compare July 19, 2024 06:07
@orgads
Copy link
Contributor Author

orgads commented Jul 19, 2024

Done

@orgads orgads requested a review from bagder July 19, 2024 06:15
@bagder bagder closed this in 1f877b0 Jul 19, 2024
@bagder
Copy link
Member

bagder commented Jul 19, 2024

Thanks!

meslubi2021 pushed a commit to Unity-Curl/curl that referenced this pull request Jul 19, 2024
If OpenSSL 1.x is used, and it is configured with md4 disabled,
OPENSSL_NO_MD4 is defined in opensslconf.h, but this header was not
included before checking for this define.

Later in md4.c, openssl/md4.h is included, and it includes that header
indirectly, leading to inconsistency within md4.c.

Since the md4.h branch was taken, wincrypt.h (or others) is not
included, and later below the USE_WIN32_CRYPTO branch is taken, but the
types are not defined.

Closes curl#14218
@orgads orgads deleted the openssl-1-no-md4 branch July 20, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants