https://bugs.openldap.org/show_bug.cgi?id=10297
Issue ID: 10297 Summary: LDAP initialization does unnecessary resolution of hostname Product: OpenLDAP Version: 2.6.8 Hardware: All OS: All Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: libraries Assignee: bugs@openldap.org Reporter: simon.pichugin@gmail.com Target Milestone: ---
curl --version does try to resolve local hostname, which is usually stored in $HOSTNAME variable. It seems it does that for no good reason. It does not matter whether machine hostname is already FQDN or not, it always try it unconditionally by calling getaddrinfo(3).
Every usage of dnf tries to resolve hostname. That is then supressed by myhostname on Fedora, which returns non-helping response. Possibly, the hostname should be fetched from actual network responses.
Seen with: openldap-2.6.8-5.fc41.x86_64
Reproducible: Always
Steps to Reproduce: 1. dnf install gdb curl 2. gdb --args curl --version 3. (gdb) break getaddrinfo 4. (gdb) run Actual Results: getaddrinfo is called with current hostname, stored into ldap_int_hostname variable. That is used only when ldap client has not configured target server. But this hostname seems fetched always.
Expected Results: No network activity happens, unless something is actually requested. This is not the case.
Suggestion is to make it lazy initialized. It should be tried only when necessary. This seems to be useful when tlso_session_chkhost in libraries/libldap/tls_o.c is used. It should initialize hostname only once conditions to use it happens. There is a fallback anyway. It should query FQDN only when name_in contains unusable response.
Related: https://github.com/systemd/systemd/issues/34897
https://bugs.openldap.org/show_bug.cgi?id=10297
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|bugs@openldap.org |ondra@mistotebe.net Target Milestone|--- |2.7.0 Keywords|needs_review |
https://bugs.openldap.org/show_bug.cgi?id=10297
Ondřej Kuzník ondra@mistotebe.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |IN_PROGRESS Ever confirmed|0 |1
--- Comment #1 from Ondřej Kuzník ondra@mistotebe.net --- Hi Simon, can you try MR!768 to check it's sorted with that change?
https://git.openldap.org/openldap/openldap/-/merge_requests/768
Thanks, Ondrej
https://bugs.openldap.org/show_bug.cgi?id=10297
--- Comment #2 from Simon Pichugin simon.pichugin@gmail.com --- Hi Ondřej, Yes! With the change, getaddrinfo is not called unless its needed.
Thanks! Simon
https://bugs.openldap.org/show_bug.cgi?id=10297
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|IN_PROGRESS |RESOLVED
--- Comment #3 from Quanah Gibson-Mount quanah@openldap.org --- • 60634983 by Ondřej Kuzník at 2025-05-09T09:31:26+01:00 ITS#10297 Defer hostname resolution til first use
https://bugs.openldap.org/show_bug.cgi?id=10297
--- Comment #4 from pemensik@redhat.com --- Ah, great to have this fixed. It were causing mess at many places.
It seems it could still be made used from only special cases where ( !name_in || !strcasecmp( name_in, "localhost" ) ) condition were already checked. Only in that case it should start resolution attempt, because only only that rare case it would get used.
The current fix postpones it only when some actual TLS connection is made. But I think it should be posponed until localhost connection is made only. For well configured client it should still be avoided.
Looking at MR, os-ip.c would use it only for ::1, 127.0.0.1 or 0.0.0.0 addresses or AF_LOCAL socket. But the resolution is started even for different addresses and it could and should be avoided IMO. Resolution could be moved in all cases just right before strdup:
if ( !ldap_int_hostname ) ldap_int_resolve_hostname(); return LDAP_STRDUP( ldap_int_hostname );
Similarly with localhost or unset name in tls. I think something like this should be used instead in tls variants:
const char * ldap_int_hostname_not_localhost(const char *name_in) { if ( !name_in || !strcasecmp( name_in, "localhost" ) ) { if ( !ldap_int_hostname ) ldap_int_resolve_hostname(); if (ldap_int_hostname) { return ldap_int_hostname; } else { return name_in; } } else { return name_in; } }
// would become shared and lazy initialized name = ldap_int_hostname_not_localhost(name_in);
Ideally sharing common function instead of repeating this logic 3 times.
Maybe ldap_int_resolve_hostname should just return resolved name, whatever it is. The reading of ldap_int_hostname might be protected by the mutex as well, not just its modification. static resolved already checks whether it were already tried.
This would ensure when correct FQDN is configured, then no another resolution would be made.