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

USE_WIN32_CRYPTO check removed to get USE_NTLM2SESSION set. #3704

Closed
wants to merge 3 commits into from

Conversation

@MonkeybreadSoftware
Copy link
Contributor

MonkeybreadSoftware commented Mar 26, 2019

To fix NTLM fails #3041

@MonkeybreadSoftware

This comment has been minimized.

Copy link
Owner Author

MonkeybreadSoftware commented on fab23df Mar 26, 2019

enable USE_NTLM2SESSION even if USE_WIN32_CRYPTO is defined
for NTLM auth for SMTP, we need USE_NTLM2SESSION set.

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Mar 26, 2019

Thanks!

@bagder bagder closed this in 9130ead Mar 26, 2019
@bagder bagder mentioned this pull request Mar 26, 2019
@bagder

This comment has been minimized.

Copy link
Member

bagder commented Mar 26, 2019

Had to revert this due to #3708

@bagder bagder reopened this Mar 26, 2019
@MonkeybreadSoftware

This comment has been minimized.

Copy link
Contributor Author

MonkeybreadSoftware commented Mar 27, 2019

So we need a check for #ifdef USE_SSL?

I assumed it would be more difficult than just changing one #if

@MarcelRaad

This comment has been minimized.

Copy link
Member

MarcelRaad commented Mar 27, 2019

Something like this maybe?

 lib/curl_ntlm_core.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/curl_ntlm_core.h b/lib/curl_ntlm_core.h
index 07ef5deae..cda6135bd 100644
--- a/lib/curl_ntlm_core.h
+++ b/lib/curl_ntlm_core.h
@@ -50,7 +50,9 @@
 /* Define USE_NTLM2SESSION in order to make the type-3 message include the
    NTLM2Session response message, requires USE_NTRESPONSES defined to 1 and a
    Crypto engine that we have curl_ssl_md5sum() for. */
-#if defined(USE_NTRESPONSES) && !defined(USE_WIN32_CRYPTO)
+#if defined(USE_NTRESPONSES) && \
+  (!defined(USE_WIN32_CRYPTO) || \
+    (defined(USE_SSL) && !defined(CURL_DISABLE_CRYPTO_AUTH)))
 #define USE_NTLM2SESSION
 #endif
@bagder

This comment has been minimized.

Copy link
Member

bagder commented Apr 11, 2019

Looks fine to me. @MonkeybreadSoftware will you amend the PR?

change as discussed
@MonkeybreadSoftware

This comment has been minimized.

Copy link
Contributor Author

MonkeybreadSoftware commented Apr 14, 2019

I changed commit, so lets see what is says.

@bagder

This comment has been minimized.

Copy link
Member

bagder commented May 11, 2019

That had a syntax error. Fix and try again?

@MonkeybreadSoftware

This comment has been minimized.

Copy link
Contributor Author

MonkeybreadSoftware commented May 11, 2019

Well, tests failed due to #if line broken into three lines.
Should we simply change to one line or will that cause trouble with other checks due to line length?

@bagder

This comment has been minimized.

Copy link
Member

bagder commented May 11, 2019

Yes, splitting an #if on multiple lines needs each line to end with a backslash (\) to indicate a continuation.

@MonkeybreadSoftware

This comment has been minimized.

Copy link
Contributor Author

MonkeybreadSoftware commented May 11, 2019

but that is exactly what the change above shows!?
Sorry, I am confused.

@bagder

This comment has been minimized.

Copy link
Member

bagder commented May 11, 2019

Look again at your commit: d0a030e ... it has no backslashes on the end of the two lines that are continued.

added backslashes
@MonkeybreadSoftware

This comment has been minimized.

Copy link
Contributor Author

MonkeybreadSoftware commented May 11, 2019

I was looking at Marcel's diff above. Now I committed the change with the missing backslashes. Sorry.

@stale

This comment has been minimized.

Copy link

stale bot commented Nov 7, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 7, 2019
@MonkeybreadSoftware

This comment has been minimized.

Copy link
Contributor Author

MonkeybreadSoftware commented Nov 7, 2019

and now?
Did this go into a release?

@stale stale bot removed the stale label Nov 7, 2019
@bagder

This comment has been minimized.

Copy link
Member

bagder commented Nov 7, 2019

If it had, it would say so. It has just collected dust here...

@bagder bagder closed this in 93213b2 Nov 7, 2019
@bagder

This comment has been minimized.

Copy link
Member

bagder commented Nov 7, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.