Issue 7027 - [PATCH] Implement priority/weight for DNS SRV records
Summary: [PATCH] Implement priority/weight for DNS SRV records
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-23 21:07 UTC by james.leddy@redhat.com
Modified: 2020-05-12 22:47 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description james.leddy@redhat.com 2011-08-23 21:07:42 UTC
Full_Name: James M Leddy
Version: HEAD
OS: Linux
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (66.187.233.202)


From RFC 2782:

  A client MUST attempt to contact the target host with the
  lowest-numbered priority it can reach.

This patch sorts the DNS SRV records by their priority, and
additionally gives records with a larger weight a higher probability
of appearing earlier. This way, the DNS SRV records are tried in the
order of their priority.
Comment 1 james.leddy@redhat.com 2011-08-23 21:16:07 UTC
File at james-leddy-110823.patch.1

    The attached file is derived from OpenLDAP Software. All of the
    modifications to OpenLDAP Software represented in the following
    patch(es) were developed by Red Hat Inc. Red Hat Inc. has not
    assigned rights and/or interest in this work to any party. I,
    James M. Leddy am authorized by Red Hat Inc., my employer, to
    release this work under the following terms.

    Red Hat Inc. hereby place the following modifications to OpenLDAP
    Software (and only these modifications) into the public
    domain. Hence, these modifications may be freely used and/or
    redistributed for any purpose with or without attribution and/or
    other notice.

-- 
James M. Leddy
Technical Account Manager
Red Hat Inc.

Comment 2 james.leddy@redhat.com 2011-08-24 19:27:34 UTC
Fixed up some whitespace formatting. New version is
james-leddy-110824.patch 

on incoming ftp

ftp://ftp.openldap.org/incoming/james-leddy-110824.patch

Comment 3 jvcelak@redhat.com 2011-09-20 12:08:35 UTC
On Wednesday 24 August 2011 21:28:03, James Leddy wrote:
> on incoming ftp

There is a typo in the patch:

 +              hostent_head[hostent_count].priority=priority;
-+              hostent_head[hostent_count].weight=priority;
++              hostent_head[hostent_count].weight=weight;
 +              hostent_head[hostent_count].port=port;
 +              strncpy(hostent_head[hostent_count].hostname, host,255);
 +              hostent_count=hostent_count+1;


In addition, random seed has to be initialized, otherwise the behavior is 
predictable. On the other side, I do not think that calling srand() in libldap 
is a good idea. The client application should do that. Or is there any other 
option?

Jan

Comment 4 james.leddy@redhat.com 2011-09-20 15:38:42 UTC
> In addition, random seed has to be initialized, otherwise the behavior
> is predictable. 

I didn't think it too big of a deal, since it doesn't have to be
cryptographically random. I can submit a patch to call srandom()
first.

> On the other side, I do not think that calling srand()
> in libldap is a good idea. The client application should do that. Or
> is there any other option?

The function ldap_domain2hostlist returns a list of strings, there
isn't any weight/prio information for the client to act
on. Furthermore the weight prio section was commented out, I figured
the library was the right place to do it.

The algo specified in rfc2782 is detailed, and specifies a random
number should be generated., I didn't want to implement it because it
results in sorting once by prio, once by weight instead of one sort
that does both.


-- 
James M. Leddy
Technical Account Manager
Red Hat Inc.

Comment 5 Howard Chu 2014-07-21 22:29:22 UTC
changed notes
changed state Open to Test
moved from Incoming to Software Enhancements
Comment 6 Howard Chu 2014-07-22 05:28:54 UTC
Taking a look again as it was recently called to our attention in 
https://bugzilla.redhat.com/show_bug.cgi?id=1095976

>> In addition, random seed has to be initialized, otherwise the behavior
>> is predictable.
>
> I didn't think it too big of a deal, since it doesn't have to be
> cryptographically random. I can submit a patch to call srandom()
> first.
>
>> On the other side, I do not think that calling srand()
>> in libldap is a good idea. The client application should do that. Or
>> is there any other option?

Agreed, calling srand() inside libldap would probably be a bad idea. Nor does 
it seem critical to use any particularly high quality PRNG here. I've added a 
commonly used LCG instead, seeded by time().

> The function ldap_domain2hostlist returns a list of strings, there
> isn't any weight/prio information for the client to act
> on. Furthermore the weight prio section was commented out, I figured
> the library was the right place to do it.
>
> The algo specified in rfc2782 is detailed, and specifies a random
> number should be generated., I didn't want to implement it because it
> results in sorting once by prio, once by weight instead of one sort
> that does both.

You cannot shuffle inside quicksort, its behavior is undefined if the compare 
function isn't consistent/transitive. There's a good illustration of one 
problem with that here http://bost.ocks.org/mike/algorithms/#shuffling

I've simplified the qsort comparator and added the explicit shuffle step.

This is now fixed in git master.

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

Comment 7 Quanah Gibson-Mount 2014-07-22 15:34:14 UTC
changed notes
changed state Test to Release
Comment 8 OpenLDAP project 2014-10-23 07:30:01 UTC
fixed in master
fixed in RE25
fixed in RE24
Comment 9 Quanah Gibson-Mount 2014-10-23 07:30:01 UTC
changed notes
changed state Release to Closed