[PATCH 0/1] MR2508: wldap32: Implement setting LDAP_OPT_SSL and add tests.
Windows has the LDAP_OPT_SSL option to turn SSL on or off after calling ldap_init but before connecting to the server. OpenLDAP doesn't have that option and instead expects a fully SSL connection to be requested by passing one or more "ldaps://" URIs to ldap_init. However, OpenLDAP also has an LDAP_OPT_URI option, which Windows does not have, that can be used to change the URIs before connecting. And OpenLDAP already takes care of converting "ldap://" or "ldaps://" to lowercase, so all we have to do is find and replace that exact string in each URI in the list. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54727 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2508
From: Alex Henrie <alexhenrie24(a)gmail.com> Windows has the LDAP_OPT_SSL option to turn SSL on or off after calling ldap_init but before connecting to the server. OpenLDAP doesn't have that option and instead expects a fully SSL connection to be requested by passing one or more "ldaps://" URIs to ldap_init. However, OpenLDAP also has an LDAP_OPT_URI option, which Windows does not have, that can be used to change the URIs before connecting. And OpenLDAP already takes care of converting "ldap://" or "ldaps://" to lowercase, so all we have to do is find and replace that exact string in each URI in the list. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54727 --- dlls/wldap32/option.c | 33 +++++++++++++++++++-- dlls/wldap32/tests/parse.c | 54 ++++++++++++++++++++++++++++++++++ dlls/wldap32/winldap_private.h | 18 ++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/dlls/wldap32/option.c b/dlls/wldap32/option.c index cc85ed1340b..d77cfa843cf 100644 --- a/dlls/wldap32/option.c +++ b/dlls/wldap32/option.c @@ -291,6 +291,7 @@ ULONG CDECL ldap_set_optionA( LDAP *ld, int option, void *value ) case WLDAP32_LDAP_OPT_PROTOCOL_VERSION: case WLDAP32_LDAP_OPT_REFERRALS: case WLDAP32_LDAP_OPT_SIZELIMIT: + case WLDAP32_LDAP_OPT_SSL: case WLDAP32_LDAP_OPT_TIMELIMIT: return ldap_set_optionW( ld, option, value ); @@ -333,7 +334,6 @@ ULONG CDECL ldap_set_optionA( LDAP *ld, int option, void *value ) case WLDAP32_LDAP_OPT_SERVER_ERROR: case WLDAP32_LDAP_OPT_SERVER_EXT_ERROR: case WLDAP32_LDAP_OPT_SIGN: - case WLDAP32_LDAP_OPT_SSL: case WLDAP32_LDAP_OPT_SSL_INFO: case WLDAP32_LDAP_OPT_SSPI_FLAGS: case WLDAP32_LDAP_OPT_TCP_KEEPALIVE: @@ -447,6 +447,36 @@ ULONG CDECL ldap_set_optionW( LDAP *ld, int option, void *value ) case WLDAP32_LDAP_OPT_TIMELIMIT: return map_error( ldap_set_option( CTX(ld), option, value ) ); + case WLDAP32_LDAP_OPT_SSL: + { + BOOL turn_on; + char *uri, *new_uri; + + if (value == WLDAP32_LDAP_OPT_ON) + turn_on = TRUE; + else if (value == WLDAP32_LDAP_OPT_OFF) + turn_on = FALSE; + else if (*(ULONG *)value == 1) + turn_on = TRUE; + else if (*(ULONG *)value == 0) + turn_on = FALSE; + else + return WLDAP32_LDAP_PARAM_ERROR; + + ret = ldap_get_option( CTX(ld), LDAP_OPT_URI, &uri ); + if (ret != LDAP_SUCCESS) return ret; + + if (turn_on) + new_uri = strreplace( uri, "ldap://", "ldaps://" ); + else + new_uri = strreplace( uri, "ldaps://", "ldap://" ); + + ret = map_error( ldap_set_option( CTX(ld), LDAP_OPT_URI, new_uri ) ); + ldap_memfree(uri); + free(new_uri); + return ret; + } + case WLDAP32_LDAP_OPT_CACHE_ENABLE: case WLDAP32_LDAP_OPT_CACHE_FN_PTRS: case WLDAP32_LDAP_OPT_CACHE_STRATEGY: @@ -486,7 +516,6 @@ ULONG CDECL ldap_set_optionW( LDAP *ld, int option, void *value ) case WLDAP32_LDAP_OPT_SERVER_ERROR: case WLDAP32_LDAP_OPT_SERVER_EXT_ERROR: case WLDAP32_LDAP_OPT_SIGN: - case WLDAP32_LDAP_OPT_SSL: case WLDAP32_LDAP_OPT_SSL_INFO: case WLDAP32_LDAP_OPT_SSPI_FLAGS: case WLDAP32_LDAP_OPT_TCP_KEEPALIVE: diff --git a/dlls/wldap32/tests/parse.c b/dlls/wldap32/tests/parse.c index 19e7a48b471..8cd313cd6ca 100644 --- a/dlls/wldap32/tests/parse.c +++ b/dlls/wldap32/tests/parse.c @@ -491,6 +491,59 @@ static void test_ldap_paged_search(void) ldap_unbind( ld ); } +static void test_opt_ssl(void) +{ + LDAP *ld; + ULONG ret, version = LDAP_VERSION3, value; + + /* Turning on LDAP_OPT_SSL without setting the protocol version does not work */ + ld = ldap_initA( (char *)"db.debian.org", 636 ); + ok( ld != NULL, "ldap_init failed\n" ); + ret = ldap_set_optionA( ld, LDAP_OPT_SSL, LDAP_OPT_ON ); + ok( !ret, "ldap_set_optionA should succeed, got %#lx\n", ret ); + ret = ldap_simple_bind_sA( ld, NULL, NULL ); + todo_wine ok( ret == LDAP_PROTOCOL_ERROR, "ldap_simple_bind_sA should fail, got %#lx\n", ret ); + ldap_unbind( ld ); + + /* Setting the protocol version to 3 automatically enables SSL when using port 636 */ + ld = ldap_initA( (char *)"db.debian.org", 636 ); + ok( ld != NULL, "ldap_init failed\n" ); + ret = ldap_set_optionA( ld, LDAP_OPT_PROTOCOL_VERSION, &version ); + ok( !ret, "ldap_set_optionA should succeed, got %#lx\n", ret ); + ret = ldap_set_optionA( ld, LDAP_OPT_SSL, LDAP_OPT_OFF ); + ok( !ret, "ldap_set_optionA should succeed, got %#lx\n", ret ); + ret = ldap_simple_bind_sA( ld, NULL, NULL ); + todo_wine ok( !ret, "ldap_simple_bind_sA should succeed, got %#lx\n", ret ); + ldap_unbind( ld ); + + /* Turning on SSL when the server does not expect it does not work */ + ld = ldap_initA( (char *)"db.debian.org", 389 ); + ok( ld != NULL, "ldap_init failed\n" ); + ret = ldap_set_optionA( ld, LDAP_OPT_SSL, LDAP_OPT_ON ); + ok( !ret, "ldap_set_optionA should succeed, got %#lx\n", ret ); + ret = ldap_simple_bind_sA( ld, NULL, NULL ); + ok( ret == LDAP_SERVER_DOWN, "ldap_simple_bind_sA should fail, got %#lx\n", ret ); + ldap_unbind( ld ); + + /* SSL can also be turned on by passing a pointer to a variable with value 1 */ + ld = ldap_initA( (char *)"db.debian.org", 389 ); + ok( ld != NULL, "ldap_init failed\n" ); + value = 1; + ret = ldap_set_optionA( ld, LDAP_OPT_SSL, &value ); + ok( !ret, "ldap_set_optionA should succeed, got %#lx\n", ret ); + ret = ldap_simple_bind_sA( ld, NULL, NULL ); + ok( ret == LDAP_SERVER_DOWN, "ldap_simple_bind_sA should fail, got %#lx\n", ret ); + ldap_unbind( ld ); + + /* SSL cannot be turned on by passing a pointer to a variable with a bogus value */ + ld = ldap_initA( (char *)"db.debian.org", 389 ); + ok( ld != NULL, "ldap_init failed\n" ); + value = 2; + ret = ldap_set_optionA( ld, LDAP_OPT_SSL, &value ); + ok( ret == LDAP_PARAM_ERROR, "ldap_set_optionA should fail, got %#lx\n", ret ); + ldap_unbind( ld ); +} + START_TEST (parse) { LDAP *ld; @@ -498,6 +551,7 @@ START_TEST (parse) test_ldap_paged_search(); test_ldap_server_control(); test_ldap_bind_sA(); + test_opt_ssl(); ld = ldap_initA( (char *)"db.debian.org", 389 ); ok( ld != NULL, "ldap_init failed\n" ); diff --git a/dlls/wldap32/winldap_private.h b/dlls/wldap32/winldap_private.h index aeeffcfcc15..7adf77f151e 100644 --- a/dlls/wldap32/winldap_private.h +++ b/dlls/wldap32/winldap_private.h @@ -598,6 +598,24 @@ static inline WCHAR *strnAtoW( const char *str, DWORD in_len, DWORD *out_len ) return ret; } +static inline char *strreplace( const char *s, const char *before, const char *after ) +{ + char *ret = malloc( strlen( s ) + strlen( after ) / strlen( before ) + 1 ); + char *cur, *prev = ret; + if (ret) + { + ret[0] = 0; + for (cur = strstr( s, before ); cur; cur = strstr( prev, before )) + { + strncat( ret, prev, cur - prev ); + strcat( ret, after ); + prev = cur + strlen( before ); + } + strncat( ret, prev, cur - prev ); + } + return ret; +} + static inline DWORD bvarraylenW( struct WLDAP32_berval **bv ) { struct WLDAP32_berval **p = bv; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2508
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=131089 Your paranoid android. === build (build log) === error: patch failed: dlls/wldap32/tests/parse.c:498 Task: Patch failed to apply
Hans Leidekker (@hans) commented about dlls/wldap32/option.c:
+ BOOL turn_on; + char *uri, *new_uri; + + if (value == WLDAP32_LDAP_OPT_ON) + turn_on = TRUE; + else if (value == WLDAP32_LDAP_OPT_OFF) + turn_on = FALSE; + else if (*(ULONG *)value == 1) + turn_on = TRUE; + else if (*(ULONG *)value == 0) + turn_on = FALSE; + else + return WLDAP32_LDAP_PARAM_ERROR; + + ret = ldap_get_option( CTX(ld), LDAP_OPT_URI, &uri ); + if (ret != LDAP_SUCCESS) return ret; You should use map_error() here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2508#note_27949
participants (4)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Hans Leidekker (＠hans)
-
Marvin