Re: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

--On Monday, August 07, 2017 4:49 PM -0700 Ryan Tandy  
wrote:


For easier digestion, the patch supplied by Daniel is available in my 
scratch repo in the its8654 branch.  I did change the socket option to 
match the MS socket option.





--Quanah



--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

Re: ITS#8654 - Option for LDAP client to bind to a local address

[Ryan Tandy]

On Mon, Jun 12, 2017 at 10:15:56PM +, Daniel Le wrote:
Please review the code change. The diff is against the master branch of 
git://git.openldap.org/openldap.git.


I'm not able to apply the patch from this email. The whitespace in the 
context has been mangled - your mail only contains spaces, where the 
original files contain tabs. My review is therefore only based on 
reading the code, not testing it. Please attach the actual git commit 
(use git-format-patch(1)), or link to a branch in a git repository.


I don't know a lot of this code very well, so please forgive any stupid 
questions and try to answer patiently. :)


In os-ip.c, the ldap_socket_bind_addr() function is only called to 
effectively bind LDAP client socket to a local IP address if 
HAVE_GETADDRINFO and HAVE_INET_NTOP are defined.


OK. Does the code still compile and behave reasonably if they aren't? 
I'm wondering if it still makes sense to compile any of the contents of 
addr.c or even ldap_socket_bind_addr itself, for example, if getaddrinfo 
is missing. And I wonder also about the expected return value of 
ldap_set_option() when it's not actually possible to respect the 
requested setting.



Changes to be committed:
 (use "git reset HEAD ..." to unstage)

   new file:   addr.c

Changes not staged for commit:
 (use "git add ..." to update what will be committed)
 (use "git checkout -- ..." to discard changes in working directory)

   modified:   ../../include/ldap.h
   modified:   Makefile.in
   modified:   ldap-int.h
   modified:   options.c
   modified:   os-ip.c


Please also update the ldap_get_option.3 man page. (And hopefully I 
don't need to say this, but I expect Microsoft's documentation is 
copyrighted and simply cloning or paraphrasing it would be a bad idea.)



include$git diff ldap.h
diff --git a/include/ldap.h b/include/ldap.h
index 588e906..0268f0e 100644
--- a/include/ldap.h
+++ b/include/ldap.h
@@ -109,6 +109,8 @@ LDAP_BEGIN_DECL
#define LDAP_OPT_DIAGNOSTIC_MESSAGE0x0032
#define LDAP_OPT_ERROR_STRING  LDAP_OPT_DIAGNOSTIC_MESSAGE
#define LDAP_OPT_MATCHED_DN0x0033
+/* same option code as Microsoft LDAP_OPT_SOCKET_BIND_ADDRESSES */
+#define LDAP_OPT_BIND_ADDRESSES 0x0044


Wouldn't it be more compatible to name the option the same?


/* 0x0034 - 0x3fff not defined */
/* 0x0091 used by Microsoft for LDAP_OPT_AUTO_RECONNECT */
#define LDAP_OPT_SSPI_FLAGS0x0092
@@ -815,6 +817,15 @@ typedef struct ldap_url_desc {
#define LDAP_URL_ERR_BADEXTS   0x0a/* bad or missing extensions */

/*
+ *  data type for ldap socket bind addresses
+ */
+typedef struct ldap_bind_addr {
+struct ldap_bind_addr *lba_next;
+char   *lba_address;
+intlba_family;
+} LDAPBindAddr;


Does this definition need to be exposed to the world? Could it be in 
ldap-int.h?





libldap$git diff os-ip.c
diff --git a/libraries/libldap/os-ip.c b/libraries/libldap/os-ip.c
index c7cee92..fbfb8a9 100644
--- a/libraries/libldap/os-ip.c
+++ b/libraries/libldap/os-ip.c
@@ -209,6 +209,50 @@ ldap_int_prepare_socket(LDAP *ld, int s, int proto )
   return 0;
}

+static int
+ldap_socket_bind_addr(LDAP *ld, int s, int sf, int st)
+{
+   LDAPBindAddr **ba;
+   LDAPBindAddr *bap;


Hmm. The double-pointer is the one with fewer "p"s in its name? :)


+   struct addrinfo hints, *bai;
+   int err;
+   int matched = 0;
+
+   for ( ba = >ld_options.ldo_bind_addr; *ba != NULL; ba = 
&(*ba)->lba_next ) {


What's the use of the double pointer when we're only reading the list? I 
know this idiom is helpful when we want to modify the list, but that's 
not the case here, right? The addr.c functions later on also only use a 
single pointer.



+   bap = *ba;
+   if ( bap->lba_family == sf ) {
+   matched = 1;
+   break;
+   }
+   }
+
+   if ( !matched ) {


This looks like it could perhaps just be if (*ba == NULL), i.e. we fell 
off the end of the list?



+   osip_debug(ld, "ldap_socket_bind_addr: no match\n", 0, 0, 0);
+   return -1;
+   }
+
+   memset( , 0, sizeof(hints) );
+   hints.ai_flags = AI_ADDRCONFIG;


Not AI_NUMERICHOST?

(I'm not necessarily looking for one answer or the other here; just a 
rationale, and the chosen behaviour to be documented.)



+   hints.ai_family = sf;
+   hints.ai_socktype = st;


Did you also test this with a UDP (cldap://) socket? I can't think why 
it wouldn't work, but I'd like to be certain. :)



+
+   err = getaddrinfo( bap->lba_address, NULL, ,  );


Assuming we're _not_ using AI_NUMERICHOST, then does this need to be 
holding the ldap_int_resolv_mutex?



+   if ( err != 0 ) {
+   osip_debug( ld, "ldap_socket_bind_addr: %s getaddrinfo error 
%s\n",
+   bap->lba_address, AC_GAI_STRERROR(err), 0 );
+ 

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

--On Thursday, June 15, 2017 3:29 PM + Daniel Le  
wrote:



Could someone take a stab at reviewing this code change and/or let me
know questions you might have? The ldap_get_option, ldap_set_option and
socket binding functions were successfully validated using a test program
under Linux/Unix.

Unfortunately, I don't have a set-up to test other operating systems, but
would be willing to further modidy ldap_connect_to_host in os-ip.c to
cover them if you can help verify it.


Howard's been hoping someone would be able to review in addition to him.  I 
gave it a lookover, but my C is quite rusty.  I could test it out on 
Solaris, and possibly get it tested with windows.  That's assuming you had 
it added as an option to the client utilities (ldapsearch, etc), similar to 
what I did with the TLS options I put out for review.


Also, I would strongly advise creating your own git repository on somewhre 
like github, like I did with my openldap-scratch repo, where you can commit 
your changes and work on them, and then create a single diff containing the 
entire changeset.


Example:



--Quanah


--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Daniel Le]

Hello,

Please review the code change. The diff is against the master branch of 
git://git.openldap.org/openldap.git.

Five files are modified and one is added (see output of 'git status -uno' 
command).

It was written and tested for Linux/Unix operating system. Windows, VxWorks, 
etc. are not covered/affected. In os-ip.c, the ldap_socket_bind_addr() function 
is only called to effectively bind LDAP client socket to a local IP address if 
HAVE_GETADDRINFO and HAVE_INET_NTOP are defined.

The functionality is made compatible with Microsoft implementation of the 
LDAP_OPT_SOCKET_BIND_ADDRESSES option wrt "You should provide both IPv4 and 
IPv6 local addresses, if available, because both IPv4 and IPv6 server addresses 
can be used for socket connect. Socket bind will fail if there is an address 
family mismatch. (Re. 
https://msdn.microsoft.com/en-us/library/aa367019(v=vs.85).aspx). This code 
change handles space-separated and comma- separated addresses.

libldap$git status -uno
On branch master
Your branch is up-to-date with 'origin/master'.
Changes to be committed:
  (use "git reset HEAD ..." to unstage)

new file:   addr.c

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   ../../include/ldap.h
modified:   Makefile.in
modified:   ldap-int.h
modified:   options.c
modified:   os-ip.c

include$git diff ldap.h
diff --git a/include/ldap.h b/include/ldap.h
index 588e906..0268f0e 100644
--- a/include/ldap.h
+++ b/include/ldap.h
@@ -109,6 +109,8 @@ LDAP_BEGIN_DECL
 #define LDAP_OPT_DIAGNOSTIC_MESSAGE0x0032
 #define LDAP_OPT_ERROR_STRING  LDAP_OPT_DIAGNOSTIC_MESSAGE
 #define LDAP_OPT_MATCHED_DN0x0033
+/* same option code as Microsoft LDAP_OPT_SOCKET_BIND_ADDRESSES */
+#define LDAP_OPT_BIND_ADDRESSES 0x0044
 /* 0x0034 - 0x3fff not defined */
 /* 0x0091 used by Microsoft for LDAP_OPT_AUTO_RECONNECT */
 #define LDAP_OPT_SSPI_FLAGS0x0092
@@ -815,6 +817,15 @@ typedef struct ldap_url_desc {
 #define LDAP_URL_ERR_BADEXTS   0x0a/* bad or missing extensions */
 
 /*
+ *  data type for ldap socket bind addresses
+ */
+typedef struct ldap_bind_addr {
+struct ldap_bind_addr *lba_next;
+char   *lba_address;
+intlba_family;
+} LDAPBindAddr;
+
+/*
  * LDAP sync (RFC4533) API
  */

libldap$git diff ldap-int.h 
diff --git a/libraries/libldap/ldap-int.h b/libraries/libldap/ldap-int.h
index bcd6118..1cb466d 100644
--- a/libraries/libldap/ldap-int.h
+++ b/libraries/libldap/ldap-int.h
@@ -206,6 +206,9 @@ struct ldapoptions {
char*   ldo_defbase;
char*   ldo_defbinddn;  /* bind dn */
 
+   /* socket bind addresses */
+   LDAPBindAddr *ldo_bind_addr;
+
/*
 * Per connection tcp-keepalive settings (Linux only,
 * ignored where unsupported)
@@ -804,6 +807,19 @@ LDAP_F (char *) ldap_url_list2hosts LDAP_P((
LDAPURLDesc *ludlist ));
 
 /*
+ * in addr.c
+ */
+LDAP_F (void) ldap_free_bind_addr LDAP_P((
+LDAPBindAddr *lba_ptr ));
+
+LDAP_F (int) ldap_parse_bind_addr LDAP_P((
+LDAPBindAddr **lba_list,
+const char *addresses ));
+
+LDAP_F (char *) ldap_list_bind_addr LDAP_P((
+LDAPBindAddr *lba_list ));
+
+/*
  * in cyrus.c
  */

libldap$git diff options.c 
diff --git a/libraries/libldap/options.c b/libraries/libldap/options.c
index 1705bd9..e3a9077 100644
--- a/libraries/libldap/options.c
+++ b/libraries/libldap/options.c
@@ -245,6 +245,11 @@ ldap_get_option(
rc = LDAP_OPT_SUCCESS;
break;
 
+   case LDAP_OPT_BIND_ADDRESSES:
+   * (char **) outvalue = ldap_list_bind_addr(lo->ldo_bind_addr);
+   rc = LDAP_OPT_SUCCESS;
+   break;
+
case LDAP_OPT_HOST_NAME:
* (char **) outvalue = ldap_url_list2hosts(lo->ldo_defludp);
rc = LDAP_OPT_SUCCESS;
@@ -541,6 +546,23 @@ ldap_set_option(
rc = LDAP_OPT_SUCCESS;
break;
 
+   case LDAP_OPT_BIND_ADDRESSES: {
+   const char *addr = (const char *) invalue;
+   LDAPBindAddr *lba_list = NULL;
+   rc = LDAP_OPT_SUCCESS;
+
+   if(addr != NULL) {
+   rc = ldap_parse_bind_addr(_list, addr);
+   }
+
+   if (rc == LDAP_OPT_SUCCESS) {
+   if (lo->ldo_bind_addr != NULL) {
+   ldap_free_bind_addr(lo->ldo_bind_addr);
+   }
+   lo->ldo_bind_addr = lba_list;
+   }
+   break;
+   }
 
case LDAP_OPT_HOST_NAME: {
const char *host = (const char *) invalue;

libldap$git diff 

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

Hi Daniel,

I would suggest sending in a patch against master to the -devel list for 
review.


For final inclusion if it is approved, see:



Regards,
Quanah

--On Monday, June 12, 2017 8:34 PM + Daniel Le  
wrote:



I've got a chance to write (and test) the code to add API support for
socket binding addresses.

Should I send the code diff to this openldap-devel email list for review?
How to submit a patch request?

Daniel

-Original Message-
From: Daniel Le
Sent: Tuesday, May 16, 2017 6:02 PM
To: 'openldap-devel@openldap.org' 
Subject: ITS#8654 - Option for LDAP client to bind to a local address

Hello,

In reference to the enhancement request ITS#865, please comment on the
following to add support for binding a local IP address to client socket.
This is just an outline of changes for one local address. I am not sure
whether a list of local addresses is necessary. If it is, then a new
function, similarly to ldap_url_parsehosts, may be written to parse the
list of local addresses and store them into a linked list. In my use
case, only one IPv4 or IPv6 local address is used for binding.

- Modify ldap.h and ldap_set_option to handle the new option
LDAP_OPT_LOCAL_ADDRESS.Should it be named LDAP_OPT_CLIENT_ADDRESS,
LDAP_OPT_SOCKET_BIND_ADDRESS...?

- Modify struct ldapoptions in ldap-int.h to add element "char
*ldo_local_address" to hold client local address when
ldap_set_option(LDAP_OPT_LOCAL_ADDRESS...) is executed.   This can char
pointer can point to an IPv4 address or IPv6 address.

- ldap_connect_to_host() in os-ip.c
  After the connection socket is created (ldap_int_socket) and before it
is connected (ldap_pvt_connect), extract the local IP address.   If local
address family (AF_INET/ AF_INET6) matches the one of the host, bind
socket to the local address.

Regards,
Daniel






--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Daniel Le]

Thanks. I'm surprised that this patch ended nowhere. Thought it only missed 
2.4...

Daniel

-Original Message-
From: Quanah Gibson-Mount [mailto:qua...@symas.com] 
Sent: Tuesday, May 23, 2017 8:10 PM
To: Daniel Le ; 'openldap-devel@openldap.org' 

Subject: RE: ITS#8654 - Option for LDAP client to bind to a local address

--On Tuesday, May 23, 2017 9:48 PM + Daniel Le 
wrote:

> I cloned the master branch (git clone
> git://git.openldap.org/openldap.git), then checked for the ITS#4707 
> patch commit
> (https://www.openldap.org/lists/openldap-bugs/200610/threads.html#0004
> ) in the OpenLDAP master code base and scanned the log of 2.3 branch 
> (http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=shortlog;h=
> ref s/heads/OPENLDAP_REL_ENG_2_3), but didn't find it at both places. 
> Seems this patch never got committed. Can you help to confirm?

Howard already confirmed it was never committed, just the different API was 
committed.

--Quanah


--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

--On Tuesday, May 23, 2017 9:48 PM + Daniel Le  
wrote:



I cloned the master branch (git clone
git://git.openldap.org/openldap.git), then checked for the ITS#4707 patch
commit
(https://www.openldap.org/lists/openldap-bugs/200610/threads.html#0004)
in the OpenLDAP master code base and scanned the log of 2.3 branch
(http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=shortlog;h=ref
s/heads/OPENLDAP_REL_ENG_2_3), but didn't find it at both places. Seems
this patch never got committed. Can you help to confirm?


Howard already confirmed it was never committed, just the different API was 
committed.


--Quanah


--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

--On Tuesday, May 23, 2017 4:47 PM + Daniel Le  
wrote:



How does one check out code from the OpenLDAP master branch?


As noted on the www.openldap.org main page:



--Quanah



--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Daniel Le]

How does one check out code from the OpenLDAP master branch?

I don't know the telnet code but its manual about -b option refers to 'an' 
aliased address or to 'the' address, not multiple IPV4 or IPv6 addresses.

-b hostalias
Uses bind(2) on the local socket to bind it to an aliased address (see 
ifconfig(8) and the ''alias'' specifier) or to the address of another interface 
than the one naturally chosen by connect(2). This can be useful when connecting 
to services which use IP addresses for authentication and reconfiguration of 
the server is undesirable (or impossible).

Daniel

-Original Message-
From: Quanah Gibson-Mount [mailto:qua...@symas.com] 
Sent: Tuesday, May 23, 2017 11:32 AM
To: Daniel Le ; 'Howard Chu' ; 
'openldap-devel@openldap.org' 
Subject: RE: ITS#8654 - Option for LDAP client to bind to a local address

--On Tuesday, May 23, 2017 4:18 PM + Daniel Le 
wrote:

> Since the ITS#4707 patch was not merged into 2.4 and is subject to 
> additional modification to handle a list of local addresses, perhaps 
> it makes sense to leverage from what was done in 2.3 and write a new 
> patch for 2.4 (i.e. no merge)?

All work should be done against OpenLDAP master rather than a release branch

> Regarding multiple local IP addresses, I can think of a scenario where 
> the desired local network interface has an IPv4 address and an IPv6 
> address, then the one which has the same address family as the 
> server's is chosen at the time of socket binding, but how to match a 
> client IP address with a server host address when there are multiple 
> IPv4 and/or
> IPv6 local addresses?

Curious, does telnet -b handle this currently?

> Daniel
>
> -Original Message-
> From: Daniel Le
> Sent: Monday, May 22, 2017 5:17 PM
> To: 'Howard Chu' ; Quanah Gibson-Mount 
> ; 'openldap-devel@openldap.org' 
>  Subject: RE:
> ITS#8654 - Option for LDAP client to bind to a local address
>
> From what I can see, ldap_init_fd() only allows the setting of 
> UDP/TCP/IPC protocol type and server's URI.
>
> Will the ITS#4707 patch
> (ftp://ftp.openldap.org/incoming/openldap-2.3.27-bindaddr.patch.txt) 
> be merged into 2.4?
>
> And since this patch only handles one IP address, further change will 
> be necessary to support a list of IP addresses.
>
> Daniel
>
> -Original Message-
> From: Howard Chu [mailto:h...@symas.com]
> Sent: Monday, May 22, 2017 2:49 PM
> To: Quanah Gibson-Mount ; Daniel Le 
> ; 'openldap-devel@openldap.org'
>  Subject: Re: ITS#8654 - Option for LDAP 
> client to bind to a local address
>
> Quanah Gibson-Mount wrote:
>> --On Monday, May 22, 2017 7:28 PM + Daniel Le 
>> 
>> wrote:
>>
>>> I tumbled into the following ITS#4707 thread:
>>> Patch: option to bind client socket to an address
>>> https://www.openldap.org/lists/openldap-bugs/200610/threads.html#000
>>> 4
>>> 7
>>>
>>> Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it 
>>> in the 2.4.44 code base.
>>
>> The notes say it was added for 2.4, but that apparently is incorrect.
>>
>> "new API added in HEAD/RE24"
>
> The notes say *a* new API was added to 2.4, not necessarily *this* one.
> And that is true, we added ldap_init_fd() which lets the caller setup 
> their socket any way they please.
>
> commit 1f635b8bcfaaac666005a88a5620e9798c9565e1
>
> --
>-- Howard Chu
>CTO, Symas Corp.   http://www.symas.com
>Director, Highland Sun http://highlandsun.com/hyc/
>Chief Architect, OpenLDAP  http://www.openldap.org/project/



--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

--On Tuesday, May 23, 2017 4:18 PM + Daniel Le  
wrote:



Since the ITS#4707 patch was not merged into 2.4 and is subject to
additional modification to handle a list of local addresses, perhaps it
makes sense to leverage from what was done in 2.3 and write a new patch
for 2.4 (i.e. no merge)?


All work should be done against OpenLDAP master rather than a release branch


Regarding multiple local IP addresses, I can think of a scenario where
the desired local network interface has an IPv4 address and an IPv6
address, then the one which has the same address family as the server's
is chosen at the time of socket binding, but how to match a client IP
address with a server host address when there are multiple IPv4 and/or
IPv6 local addresses?


Curious, does telnet -b handle this currently?


Daniel

-Original Message-
From: Daniel Le
Sent: Monday, May 22, 2017 5:17 PM
To: 'Howard Chu' ; Quanah Gibson-Mount ;
'openldap-devel@openldap.org'  Subject: RE:
ITS#8654 - Option for LDAP client to bind to a local address

From what I can see, ldap_init_fd() only allows the setting of
UDP/TCP/IPC protocol type and server's URI.

Will the ITS#4707 patch
(ftp://ftp.openldap.org/incoming/openldap-2.3.27-bindaddr.patch.txt) be
merged into 2.4?

And since this patch only handles one IP address, further change will be
necessary to support a list of IP addresses.

Daniel

-Original Message-
From: Howard Chu [mailto:h...@symas.com]
Sent: Monday, May 22, 2017 2:49 PM
To: Quanah Gibson-Mount ; Daniel Le
; 'openldap-devel@openldap.org'
 Subject: Re: ITS#8654 - Option for LDAP
client to bind to a local address

Quanah Gibson-Mount wrote:

--On Monday, May 22, 2017 7:28 PM + Daniel Le 
wrote:


I tumbled into the following ITS#4707 thread:
Patch: option to bind client socket to an address
https://www.openldap.org/lists/openldap-bugs/200610/threads.html#0004
7

Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it
in the 2.4.44 code base.


The notes say it was added for 2.4, but that apparently is incorrect.

"new API added in HEAD/RE24"


The notes say *a* new API was added to 2.4, not necessarily *this* one.
And that is true, we added ldap_init_fd() which lets the caller setup
their socket any way they please.

commit 1f635b8bcfaaac666005a88a5620e9798c9565e1

--
   -- Howard Chu
   CTO, Symas Corp.   http://www.symas.com
   Director, Highland Sun http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/




--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Daniel Le]

Since the ITS#4707 patch was not merged into 2.4 and is subject to additional 
modification to handle a list of local addresses, perhaps it makes sense to 
leverage from what was done in 2.3 and write a new patch for 2.4 (i.e. no 
merge)?

Regarding multiple local IP addresses, I can think of a scenario where the 
desired local network interface has an IPv4 address and an IPv6 address, then 
the one which has the same address family as the server's is chosen at the time 
of socket binding, but how to match a client IP address with a server host 
address when there are multiple IPv4 and/or IPv6 local addresses?

Daniel

-Original Message-
From: Daniel Le 
Sent: Monday, May 22, 2017 5:17 PM
To: 'Howard Chu' ; Quanah Gibson-Mount ; 
'openldap-devel@openldap.org' 
Subject: RE: ITS#8654 - Option for LDAP client to bind to a local address

>From what I can see, ldap_init_fd() only allows the setting of UDP/TCP/IPC 
>protocol type and server's URI.

Will the ITS#4707 patch 
(ftp://ftp.openldap.org/incoming/openldap-2.3.27-bindaddr.patch.txt) be merged 
into 2.4?

And since this patch only handles one IP address, further change will be 
necessary to support a list of IP addresses.

Daniel

-Original Message-
From: Howard Chu [mailto:h...@symas.com]
Sent: Monday, May 22, 2017 2:49 PM
To: Quanah Gibson-Mount ; Daniel Le ; 
'openldap-devel@openldap.org' 
Subject: Re: ITS#8654 - Option for LDAP client to bind to a local address

Quanah Gibson-Mount wrote:
> --On Monday, May 22, 2017 7:28 PM + Daniel Le  wrote:
>
>> I tumbled into the following ITS#4707 thread:
>> Patch: option to bind client socket to an address
>> https://www.openldap.org/lists/openldap-bugs/200610/threads.html#0004
>> 7
>>
>> Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it 
>> in the 2.4.44 code base.
>
> The notes say it was added for 2.4, but that apparently is incorrect.
>
> "new API added in HEAD/RE24"

The notes say *a* new API was added to 2.4, not necessarily *this* one. And 
that is true, we added ldap_init_fd() which lets the caller setup their socket 
any way they please.

commit 1f635b8bcfaaac666005a88a5620e9798c9565e1

-- 
   -- Howard Chu
   CTO, Symas Corp.   http://www.symas.com
   Director, Highland Sun http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Daniel Le]

>From what I can see, ldap_init_fd() only allows the setting of UDP/TCP/IPC 
>protocol type and server's URI.

Will the ITS#4707 patch 
(ftp://ftp.openldap.org/incoming/openldap-2.3.27-bindaddr.patch.txt) be merged 
into 2.4?

And since this patch only handles one IP address, further change will be 
necessary to support a list of IP addresses.

Daniel

-Original Message-
From: Howard Chu [mailto:h...@symas.com] 
Sent: Monday, May 22, 2017 2:49 PM
To: Quanah Gibson-Mount ; Daniel Le ; 
'openldap-devel@openldap.org' 
Subject: Re: ITS#8654 - Option for LDAP client to bind to a local address

Quanah Gibson-Mount wrote:
> --On Monday, May 22, 2017 7:28 PM + Daniel Le  wrote:
>
>> I tumbled into the following ITS#4707 thread:
>> Patch: option to bind client socket to an address
>> https://www.openldap.org/lists/openldap-bugs/200610/threads.html#0004
>> 7
>>
>> Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it 
>> in the 2.4.44 code base.
>
> The notes say it was added for 2.4, but that apparently is incorrect.
>
> "new API added in HEAD/RE24"

The notes say *a* new API was added to 2.4, not necessarily *this* one. And 
that is true, we added ldap_init_fd() which lets the caller setup their socket 
any way they please.

commit 1f635b8bcfaaac666005a88a5620e9798c9565e1

-- 
   -- Howard Chu
   CTO, Symas Corp.   http://www.symas.com
   Director, Highland Sun http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: ITS#8654 - Option for LDAP client to bind to a local address

[Howard Chu]

Quanah Gibson-Mount wrote:

--On Monday, May 22, 2017 7:28 PM + Daniel Le  wrote:


I tumbled into the following ITS#4707 thread:
Patch: option to bind client socket to an address
https://www.openldap.org/lists/openldap-bugs/200610/threads.html#00047

Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it in
the 2.4.44 code base.


The notes say it was added for 2.4, but that apparently is incorrect.

"new API added in HEAD/RE24"


The notes say *a* new API was added to 2.4, not necessarily *this* one. And 
that is true, we added ldap_init_fd() which lets the caller setup their socket 
any way they please.


commit 1f635b8bcfaaac666005a88a5620e9798c9565e1

--
  -- Howard Chu
  CTO, Symas Corp.   http://www.symas.com
  Director, Highland Sun http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Quanah Gibson-Mount]

--On Monday, May 22, 2017 7:28 PM + Daniel Le  
wrote:



I tumbled into the following ITS#4707 thread:
Patch: option to bind client socket to an address
https://www.openldap.org/lists/openldap-bugs/200610/threads.html#00047

Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it in
the 2.4.44 code base.


The notes say it was added for 2.4, but that apparently is incorrect.

"new API added in HEAD/RE24"

--Quanah

--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:

RE: ITS#8654 - Option for LDAP client to bind to a local address

[Daniel Le]

I tumbled into the following ITS#4707 thread:
Patch: option to bind client socket to an address
https://www.openldap.org/lists/openldap-bugs/200610/threads.html#00047

Wasn't this patch integrated into OpenLDAP 2.3.X? I couldn't see it in the 
2.4.44 code base.

Daniel

-Original Message-
From: Howard Chu [mailto:h...@symas.com] 
Sent: Monday, May 22, 2017 8:57 AM
To: Daniel Le ; 'openldap-devel@openldap.org' 

Subject: Re: ITS#8654 - Option for LDAP client to bind to a local address

Daniel Le wrote:
> Hello,
>
> In reference to the enhancement request ITS#865, please comment on the 
> following to add support for binding a local IP address to client socket. 
> This is just an outline of changes for one local address. I am not sure 
> whether a list of local addresses is necessary. If it is, then a new 
> function, similarly to ldap_url_parsehosts, may be written to parse the list 
> of local addresses and store them into a linked list. In my use case, only 
> one IPv4 or IPv6 local address is used for binding.

As I commented in the ITS, since there is no existing IETF spec for this 
option, and since there is an existing implementation in Microsoft's API, we 
should copy the Microsoft API behavior, to maintain source level compatibility 
and make app developers' lives easier.

Since they support a list of IP addresses, so should we.
>
> - Modify ldap.h and ldap_set_option to handle the new option 
> LDAP_OPT_LOCAL_ADDRESS.
>Should it be named LDAP_OPT_CLIENT_ADDRESS, 
> LDAP_OPT_SOCKET_BIND_ADDRESS...?
>
> - Modify struct ldapoptions in ldap-int.h to add element "char 
> *ldo_local_address" to hold client local address when 
> ldap_set_option(LDAP_OPT_LOCAL_ADDRESS...) is executed.
>   This can char pointer can point to an IPv4 address or IPv6 address.
>
> - ldap_connect_to_host() in os-ip.c
>   After the connection socket is created (ldap_int_socket) and before it is 
> connected (ldap_pvt_connect), extract the local IP address.
>   If local address family (AF_INET/ AF_INET6) matches the one of the host, 
> bind socket to the local address.
>
> Regards,
> Daniel

-- 
   -- Howard Chu
   CTO, Symas Corp.   http://www.symas.com
   Director, Highland Sun http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: ITS#8654 - Option for LDAP client to bind to a local address

[Howard Chu]

Daniel Le wrote:

Hello,

In reference to the enhancement request ITS#865, please comment on the 
following to add support for binding a local IP address to client socket. This 
is just an outline of changes for one local address. I am not sure whether a 
list of local addresses is necessary. If it is, then a new function, similarly 
to ldap_url_parsehosts, may be written to parse the list of local addresses and 
store them into a linked list. In my use case, only one IPv4 or IPv6 local 
address is used for binding.


As I commented in the ITS, since there is no existing IETF spec for this 
option, and since there is an existing implementation in Microsoft's API, we 
should copy the Microsoft API behavior, to maintain source level compatibility 
and make app developers' lives easier.


Since they support a list of IP addresses, so should we.


- Modify ldap.h and ldap_set_option to handle the new option 
LDAP_OPT_LOCAL_ADDRESS.
   Should it be named LDAP_OPT_CLIENT_ADDRESS, LDAP_OPT_SOCKET_BIND_ADDRESS...?

- Modify struct ldapoptions in ldap-int.h to add element "char 
*ldo_local_address" to hold client local address when 
ldap_set_option(LDAP_OPT_LOCAL_ADDRESS...) is executed.
  This can char pointer can point to an IPv4 address or IPv6 address.

- ldap_connect_to_host() in os-ip.c
  After the connection socket is created (ldap_int_socket) and before it is 
connected (ldap_pvt_connect), extract the local IP address.
  If local address family (AF_INET/ AF_INET6) matches the one of the host, bind 
socket to the local address.

Regards,
Daniel






--
  -- Howard Chu
  CTO, Symas Corp.   http://www.symas.com
  Director, Highland Sun http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/