schannel: disable client cert path feature if APIs not available #2522

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@ArchangelSDY
Contributor

ArchangelSDY commented Apr 23, 2018

Original MinGW targets Windows 2000 by default, which lacks some APIs and definitions for this feature. Disable it if required APIs not available.

@mback2k Could you help review that if the __MINGW_H macro is sufficient for the judgement? I know little about MinGW.

@mback2k

This comment has been minimized.

Show comment
Hide comment
@mback2k

mback2k Apr 23, 2018

Member

Please do not disable this feature for all MinGW builds. MinGW-w64 and Msys2 provide updated APIs which include the required definitions, see the other builds here: https://curlbuild.uxnr.de/waterfall (only 2 of them are failing.)

Please check for the required API instead. Another approach would be to add the required definitions if they are missing, like here: https://github.com/curl/curl/blob/master/lib/vtls/schannel.c#L99

Member

mback2k commented Apr 23, 2018

Please do not disable this feature for all MinGW builds. MinGW-w64 and Msys2 provide updated APIs which include the required definitions, see the other builds here: https://curlbuild.uxnr.de/waterfall (only 2 of them are failing.)

Please check for the required API instead. Another approach would be to add the required definitions if they are missing, like here: https://github.com/curl/curl/blob/master/lib/vtls/schannel.c#L99

schannel: disable client cert path feature if required APIs not avail…
…able

Original MinGW targets Windows 2000 by default, which lacks some APIs and
definitions for this feature. Disable it if these APIs are not available.
@ArchangelSDY

This comment has been minimized.

Show comment
Hide comment
@ArchangelSDY

ArchangelSDY Apr 23, 2018

Contributor

OK. Updated to check macro definitions.

Contributor

ArchangelSDY commented Apr 23, 2018

OK. Updated to check macro definitions.

@ArchangelSDY ArchangelSDY changed the title from schannel: disable client cert path feature in MinGW to schannel: disable client cert path feature if APIs not available Apr 23, 2018

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 24, 2018

Member

@mback2k looking like a 👍 for you too?

Member

bagder commented Apr 24, 2018

@mback2k looking like a 👍 for you too?

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Apr 25, 2018

Member

what about convert the hex to binary on our own

Member

jay commented Apr 25, 2018

what about convert the hex to binary on our own

@ArchangelSDY

This comment has been minimized.

Show comment
Hide comment
@ArchangelSDY

ArchangelSDY Apr 25, 2018

Contributor

Well, basically I don't think it's a good idea to re-invent a function in a crypto library. Since MinGW seems not to be actively maintained, MinGW-w64 should be a better choice.

Contributor

ArchangelSDY commented Apr 25, 2018

Well, basically I don't think it's a good idea to re-invent a function in a crypto library. Since MinGW seems not to be actively maintained, MinGW-w64 should be a better choice.

@jay jay closed this in 1592ea9 May 16, 2018

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay May 16, 2018

Member

Thanks

Member

jay commented May 16, 2018

Thanks

@mback2k

This comment has been minimized.

Show comment
Hide comment
@mback2k

mback2k May 16, 2018

Member

Sorry for my absence. Thanks a lot.

Member

mback2k commented May 16, 2018

Sorry for my absence. Thanks a lot.

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder May 16, 2018

Member

I had to polish the schannel code slightly with f0c466d, but it shouldn't cause any problems...

Member

bagder commented May 16, 2018

I had to polish the schannel code slightly with f0c466d, but it shouldn't cause any problems...

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