You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by "Paul J. Reder" <re...@remulak.net> on 2007/11/15 02:02:27 UTC
[Patch]: Add basic function to APR for LDAP rebind callback support.
Back in July I submitted a pair of alternative patches with the intent of
providing LDAP rebind callback support when chasing referrals with MSAD 2003+
utilized as the LDAP server and Apache+mod_ldap as the client. I provided
both an Apache-only solution and a mixed APR+Apache solution. The Apache only
solution was provided in an effort to speed the process. At the time there
was no discussion of the patches.
I believe the mixed solution is the more appropriate one, so I am resubmitting
only that one in parts. The attached link provides the APR portion of the patch
and must be applied/committed to trunk before the Apache portion of the code
can be committed.
The APR portion of the patch:
http://people.apache.org/~rederpj/APR-trunk_rebind.diff
I don't have karma to commit the APR portion, but I can't proceed with the
Apache portion until the APR part has been committed.
For reference (and being submitted separately to Apache dev) here is the
Apache portion of the patch.
http://people.apache.org/~rederpj/Apache-trunk_rebind.diff
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Paul J. Reder wrote:
> I addressed the comments and haven't heard back from anyone
> (Bojan or others). I can't commit to APR and can't commit the
> Apache portion until the APR part has been committed. I know
> folks were busy with the latest APR update... Has the dust
> settled?
My own attention is distracted with httpd 2.2.7 and mod_ftp
(along with other places) but I'd love to see this addressed
before we call 1.3.0 baked.
Bill
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by Graham Leggett <mi...@sharp.fm>.
Paul J. Reder wrote:
> Your changes look good to me (assuming the ldap struct does get passed
> in to
> the cleanup). I made a few minor alterations to your patch and attached it
> here. I like the simplification of automatically registering the callback
> and better name space protection.
>
> I'm a little concerned about the pool cleanup though. I'll probably still
> explicitly call the remove because I'm concerned that in a busy system
> there
> could be a window where the ldap struct is freed and reused by another
> request
> before the first request processing is completed and the cleanup is
> paged in
> and run. In that case the old xref might be found (even though the new
> one will
> likely have been added). Calling the remove explicitly allows the xref
> to be
> removed as soon as the ldap struct is no longer required rather than
> waiting
> for the rest of the HTTP processing to happen before the pool is cleaned
> up.
I looked at this scenario while I was adding the cleanup, and managed to
talk myself out of it being possible. Now you have talked me back into
it again :)
Thinking about it again...
The line of thinking that I took, was to confirm whether it is
reasonable for the LDAP structure to outlive the pool used when the
thing was created, but then the LDAP structure didn't come from the
pool, and we don't register a cleanup for that (as I recall), so to be
consistent, we shouldn't make a cleanup at all. Will change it.
Regards,
Graham
--
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by "Paul J. Reder" <re...@remulak.net>.
That resolves my concerns and appears to provide the best of both
worlds (allows for timely cleanup to avoid re-use timing issues as well as
provide a safety net to make sure it gets cleaned up eventually - just in case).
Graham Leggett wrote:
> Graham Leggett wrote:
>
>> Can you try this version?
>>
>> I have asked the _remove function to kill the cleanup that was
>> registered, which should make this version safe, as well as
>> guaranteeing that the rebind will never outlive the pool that created it.
>
> Oops, let me try that again.
>
> Regards,
> Graham
> --
>
>
> ------------------------------------------------------------------------
>
> Index: ldap/apr_ldap_rebind.c
> ===================================================================
> --- ldap/apr_ldap_rebind.c (revision 0)
> +++ ldap/apr_ldap_rebind.c (revision 0)
> @@ -0,0 +1,266 @@
> +/* Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +/* apr_ldap_rebind.c -- LDAP rebind callbacks for referrals
> + *
> + * The LDAP SDK allows a callback to be set to enable rebinding
> + * for referral processing.
> + *
> + */
> +
> +#include "apr.h"
> +#include "apu.h"
> +#include "apr_ldap.h"
> +#include "apr_errno.h"
> +#include "apr_strings.h"
> +#include "apr_ldap_rebind.h"
> +
> +#include "stdio.h"
> +
> +#if APR_HAS_THREADS
> +static apr_thread_mutex_t *apr_ldap_xref_lock = NULL;
> +#endif
> +
> +/* Used to store information about connections for use in the referral rebind callback. */
> +struct apr_ldap_rebind_entry {
> + apr_pool_t *pool;
> + LDAP *index;
> + const char *bindDN;
> + const char *bindPW;
> + struct apr_ldap_rebind_entry *next;
> +};
> +typedef struct apr_ldap_rebind_entry apr_ldap_rebind_entry_t;
> +
> +static apr_ldap_rebind_entry_t *xref_head = NULL;
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld);
> +static apr_status_t apr_ldap_rebind_remove_helper(void *data);
> +
> +/* APR utility routine used to create the xref_lock. */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_init(apr_pool_t *pool)
> +{
> + apr_status_t retcode = APR_SUCCESS;
> +
> +#if APR_HAS_THREADS
> + if (apr_ldap_xref_lock == NULL) {
> + retcode = apr_thread_mutex_create(&apr_ldap_xref_lock, APR_THREAD_MUTEX_DEFAULT, pool);
> + }
> +#endif
> +
> + return(retcode);
> +}
> +
> +
> +/*************************************************************************************/
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_add(apr_pool_t *pool, LDAP *ld, const char *bindDN, const char *bindPW)
> +{
> + apr_status_t retcode = APR_SUCCESS;
> + apr_ldap_rebind_entry_t *new_xref;
> +
> + new_xref = (apr_ldap_rebind_entry_t *)apr_pcalloc(pool, sizeof(apr_ldap_rebind_entry_t));
> + if (new_xref) {
> + new_xref->pool = pool;
> + new_xref->index = ld;
> + if (bindDN) {
> + new_xref->bindDN = apr_pstrdup(pool, bindDN);
> + }
> + if (bindPW) {
> + new_xref->bindPW = apr_pstrdup(pool, bindPW);
> + }
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(apr_ldap_xref_lock);
> +#endif
> +
> + new_xref->next = xref_head;
> + xref_head = new_xref;
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_unlock(apr_ldap_xref_lock);
> +#endif
> + }
> + else {
> + return(APR_ENOMEM);
> + }
> +
> + retcode = apr_ldap_rebind_set_callback(ld);
> + if (APR_SUCCESS != retcode) {
> + apr_ldap_rebind_remove(ld);
> + return retcode;
> + }
> +
> + apr_pool_cleanup_register(pool, ld,
> + apr_ldap_rebind_remove_helper,
> + apr_pool_cleanup_null);
> +
> + return(APR_SUCCESS);
> +}
> +
> +/*************************************************************************************/
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_remove(LDAP *ld)
> +{
> + apr_ldap_rebind_entry_t *tmp_xref, *prev = NULL;
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(apr_ldap_xref_lock);
> +#endif
> + tmp_xref = xref_head;
> +
> + while ((tmp_xref) && (tmp_xref->index != ld)) {
> + prev = tmp_xref;
> + tmp_xref = tmp_xref->next;
> + }
> +
> + if (tmp_xref) {
> + if (tmp_xref == xref_head) {
> + xref_head = xref_head->next;
> + }
> + else {
> + prev->next = tmp_xref->next;
> + }
> +
> + /* tmp_xref and its contents were pool allocated so they don't need to be freed here. */
> +
> + /* remove the cleanup, just in case this was done manually */
> + apr_pool_cleanup_kill(tmp_xref->pool, tmp_xref->index,
> + apr_ldap_rebind_remove_helper);
> + }
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_unlock(apr_ldap_xref_lock);
> +#endif
> + return APR_SUCCESS;
> +}
> +
> +static apr_status_t apr_ldap_rebind_remove_helper(void *data)
> +{
> + LDAP *ld = (LDAP *)data;
> + apr_ldap_rebind_remove(ld);
> + return APR_SUCCESS;
> +}
> +
> +/*************************************************************************************/
> +static apr_ldap_rebind_entry_t *apr_ldap_rebind_lookup(LDAP *ld)
> +{
> + apr_ldap_rebind_entry_t *tmp_xref, *match = NULL;
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(apr_ldap_xref_lock);
> +#endif
> + tmp_xref = xref_head;
> +
> + while (tmp_xref) {
> + if (tmp_xref->index == ld) {
> + match = tmp_xref;
> + tmp_xref = NULL;
> + }
> + else {
> + tmp_xref = tmp_xref->next;
> + }
> + }
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_unlock(apr_ldap_xref_lock);
> +#endif
> +
> + return (match);
> +}
> +
> +#if APR_HAS_TIVOLI_LDAPSDK
> +
> +/* LDAP_rebindproc() Tivoli LDAP style
> + * Rebind callback function. Called when chasing referrals. See API docs.
> + * ON ENTRY:
> + * ld Pointer to an LDAP control structure. (input only)
> + * binddnp Pointer to an Application DName used for binding (in *or* out)
> + * passwdp Pointer to the password associated with the DName (in *or* out)
> + * methodp Pointer to the Auth method (output only)
> + * freeit Flag to indicate if this is a lookup or a free request (input only)
> + */
> +static int LDAP_rebindproc(LDAP *ld, char **binddnp, char **passwdp, int *methodp, int freeit)
> +{
> + if (!freeit) {
> + apr_ldap_rebind_entry_t *my_conn;
> +
> + *methodp = LDAP_AUTH_SIMPLE;
> + my_conn = apr_ldap_rebind_lookup(ld);
> +
> + if ((my_conn) && (my_conn->bindDN != NULL)) {
> + *binddnp = strdup(my_conn->bindDN);
> + *passwdp = strdup(my_conn->bindPW);
> + } else {
> + *binddnp = NULL;
> + *passwdp = NULL;
> + }
> + } else {
> + if (*binddnp) {
> + free(*binddnp);
> + }
> + if (*passwdp) {
> + free(*passwdp);
> + }
> + }
> +
> + return LDAP_SUCCESS;
> +}
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld)
> +{
> + ldap_set_rebind_proc(ld, (LDAPRebindProc)LDAP_rebindproc);
> + return APR_SUCCESS;
> +}
> +
> +#elif APR_HAS_OPENLDAP_LDAPSDK
> +
> +/* LDAP_rebindproc() openLDAP V3 style
> + * ON ENTRY:
> + * ld Pointer to an LDAP control structure. (input only)
> + * url Unused in this routine
> + * request Unused in this routine
> + * msgid Unused in this routine
> + * params Unused in this routine
> + */
> +static int LDAP_rebindproc(LDAP *ld, LDAP_CONST char *url, ber_tag_t request, ber_int_t msgid, void *params)
> +{
> + apr_ldap_rebind_entry_t *my_conn;
> + const char *bindDN = NULL;
> + const char *bindPW = NULL;
> +
> + my_conn = apr_ldap_rebind_lookup(ld);
> +
> + if ((my_conn) && (my_conn->bindDN != NULL)) {
> + bindDN = my_conn->bindDN;
> + bindPW = my_conn->bindPW;
> + }
> +
> + return (ldap_bind_s(ld, bindDN, bindPW, LDAP_AUTH_SIMPLE));
> +}
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld)
> +{
> + ldap_set_rebind_proc(ld, LDAP_rebindproc, NULL);
> + return APR_SUCCESS;
> +}
> +
> +#else /* Implementation not recognised */
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld)
> +{
> + return APR_ENOTIMPL;
> +}
> +
> +#endif
> +
> Index: ldap/NWGNUmakefile
> ===================================================================
> --- ldap/NWGNUmakefile (revision 599173)
> +++ ldap/NWGNUmakefile (working copy)
> @@ -231,6 +231,7 @@
> $(OBJDIR)/apr_ldap_init.o \
> $(OBJDIR)/apr_ldap_option.o \
> $(OBJDIR)/apr_ldap_url.o \
> + $(OBJDIR)/apr_ldap_rebind.o \
> $(EOLIST)
>
> #
> Index: include/apr_ldap_rebind.h
> ===================================================================
> --- include/apr_ldap_rebind.h (revision 0)
> +++ include/apr_ldap_rebind.h (revision 0)
> @@ -0,0 +1,80 @@
> +/* Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +/**
> + * The APR LDAP rebind functions provide an implementation of
> + * a rebind procedure that can be used to allow clients to chase referrals,
> + * using the same credentials used to log in originally.
> + *
> + * Use of this implementation is optional.
> + *
> + * @file apu_ldap_rebind.h
> + * @brief Apache LDAP library
> + */
> +
> +#ifndef APU_LDAP_REBIND_H
> +#define APU_LDAP_REBIND_H
> +
> +/**
> + * APR LDAP initialize rebind lock
> + *
> + * This function creates the lock for controlling access to the xref list..
> + * @param pool Pool to use when creating the xref_lock.
> + */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_init(apr_pool_t *pool);
> +
> +
> +/**
> + * APR LDAP rebind_add function
> + *
> + * This function creates a cross reference entry for the specified ldap
> + * connection. The rebind callback function will look up this ldap
> + * connection so it can retrieve the bindDN and bindPW for use in any
> + * binds while referrals are being chased.
> + *
> + * This function will add the callback to the LDAP handle passed in.
> + *
> + * A cleanup is registered within the pool provided to remove this
> + * entry when the pool is removed. Alternatively apr_ldap_rebind_remove()
> + * can be called to explicitly remove the entry at will.
> + *
> + * @param pool The pool to use
> + * @param ld The LDAP connectionhandle
> + * @param bindDN The bind DN to be used for any binds while chasing
> + * referrals on this ldap connection.
> + * @param bindPW The bind Password to be used for any binds while
> + * chasing referrals on this ldap connection.
> + */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_add(apr_pool_t *pool,
> + LDAP *ld,
> + const char *bindDN,
> + const char *bindPW);
> +
> +/**
> + * APR LDAP rebind_remove function
> + *
> + * This function removes the rebind cross reference entry for the
> + * specified ldap connection.
> + *
> + * If not explicitly removed, this function will be called automatically
> + * when the pool is cleaned up.
> + *
> + * @param ld The LDAP connectionhandle
> + */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_remove(LDAP *ld);
> +
> +#endif /* APU_LDAP_REBIND_H */
> +
> Index: include/apr_ldap.h.in
> ===================================================================
> --- include/apr_ldap.h.in (revision 599173)
> +++ include/apr_ldap.h.in (working copy)
> @@ -154,6 +154,7 @@
> #include "apr_ldap_url.h"
> #include "apr_ldap_init.h"
> #include "apr_ldap_option.h"
> +#include "apr_ldap_rebind.h"
>
> /** @} */
> #endif /* APR_HAS_LDAP */
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by Graham Leggett <mi...@sharp.fm>.
Graham Leggett wrote:
> Can you try this version?
>
> I have asked the _remove function to kill the cleanup that was
> registered, which should make this version safe, as well as guaranteeing
> that the rebind will never outlive the pool that created it.
Oops, let me try that again.
Regards,
Graham
--
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by Graham Leggett <mi...@sharp.fm>.
Paul J. Reder wrote:
> Your changes look good to me (assuming the ldap struct does get passed
> in to
> the cleanup). I made a few minor alterations to your patch and attached it
> here. I like the simplification of automatically registering the callback
> and better name space protection.
Can you try this version?
I have asked the _remove function to kill the cleanup that was
registered, which should make this version safe, as well as guaranteeing
that the rebind will never outlive the pool that created it.
Regards,
Graham
--
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by "Paul J. Reder" <re...@remulak.net>.
Graham,
Your changes look good to me (assuming the ldap struct does get passed in to
the cleanup). I made a few minor alterations to your patch and attached it
here. I like the simplification of automatically registering the callback
and better name space protection.
I'm a little concerned about the pool cleanup though. I'll probably still
explicitly call the remove because I'm concerned that in a busy system there
could be a window where the ldap struct is freed and reused by another request
before the first request processing is completed and the cleanup is paged in
and run. In that case the old xref might be found (even though the new one will
likely have been added). Calling the remove explicitly allows the xref to be
removed as soon as the ldap struct is no longer required rather than waiting
for the rest of the HTTP processing to happen before the pool is cleaned up.
Thanks for the review and effort to commit this.
Paul J. Reder
Graham Leggett wrote:
> Paul J. Reder wrote:
>
>> I addressed the comments and haven't heard back from anyone
>> (Bojan or others). I can't commit to APR and can't commit the
>> Apache portion until the APR part has been committed. I know
>> folks were busy with the latest APR update... Has the dust
>> settled?
>
> Looking at this in more detail.
>
> It took me a while to figure out exactly what the rebind code was trying
> to do (as opposed to generally knowing what it does), and it seems to be
> "an implementation of a callback mechanism able to take advantage of the
> LDAP referral callback feature".
>
> Or in other words, use of this particular API is optional, someone using
> the APR interface may choose to use this particular implementation, or
> they may choose some other implementation of their own.
>
> Based on this, I think the API should all be in a namespace like
> apr_ldap_rebind.
>
> Looking further, unless I am missing something, I think this could
> probably be significantly simplified.
>
> In theory, the apr_ldap_set_rebind_callback() function can be called by
> apr_ldap_xref_add(), hiding apr_ldap_set_rebind_callback().
>
> In theory, there should be a way to register a pool cleanup for
> apr_ldap_rebind_remove() as well.
>
> I have updated the patch, which is attached - will this do?
>
> Regards,
> Graham
> --
>
>
> ------------------------------------------------------------------------
>
> Index: ldap/apr_ldap_rebind.c
> ===================================================================
> --- ldap/apr_ldap_rebind.c (revision 0)
> +++ ldap/apr_ldap_rebind.c (revision 0)
> @@ -0,0 +1,251 @@
> +/* Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +/* apr_ldap_rebind.c -- LDAP rebind callbacks for referrals
> + *
> + * The LDAP SDK allows a callback to be set to enable rebinding
> + * for referral processing.
> + *
> + */
> +
> +#include "apr.h"
> +#include "apu.h"
> +#include "apr_ldap.h"
> +#include "apr_errno.h"
> +#include "apr_strings.h"
> +#include "apr_ldap_rebind.h"
> +
> +#include "stdio.h"
> +
> +#if APR_HAS_THREADS
> +static apr_thread_mutex_t *apr_ldap_xref_lock = NULL;
> +#endif
> +
> +/* Used to store information about connections for use in the referral rebind callback. */
> +struct apr_ldap_rebind_entry {
> + LDAP *index;
> + const char *bindDN;
> + const char *bindPW;
> + struct apr_ldap_rebind_entry *next;
> +};
> +typedef struct apr_ldap_rebind_entry apr_ldap_rebind_entry_t;
> +
> +static apr_ldap_rebind_entry_t *xref_head = NULL;
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld);
> +static apr_status_t apr_ldap_rebind_remove_helper(void *data);
> +
> +/* APR utility routine used to create the xref_lock. */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_init(apr_pool_t *pool)
> +{
> + apr_status_t retcode = APR_SUCCESS;
> +
> +#if APR_HAS_THREADS
> + if (apr_ldap_xref_lock == NULL) {
> + retcode = apr_thread_mutex_create(&apr_ldap_xref_lock, APR_THREAD_MUTEX_DEFAULT, pool);
> + }
> +#endif
> +
> + return(retcode);
> +}
> +
> +
> +/*************************************************************************************/
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_add(apr_pool_t *pool, LDAP *ld, const char *bindDN, const char *bindPW)
> +{
> + apr_status_t retcode = APR_SUCCESS;
> + apr_ldap_rebind_entry_t *new_xref;
> +
> + new_xref = (apr_ldap_rebind_entry_t *)apr_pcalloc(pool, sizeof(apr_ldap_rebind_entry_t));
> + if (new_xref) {
> + new_xref->index = ld;
> + if (bindDN) {
> + new_xref->bindDN = apr_pstrdup(pool, bindDN);
> + }
> + if (bindPW) {
> + new_xref->bindPW = apr_pstrdup(pool, bindPW);
> + }
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(apr_ldap_xref_lock);
> +#endif
> +
> + new_xref->next = xref_head;
> + xref_head = new_xref;
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_unlock(apr_ldap_xref_lock);
> +#endif
> + }
> + else {
> + return(APR_ENOMEM);
> + }
> +
> + retcode = apr_ldap_rebind_set_callback(ld);
> + if (APR_SUCCESS != retcode) {
> + apr_ldap_rebind_remove(ld);
> + return retcode;
> + }
> +
> + apr_pool_cleanup_register(pool, ld,
> + apr_ldap_rebind_remove_helper,
> + apr_pool_cleanup_null);
> +
> + return(APR_SUCCESS);
> +}
> +
> +/*************************************************************************************/
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_remove(LDAP *ld)
> +{
> + apr_ldap_rebind_entry_t *tmp_xref, *prev = NULL;
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(apr_ldap_xref_lock);
> +#endif
> + tmp_xref = xref_head;
> +
> + while ((tmp_xref) && (tmp_xref->index != ld)) {
> + prev = tmp_xref;
> + tmp_xref = tmp_xref->next;
> + }
> +
> + if (tmp_xref) {
> + if (tmp_xref == xref_head) {
> + xref_head = xref_head->next;
> + }
> + else {
> + prev->next = tmp_xref->next;
> + }
> + /* tmp_xref and its contents were pool allocated so they don't need to be freed here. */
> + }
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_unlock(apr_ldap_xref_lock);
> +#endif
> + return APR_SUCCESS;
> +}
> +
> +static apr_status_t apr_ldap_rebind_remove_helper(void *data)
> +{
> + LDAP *ld = (LDAP *)data;
> + apr_ldap_rebind_remove(ld);
> + return APR_SUCCESS;
> +}
> +
> +/*************************************************************************************/
> +static apr_ldap_rebind_entry_t *apr_ldap_rebind_lookup(LDAP *ld)
> +{
> + apr_ldap_rebind_entry_t *tmp_xref, *match = NULL;
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(apr_ldap_xref_lock);
> +#endif
> + tmp_xref = xref_head;
> +
> + while (tmp_xref) {
> + if (tmp_xref->index == ld) {
> + match = tmp_xref;
> + tmp_xref = NULL;
> + }
> + else {
> + tmp_xref = tmp_xref->next;
> + }
> + }
> +
> +#if APR_HAS_THREADS
> + apr_thread_mutex_unlock(apr_ldap_xref_lock);
> +#endif
> +
> + return (match);
> +}
> +
> +/* LDAP_rebindproc() ITDS style
> + * Rebind callback function. Called when chasing referrals. See API docs.
> + * ON ENTRY:
> + * ld Pointer to an LDAP control structure. (input only)
> + * binddnp Pointer to an Application DName used for binding (in *or* out)
> + * passwdp Pointer to the password associated with the DName (in *or* out)
> + * methodp Pointer to the Auth method (output only)
> + * freeit Flag to indicate if this is a lookup or a free request (input only)
> + */
> +#if APR_HAS_TIVOLI_LDAPSDK
> +static int LDAP_rebindproc(LDAP *ld, char **binddnp, char **passwdp, int *methodp, int freeit)
> +{
> + if (!freeit) {
> + apr_ldap_rebind_entry_t *my_conn;
> +
> + *methodp = LDAP_AUTH_SIMPLE;
> + my_conn = apr_ldap_xref_lookup(ld);
> +
> + if ((my_conn) && (my_conn->bindDN != NULL)) {
> + *binddnp = strdup(my_conn->bindDN);
> + *passwdp = strdup(my_conn->bindPW);
> + } else {
> + *binddnp = NULL;
> + *passwdp = NULL;
> + }
> + } else {
> + if (*binddnp) {
> + free(*binddnp);
> + }
> + if (*passwdp) {
> + free(*passwdp);
> + }
> + }
> +
> + return LDAP_SUCCESS;
> +}
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld)
> +{
> + ldap_set_rebind_proc(ld, (LDAPRebindProc)LDAP_rebindproc);
> + return APR_SUCCESS;
> +}
> +
> +#elif APR_HAS_OPENLDAP_LDAPSDK
> +
> +/* LDAP_rebindproc() openLDAP V3 style */
> +static int LDAP_rebindproc(LDAP *ld, LDAP_CONST char *url, ber_tag_t request, ber_int_t msgid, void *params)
> +{
> + apr_ldap_rebind_entry_t *my_conn;
> + const char *bindDN = NULL;
> + const char *bindPW = NULL;
> +
> + my_conn = apr_ldap_rebind_lookup(ld);
> +
> + if ((my_conn) && (my_conn->bindDN != NULL)) {
> + bindDN = my_conn->bindDN;
> + bindPW = my_conn->bindPW;
> + }
> +
> + return (ldap_bind_s(ld, bindDN, bindPW, LDAP_AUTH_SIMPLE));
> +}
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld)
> +{
> + ldap_set_rebind_proc(ld, LDAP_rebindproc, NULL);
> + return APR_SUCCESS;
> +}
> +
> +#else /* Implementation not recognised */
> +
> +static int apr_ldap_rebind_set_callback(LDAP *ld)
> +{
> + return APR_ENOTIMPL;
> +}
> +
> +#endif
> +
> Index: ldap/NWGNUmakefile
> ===================================================================
> --- ldap/NWGNUmakefile (revision 599173)
> +++ ldap/NWGNUmakefile (working copy)
> @@ -231,6 +231,7 @@
> $(OBJDIR)/apr_ldap_init.o \
> $(OBJDIR)/apr_ldap_option.o \
> $(OBJDIR)/apr_ldap_url.o \
> + $(OBJDIR)/apr_ldap_rebind.o \
> $(EOLIST)
>
> #
> Index: include/apr_ldap_rebind.h
> ===================================================================
> --- include/apr_ldap_rebind.h (revision 0)
> +++ include/apr_ldap_rebind.h (revision 0)
> @@ -0,0 +1,80 @@
> +/* Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +/**
> + * The APR LDAP rebind functions provide an implementation of
> + * a rebind procedure that can be used to allow clients to chase referrals,
> + * using the same credentials used to log in originally.
> + *
> + * Use of this implementation is optional.
> + *
> + * @file apu_ldap_rebind.h
> + * @brief Apache LDAP library
> + */
> +
> +#ifndef APU_LDAP_REBIND_H
> +#define APU_LDAP_REBIND_H
> +
> +/**
> + * APR LDAP initialize rebind lock
> + *
> + * This function creates the lock for controlling access to the xref list..
> + * @param pool Pool to use when creating the xref_lock.
> + */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_init(apr_pool_t *pool);
> +
> +
> +/**
> + * APR LDAP rebind_add function
> + *
> + * This function creates a cross reference entry for the specified ldap
> + * connection. The rebind callback function will look up this ldap
> + * connection so it can retrieve the bindDN and bindPW for use in any
> + * binds while referrals are being chased.
> + *
> + * This function will add the callback to the LDAP handle passed in.
> + *
> + * A cleanup is registered within the pool provided to remove this
> + * entry when the pool is removed. Alternatively apr_ldap_rebind_remove()
> + * can be called to explicitly remove the entry at will.
> + *
> + * @param pool The pool to use
> + * @param ld The LDAP connectionhandle
> + * @param bindDN The bind DN to be used for any binds while chasing
> + * referrals on this ldap connection.
> + * @param bindPW The bind Password to be used for any binds while
> + * chasing referrals on this ldap connection.
> + */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_add(apr_pool_t *pool,
> + LDAP *ld,
> + const char *bindDN,
> + const char *bindPW);
> +
> +/**
> + * APR LDAP rebind_remove function
> + *
> + * This function removes the rebind cross reference entry for the
> + * specified ldap connection.
> + *
> + * If not explicitly removed, this function will be called automatically
> + * when the pool is cleaned up.
> + *
> + * @param ld The LDAP connectionhandle
> + */
> +APU_DECLARE(apr_status_t) apr_ldap_rebind_remove(LDAP *ld);
> +
> +#endif /* APU_LDAP_REBIND_H */
> +
> Index: include/apr_ldap.h.in
> ===================================================================
> --- include/apr_ldap.h.in (revision 599173)
> +++ include/apr_ldap.h.in (working copy)
> @@ -154,6 +154,7 @@
> #include "apr_ldap_url.h"
> #include "apr_ldap_init.h"
> #include "apr_ldap_option.h"
> +#include "apr_ldap_rebind.h"
>
> /** @} */
> #endif /* APR_HAS_LDAP */
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by Graham Leggett <mi...@sharp.fm>.
Paul J. Reder wrote:
> I addressed the comments and haven't heard back from anyone
> (Bojan or others). I can't commit to APR and can't commit the
> Apache portion until the APR part has been committed. I know
> folks were busy with the latest APR update... Has the dust
> settled?
Looking at this in more detail.
It took me a while to figure out exactly what the rebind code was trying
to do (as opposed to generally knowing what it does), and it seems to be
"an implementation of a callback mechanism able to take advantage of the
LDAP referral callback feature".
Or in other words, use of this particular API is optional, someone using
the APR interface may choose to use this particular implementation, or
they may choose some other implementation of their own.
Based on this, I think the API should all be in a namespace like
apr_ldap_rebind.
Looking further, unless I am missing something, I think this could
probably be significantly simplified.
In theory, the apr_ldap_set_rebind_callback() function can be called by
apr_ldap_xref_add(), hiding apr_ldap_set_rebind_callback().
In theory, there should be a way to register a pool cleanup for
apr_ldap_rebind_remove() as well.
I have updated the patch, which is attached - will this do?
Regards,
Graham
--
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by "Paul J. Reder" <re...@remulak.net>.
The PR is 26538 (still has the original patch attached though)
Graham Leggett wrote:
> Paul J. Reder wrote:
>
>> I addressed the comments and haven't heard back from anyone
>> (Bojan or others). I can't commit to APR and can't commit the
>> Apache portion until the APR part has been committed. I know
>> folks were busy with the latest APR update... Has the dust
>> settled?
>
> Which PR? Was going through the LDAP PRs a few days ago, will take a look.
>
> Regards,
> Graham
> --
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by Graham Leggett <mi...@sharp.fm>.
Paul J. Reder wrote:
> I addressed the comments and haven't heard back from anyone
> (Bojan or others). I can't commit to APR and can't commit the
> Apache portion until the APR part has been committed. I know
> folks were busy with the latest APR update... Has the dust
> settled?
Which PR? Was going through the LDAP PRs a few days ago, will take a look.
Regards,
Graham
--
Re: [Patch]: Add basic function to APR for LDAP rebind callback
support.
Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting "Paul J. Reder" <re...@remulak.net>:
> I addressed the comments and haven't heard back from anyone
> (Bojan or others). I can't commit to APR and can't commit the
> Apache portion until the APR part has been committed. I know
> folks were busy with the latest APR update... Has the dust
> settled?
>
> Any takers?
I feel reluctant to commit as I'm not overly familiar with design and
implementation of LDAP support in APU. I'll leave that to folks that
dealt with LDAP stuff before. Essentially, I just wanted to make sure
that your patch comes closer to acceptance, so that LDAP folks are
left with less work before committing.
--
Bojan
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by "Paul J. Reder" <re...@remulak.net>.
I addressed the comments and haven't heard back from anyone
(Bojan or others). I can't commit to APR and can't commit the
Apache portion until the APR part has been committed. I know
folks were busy with the latest APR update... Has the dust
settled?
Any takers?
Thanks,
Paul J. Reder
Bojan Smojver wrote:
> Quoting "Paul J. Reder" <re...@remulak.net>:
>
>> The patches have been updated to address the issues pointed
>> out below (thanks Bojan).
>>
>> New contents at the old links:
>> http://people.apache.org/~rederpj/APR-trunk_rebind.diff
>> http://people.apache.org/~rederpj/Apache-trunk_rebind.diff
>
> Cool.
>
>> Question: To what end? The xref entry isn't of any use to anyone except
>> in terms of a node in a list that the rebind callback can scan through.
>> The problem is that the rebind callback function is called from the ldap
>> library function and has no context related to the original request info
>> other than the ldap handle passed to it. The xrefs and the list they are
>> kept in needs to be protected to avoid concurrency issues, and having the
>> xref pointer doesn't help the rebind callback function since the xref
>> pointer can't be passed to it. Is there some reason the xref entry would
>> be of use to the caller that I've not thought of?
>
> Point taken. I didn't actually look into what the patch does when I
> reviewed the code.
>
> --Bojan
>
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [Patch]: Add basic function to APR for LDAP rebind callback
support.
Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting "Paul J. Reder" <re...@remulak.net>:
> The patches have been updated to address the issues pointed
> out below (thanks Bojan).
>
> New contents at the old links:
> http://people.apache.org/~rederpj/APR-trunk_rebind.diff
> http://people.apache.org/~rederpj/Apache-trunk_rebind.diff
Cool.
> Question: To what end? The xref entry isn't of any use to anyone except
> in terms of a node in a list that the rebind callback can scan through.
> The problem is that the rebind callback function is called from the ldap
> library function and has no context related to the original request info
> other than the ldap handle passed to it. The xrefs and the list they are
> kept in needs to be protected to avoid concurrency issues, and having the
> xref pointer doesn't help the rebind callback function since the xref
> pointer can't be passed to it. Is there some reason the xref entry would
> be of use to the caller that I've not thought of?
Point taken. I didn't actually look into what the patch does when I
reviewed the code.
--
Bojan
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Posted by "Paul J. Reder" <re...@remulak.net>.
The patches have been updated to address the issues pointed
out below (thanks Bojan).
New contents at the old links:
http://people.apache.org/~rederpj/APR-trunk_rebind.diff
http://people.apache.org/~rederpj/Apache-trunk_rebind.diff
I had a question though (inline below)...
Bojan Smojver wrote:
> On Wed, 2007-11-14 at 20:02 -0500, Paul J. Reder wrote:
>
>> The APR portion of the patch:
>>
>> http://people.apache.org/~rederpj/APR-trunk_rebind.diff
>
> Just a few comments (without going into what the patch does):
>
> - you probably want to rename apr_ldap_init_xref_lock() to
> apr_ldap_xref_init() instead
done
> - LDAP_rebindproc() should probably be static
done
> - what happens when apr_ldap_xref_init() gets called again (e.g. by
> another Apache module wanting to initialise this subsystem)?
addressed (check if it's already been initialized)
> - is it possible to make the API so that xref entry is returned instead
> of being stored for the process?
Question: To what end? The xref entry isn't of any use to anyone except
in terms of a node in a list that the rebind callback can scan through.
The problem is that the rebind callback function is called from the ldap
library function and has no context related to the original request info
other than the ldap handle passed to it. The xrefs and the list they are
kept in needs to be protected to avoid concurrency issues, and having the
xref pointer doesn't help the rebind callback function since the xref
pointer can't be passed to it. Is there some reason the xref entry would
be of use to the caller that I've not thought of?
> - LDAP_xref_entry_t live is a namespace violation, because it's in a
> public header file; should probably be called apr_ldap_xref_entry_t
done
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [Patch]: Add basic function to APR for LDAP rebind callback
support.
Posted by Bojan Smojver <bo...@rexursive.com>.
On Wed, 2007-11-14 at 20:02 -0500, Paul J. Reder wrote:
> The APR portion of the patch:
>
> http://people.apache.org/~rederpj/APR-trunk_rebind.diff
Just a few comments (without going into what the patch does):
- you probably want to rename apr_ldap_init_xref_lock() to
apr_ldap_xref_init() instead
- LDAP_rebindproc() should probably be static
- what happens when apr_ldap_xref_init() gets called again (e.g. by
another Apache module wanting to initialise this subsystem)?
- is it possible to make the API so that xref entry is returned instead
of being stored for the process?
- LDAP_xref_entry_t live is a namespace violation, because it's in a
public header file; should probably be called apr_ldap_xref_entry_t
--
Bojan