Repository navigation
Conversation
The legacy LDAP backend called Curl_conn_get_ip_info() to learn the destination libcurl's connection layer had selected, but only used ipquad.remote_port from it. ldap_init()/ldap_sslinit() were still called with conn->origin->hostname, so the native LDAP SDK resolved and connected to that hostname entirely on its own, independently of any cfilter, CURLOPT_RESOLVE, or CURLOPT_CONNECT_TO decision. An application pinning an LDAP hostname via CURLOPT_RESOLVE for SSRF protection got that pin silently ignored. For plain-text ldap:// connections, pass the already-resolved address (ipquad.remote_ip) instead of the hostname, so the SDK connects to exactly what libcurl decided. Empirically verified against a local slapd (OpenLDAP 2.7.1): a CURLOPT_RESOLVE-pinned hostname that cannot be resolved by the system resolver at all now still connects and completes a search, whereas before the fix the SDK's independent re-resolution of that hostname failed outright. ldaps:// connections keep using the hostname: the legacy backend hands ldaps:// TLS entirely to the native LDAP SDK (see the existing CURLOPT_CAINFO/CURLOPT_SSL_VERIFYPEER/CURLOPT_PINNEDPUBLICKEY/ CURLOPT_SSL_VERIFYSTATUS notes for this same backend), and the SDK verifies the server certificate against whatever host string it was given, so switching to an IP literal there would break certificate verification instead of just fixing destination selection. Documented as a caveat on CURLOPT_RESOLVE and CURLOPT_CONNECT_TO instead. Reported-by: Joshua Rogers
bagder
approved these changes
Oct 7, 2026
There was a problem hiding this comment.
🟡 Changes recommended
Proxy tunnels can redirect LDAP SDK connections and credentials to the proxy endpoint, and the CONNECT_TO documentation misstates port behavior.
2 open findings
What changed in this PR
Updates legacy LDAP routing so plaintext connections use libcurl’s resolved address while retaining hostnames for LDAPS certificate verification.
Changes:
- Uses the selected IP for plaintext LDAP.
- Documents legacy LDAPS limitations.
| File | Description |
|---|---|
lib/ldap.c |
Selects the SDK connection host by TLS mode. |
docs/libcurl/opts/CURLOPT_RESOLVE.md |
Documents legacy LDAPS behavior. |
docs/libcurl/opts/CURLOPT_CONNECT_TO.md |
Documents LDAP connection-routing behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+378
to
+379
| host = curlx_convert_UTF8_to_tchar(ldap_ssl ? conn->origin->hostname : | ||
| ipquad.remote_ip); |
Comment on lines
+82
to
+88
| This option has no effect on ldaps:// requests when libcurl uses the legacy | ||
| LDAP backend. That backend hands the connection to the native LDAP SDK, | ||
| which independently resolves and connects to the original hostname for its | ||
| TLS handshake. Plain-text ldap:// requests connect to the address this | ||
| option selects. When libcurl is built with USE_OPENLDAP, the OpenLDAP | ||
| backend routes the connection through curl's connection layer and this | ||
| option is honored for both schemes. |
Member
Author
There was a problem hiding this comment.
Good god, LLMs dueling, I feel like I'd rather close this right away.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The legacy LDAP backend called Curl_conn_get_ip_info() to learn the
destination libcurl's connection layer had selected, but only used
ipquad.remote_port from it. ldap_init()/ldap_sslinit() were still called
with conn->origin->hostname, so the native LDAP SDK resolved and
connected to that hostname entirely on its own, independently of any
cfilter, CURLOPT_RESOLVE, or CURLOPT_CONNECT_TO decision. An application
pinning an LDAP hostname via CURLOPT_RESOLVE for SSRF protection got
that pin silently ignored.
For plain-text ldap:// connections, pass the already-resolved address
(ipquad.remote_ip) instead of the hostname, so the SDK connects to
exactly what libcurl decided. Empirically verified against a local slapd
(OpenLDAP 2.7.1): a CURLOPT_RESOLVE-pinned hostname that cannot be
resolved by the system resolver at all now still connects and completes
a search, whereas before the fix the SDK's independent re-resolution of
that hostname failed outright.
ldaps:// connections keep using the hostname: the legacy backend hands
ldaps:// TLS entirely to the native LDAP SDK (see the existing
CURLOPT_CAINFO/CURLOPT_SSL_VERIFYPEER/CURLOPT_PINNEDPUBLICKEY/
CURLOPT_SSL_VERIFYSTATUS notes for this same backend), and the SDK
verifies the server certificate against whatever host string it was
given, so switching to an IP literal there would break certificate
verification instead of just fixing destination selection. Documented as
a caveat on CURLOPT_RESOLVE and CURLOPT_CONNECT_TO instead.
Reported-by: Joshua Rogers