You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ruediger Pluem <rp...@apache.org> on 2023/09/19 10:34:17 UTC
Re: POC: updated LDAP support for apr-util 1.7
On 4/18/23 2:26 PM, Graham Leggett via dev wrote:
> Hi all,
>
> The following patch adds ldapi:// (LDAP over unix domain socket) support to apr-util. It is part of a wider cleanup covering the following:
>
> - Add apr_ldap_t to hide the native LDAP type.
> - Add apr_ldap_initialize() with URL support, allowing us to do ldapi://, including proper pool cleanups.
> - Add apr_ldap_get_option_ex() and apr_ldap_set_option_ex() that use apr_ldap_t, and support all options used by httpd.
> - Options passed to apr_ldap_get_option_ex() / apr_ldap_set_option_ex() are a strongly typed union rather than the native void pointers, the assumption being the union is extensible in future.
>
> In theory this extends but does not break our ABI and is safe to go into apr-util 1.7, if this isn’t the case please tell me so I can fix it.
>
> Regards,
> Graham
> —
>
> Index: include/private/apu_internal.h
> ===================================================================
> --- include/private/apu_internal.h (revision 1909133)
> +++ include/private/apu_internal.h (working copy)
> @@ -40,29 +40,6 @@
> apr_status_t apu_dso_load(apr_dso_handle_t **dso, apr_dso_handle_sym_t *dsoptr, const char *module,
> const char *modsym, apr_pool_t *pool);
>
> -#if APR_HAS_LDAP
> -
> -/* For LDAP internal builds, wrap our LDAP namespace */
> -
> -struct apr__ldap_dso_fntable {
> - int (*info)(apr_pool_t *pool, apr_ldap_err_t **result_err);
> - int (*init)(apr_pool_t *pool, LDAP **ldap, const char *hostname,
> - int portno, int secure, apr_ldap_err_t **result_err);
> - int (*ssl_init)(apr_pool_t *pool, const char *cert_auth_file,
> - int cert_file_type, apr_ldap_err_t **result_err);
> - int (*ssl_deinit)(void);
> - int (*get_option)(apr_pool_t *pool, LDAP *ldap, int option,
> - void *outvalue, apr_ldap_err_t **result_err);
> - int (*set_option)(apr_pool_t *pool, LDAP *ldap, int option,
> - const void *invalue, apr_ldap_err_t **result_err);
> - apr_status_t (*rebind_init)(apr_pool_t *pool);
> - apr_status_t (*rebind_add)(apr_pool_t *pool, LDAP *ld,
> - const char *bindDN, const char *bindPW);
> - apr_status_t (*rebind_remove)(LDAP *ld);
> -};
> -
> -#endif /* APR_HAS_LDAP */
> -
Why is this struct removed?
> #ifdef __cplusplus
> }
> #endif
> Index: ldap/apr_ldap_init.c
> ===================================================================
> --- ldap/apr_ldap_init.c (revision 1909133)
> +++ ldap/apr_ldap_init.c (working copy)
> @@ -213,6 +214,96 @@
>
> }
>
> +static apr_status_t ldap_cleanup(void *dptr)
> +{
> + if (dptr) {
> +
> + apr_ldap_t *ldap = dptr;
> +
> + if (ldap->ld) {
> + ldap_unbind(ldap->ld);
> + ldap->ld = NULL;
> + }
> + }
> +
> + return APR_SUCCESS;
> +}
> +
> +/**
> + * APR LDAP initialise function
> + *
> + * This function is responsible for initialising an LDAP
> + * connection in a toolkit independant way. It does the
> + * job of ldap_initialize() from the C api.
> + *
> + * It handles the SSL case, the non-SSL case, and the IPC
> + * case, and attempts to hide the complexity setup from the
> + * user.
> + */
> +APU_DECLARE_LDAP(apr_status_t) apr_ldap_initialize(apr_pool_t *pool,
> + const char *uri,
> + apr_ldap_t **ldap,
> + apr_ldap_err_t *result)
> +{
> + LDAP *ld = NULL;
> + int rc;
> +
> + memset(result, 0, sizeof(*result));
> +
> + *ldap = apr_pcalloc(pool, sizeof(apr_ldap_t));
> + if (!*ldap) {
> + return APR_ENOMEM;
> + }
> +
> +#if APR_HAS_LDAP_INITIALIZE
> +
> + rc = ldap_initialize(&ld, uri);
> +
> +#else
> +
> + {
> + apr_ldap_url_desc_t *urld;
> + apr_status_t status;
> + int secure;
> +
> + status = apr_ldap_url_parse(pool, uri, &(urld), result_err);
Typo? Should be result instead of result_err?
> + if (status != APR_SUCCESS) {
> + return status;
> + }
> +
> + secure = apr_ldap_is_ldaps_url(uri);
> +
> +#if APR_HAS_LDAPSSL_INIT
> + ld = ldapssl_init(urld->lud_host, urld->lud_port, secure);
> +#elif APR_HAS_LDAP_SSLINIT
> + ld = ldap_sslinit((char *)urld->lud_host, urld->lud_port, secure);
> +#else
> + ld = ldap_init((char *)urld->lud_host, urld->lud_port);
> +#endif
> +
> + }
> +
> +#endif
> +
> + if (rc != LDAP_SUCCESS) {
> +
> + result->rc = rc;
> + result->msg = apr_pstrdup(pool, ldap_err2string(result-> rc));
> + result->reason = apr_pstrdup(pool, "LDAP: Could not initialise");
> +
> + return APR_EINVAL;
> + }
> +
> + (*ldap)->ld = ld;
> + (*ldap)->pool = pool;
> +
> + apr_pool_cleanup_register(pool, (*ldap), ldap_cleanup,
> + apr_pool_cleanup_null);
> +
> + return APR_SUCCESS;
> +}
> +
> +
> #if APU_DSO_BUILD
>
> /* For DSO builds, export the table of entry points into the apr_ldap DSO
In the end I think this effort only makes sense if we find a way to get LDAP back into trunk.
I cannot remember if these enhancements would be enough to address the points that caused the LDAP API to be removed from trunk in
r1129809 about 12 years ago.
Regards
Rüdiger
Re: POC: updated LDAP support for apr-util 1.7
Posted by Graham Leggett via dev <de...@apr.apache.org>.
On 19 Sep 2023, at 11:34, Ruediger Pluem <rp...@apache.org> wrote:
>> /* For DSO builds, export the table of entry points into the apr_ldap DSO
>
> In the end I think this effort only makes sense if we find a way to get LDAP back into trunk.
> I cannot remember if these enhancements would be enough to address the points that caused the LDAP API to be removed from trunk in
> r1129809 about 12 years ago.
My understanding was the objection was to the native LDAP type being exposed. This is an effort to no longer expose the native LDAP type.
I am still going work to eliminate the native LDAP type from all the httpd code, but there are too many bugs in the world and not enough hours in the day.
Regards,
Graham
—