curl / curl

A command line tool and library for transferring data with URL syntax, supporting DICT, FILE, FTP, FTPS, GOPHER, GOPHERS, HTTP, HTTPS, IMAP, IMAPS, LDAP, LDAPS, MQTT, POP3, POP3S, RTMP, RTMPS, RTSP, SCP, SFTP, SMB, SMBS, SMTP, SMTPS, TELNET, TFTP, WS and WSS. libcurl offers a myriad of powerful features
https://curl.se/
Other
35.44k stars 6.37k forks source link

Critical LDAP URL extensions are silently ignored #14327

Closed jscott0 closed 1 month ago

jscott0 commented 1 month ago

I did this

RFC 2255 specifies the URL syntax for LDAP, a protocol for getting information from directory servers. Just like a TLS certificate, entities are described by X.500-style distinguished name (DN). The syntax is basically the hostname/port combo, an optional entry point to do the search at (if not specified, the server is configured with a default), and then zero to four positional parameters separated by question marks. For example, here's how to use curl to find out how my name should be shown, taking advantage of the first positional parameter which can specify a particular attribute you're interested in:

$ curl 'ldap://johnscott.me/CN=John%20Scott,DC=johnscott,DC=me?displayName'
DN: cn=John Scott,dc=johnscott,dc=me
    displayName: John Scott

The URL syntax allows specifying extensions on the end, which are in their own namespace exclusively for use with LDAP URL syntax: they're not to be confused with other ways LDAP is extensible. Extensions are the very last optional positional parameter. If extensions are prefixed with exclamation marks, then (similar to critical X.509 certificate attributes) clients who are unable to effect the use of the extension, perhaps due to not being familiar with the extension, must abort. The LDAP URL RFC specifies one extension and I don't know of any others off the top of my head, and that is the bindname URL extension. This specifies the user you must authenticate as before trying to use the URL, which can be useful if there are certain attributes that are only available to duly-authorized users. (Debian's LDAP server is a good example: it's exposed to the public Internet and many attributes are public without needing authentication at all like my server is, but some more sensitive attributes can only be read by Debian Developers using their credentials.)

There is no "Bob" on my LDAP server, so since the extension is marked as critical here, curl is required by the standard to abort:

curl -v 'ldap://johnscott.me/CN=John%20Scott,DC=johnscott,DC=me?displayName???!bindname=CN=Bob,DC=johnscott,DC=me'

I expected the following

The standard requires curl to bail out if it isn't able to effectuate the use of a critical URL extension, but it doesn't seem to do that:

$ curl -v 'ldap://johnscott.me/CN=John%20Scott,DC=johnscott,DC=me?displayName???!bindname=CN=Bob,DC=johnscott,DC=me'
* Host johnscott.me:389 was resolved.
* IPv6: 2600:3c03::f03c:93ff:fef6:24
* IPv4: 172.104.214.157
*   Trying [2600:3c03::f03c:93ff:fef6:24]:389...
* Connected to johnscott.me (2600:3c03::f03c:93ff:fef6:24) port 389
* LDAP local: ldap://johnscott.me/CN=John%20Scott,DC=johnscott,DC=me?displayName???!bindname=CN=Bob,DC=johnscott,DC=me
DN: cn=John Scott,dc=johnscott,dc=me
    displayName: John Scott

* Connection #0 to host johnscott.me left intact

curl appears to not be informed on this aspect of the URL syntax at all: it doesn't care one bit.

Admittedly the bindname attribute isn't very useful with curl, at least not right now. curl only supports simple password-based authentication for LDAP at the moment, but including the "username" in the URL when the password needs to be supplied in some other channel like a curl option anyway is klunky. I therefore think that curl adding support for LDAP URL extensions is a very low priority.

What should be a higher priority, I think, is that curl should obey the RFC and terminate the transfer if the URL contains any critical extensions at all. It would be nice if curl could issue warnings for non-critical extensions that will be ignored, but that's not imperative for conformance.

curl/libcurl version

curl 8.9.0 (x86_64-pc-linux-gnu) libcurl/8.9.0 GnuTLS/3.8.6 zlib/1.3.1 brotli/1.1.0 zstd/1.5.6 libidn2/2.3.7 libpsl/0.21.2 libssh2/1.11.0 nghttp2/1.62.1 ngtcp2/1.6.0 nghttp3/1.4.0 librtmp/2.3 OpenLDAP/2.5.18 Release-Date: 2024-07-24, security patched: 8.9.0-3 Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns ldap ldaps mqtt pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp ws wss Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTP3 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM PSL SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

operating system

Linux T450 6.10-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.10.1-1~exp1 (2024-07-26) x86_64 GNU/Linux (This is Debian unstable, but with the kernel from experimental.)

bagder commented 1 month ago

curl has supported LDAP for almost 25 years and this is the first time I learn about these extensions.

RFC 4516 seems to obsolete RFC 2255 and from my quick reading of it seem to say that the syntax is different?

@hyc, any comment on this?

hyc commented 1 month ago

Yes, RFC 2255 was obsoleted by 4516 and "bindname" was changed to "e-bindname" as a hypothetical example, not as a formally specified URL extension. Nobody actually implements e-bindname.

Generally we expect libldap to do all LDAP URL parsing itself. E.g. here https://git.openldap.org/openldap/openldap/-/blob/master/libraries/libldap/url.c?ref_type=heads#L30

URL extensions are processed client-side, and libldap currently only supports 1 critical extension: StartTLS. Currently any other URL extensions are parsed but ignored. Indeed, we only even look at extensions if the build was configured to support TLS. https://git.openldap.org/openldap/openldap/-/blob/master/libraries/libldap/request.c?ref_type=heads#L532

bagder commented 1 month ago

@hyc the RFC 4516 says:

If an extension is not implemented and is marked critical, the implementation MUST NOT process the URL.

Shouldn't that then mean that a URL like @jscott0 shows should stop/return an error or something?

hyc commented 1 month ago

Yes, I guess this is a libldap bug since it doesn't check for those right now.

bagder commented 1 month ago

@jscott0 I will defer this issue to OpenLDAP, as that is the library we use for LDAP and it parses and responds to these details. While libcurl could figure this out and act on its own, I don't think we should. The LDAP knowledge exists in OpenLDAP.

hyc commented 1 month ago

Reported and fixed here https://bugs.openldap.org/show_bug.cgi?id=10247

jscott0 commented 1 month ago

Thank you both for digging into this with such care. The proposed fix in OpenLDAP looks like the perfect solution. I haven't given the OpenLDAP patch a try yet, but if I run into problems I'll report back. Seeing as changes in curl shouldn't be needed, feel free to close this issue at your leisure

bagder commented 1 month ago

Case fixed in OpenLDAP.