Package: release.debian.org Severity: normal Tags: bookworm User: release.debian.org@packages.debian.org Usertags: pu X-Debbugs-Cc: curl@packages.debian.org, samueloph@debian.org Control: affects -1 + src:curl [ Reason ] This upload is to address a regression in CURL, an undesirable change in behavior as compared to Bullseye which works correctly. This upload contains a cherry-picked fix that allows CURL to be built with OpenLDAP-specific code instead of unmaintained fallback code as intended. Basically, the Autotools build system only would fail to detect OpenLDAP properly (Debian uses OpenLDAP). The fallback code, besides being unmaintained, has numerous incidental issues. [ Impact ] If this update isn't approved, then not only will advanced LDAP functionality not work, but fetching binary LDAP attributes will completely spoil CURL's output by injecting binary bytes into what's supposed to be a text file. This is the biggest fallout of using the unmaintained code. I don't think this issue is security-relevant, but it does make the LDAP functionality somewhat useless. The number of CURL LDAP users is hard to predict; LDAP is used frequently in internal-only business environments. [ Tests ] I manually tested the package against real-world LDAP servers; that's how I ran into the bug in the first place and noticed that it was a regression from the Bullseye behavior. Furthermore, I have added an autopkgtest validating that fetching binary attributes now works correctly. This test works by spinning up a real LDAP server and then using libcurl to fetch binary data from it. I verified that the autopkgtest fails without the upstream CURL patch, and passes with the CURL patch. [ Risks ] No libcurl code is touched; it's merely a change of a few lines so that the Autotools build system builds the OpenLDAP-using code. With the new autopkgtest especially, the risks are very small. [ Checklist ] [X] *all* changes are documented in the d/changelog [X] I reviewed all changes and I approve them [X] attach debdiff against the package in (old)stable [X] the issue is verified as fixed in unstable [ Changes ] The changes consist of updating the package changelog, adding the upstream CURL patch to the patch series, and then the biggest change is the new autopkgtest. (I chose to write it in C, so you could say it's rather verbose.) The OpenLDAP maintainers are aware of the new autopkgtest and do not have reservations. [ Other info ] I do not have uploading privileges and this is not an NMU; this upload was prepared with the assistance and supervision of DD Samuel Henrique who has been maintaining CURL lately. He will upload the package when this unblock request is approved.
diff -Nru curl-7.88.1/debian/changelog curl-7.88.1/debian/changelog
--- curl-7.88.1/debian/changelog 2023-07-23 17:43:52.000000000 -0400
+++ curl-7.88.1/debian/changelog 2023-07-25 08:11:34.000000000 -0400
@@ -1,3 +1,13 @@
+curl (7.88.1-10+deb12u2) bookworm; urgency=medium
+
+ * Team upload.
+ * LDAP backend: correct the usage of OpenLDAP-specific functionality being
+ disabled with an upstream patch (Closes: #1041964)
+ This corrects the improper fetching of binary attributes.
+ * debian/tests: add a DEP-8 test that getting binary LDAP attributes works now
+
+ -- John Scott <jscott@posteo.net> Tue, 25 Jul 2023 08:11:34 -0400
+
curl (7.88.1-10+deb12u1) bookworm-security; urgency=medium
* Team upload.
diff -Nru curl-7.88.1/debian/patches/series curl-7.88.1/debian/patches/series
--- curl-7.88.1/debian/patches/series 2023-07-23 17:43:52.000000000 -0400
+++ curl-7.88.1/debian/patches/series 2023-07-25 08:11:34.000000000 -0400
@@ -31,3 +31,4 @@
# Used to generate packages for the other crypto libraries.
90_gnutls.patch
99_nss.patch
+Use-OpenLDAP-specific-functionality.patch
diff -Nru curl-7.88.1/debian/patches/Use-OpenLDAP-specific-functionality.patch curl-7.88.1/debian/patches/Use-OpenLDAP-specific-functionality.patch
--- curl-7.88.1/debian/patches/Use-OpenLDAP-specific-functionality.patch 1969-12-31 19:00:00.000000000 -0500
+++ curl-7.88.1/debian/patches/Use-OpenLDAP-specific-functionality.patch 2023-07-25 08:11:34.000000000 -0400
@@ -0,0 +1,35 @@
+Description: Fix Autotools not enabling OpenLDAP-specific functionality
+ The non-OpenLDAP code paths are less tested, less featureful, less secure,
+ and omitted in the build system by accident. It has been discovered that this
+ also mitigates curl not being able to make LDIF output when attributes have
+ binary values.
+Origin: upstream, https://github.com/curl/curl/commit/0ac6108856b9d500bc376d1d7e0b648d15499837.patch|commit:<commit-id>)
+Bug: https://github.com/curl/curl/issues/11372
+Applied-Upstream: 8.2.0, https://github.com/curl/curl/commit/0ac6108856b9d500bc376d1d7e0b648d15499837
+Reviewed-By: John Scott <jscott@posteo.net>
+Last-Update: 2023-07-25
+
+--- curl-7.88.1.orig/configure.ac
++++ curl-7.88.1/configure.ac
+@@ -1663,16 +1663,19 @@ if test x$CURL_DISABLE_LDAP != x1 ; then
+ fi
+
+ if test x$CURL_DISABLE_LDAP != x1 ; then
+- AC_CHECK_FUNCS([ldap_url_parse])
++ AC_CHECK_FUNCS([ldap_url_parse \
++ ldap_init_fd])
+
+ if test "$LDAPLIBNAME" = "wldap32"; then
+ curl_ldap_msg="enabled (winldap)"
+ AC_DEFINE(USE_WIN32_LDAP, 1, [Use Windows LDAP implementation])
+ else
+- curl_ldap_msg="enabled (OpenLDAP)"
+ if test "x$ac_cv_func_ldap_init_fd" = "xyes"; then
++ curl_ldap_msg="enabled (OpenLDAP)"
+ AC_DEFINE(USE_OPENLDAP, 1, [Use OpenLDAP-specific code])
+ AC_SUBST(USE_OPENLDAP, [1])
++ else
++ curl_ldap_msg="enabled (ancient OpenLDAP)"
+ fi
+ fi
+ fi
diff -Nru curl-7.88.1/debian/tests/control curl-7.88.1/debian/tests/control
--- curl-7.88.1/debian/tests/control 2023-07-23 17:43:52.000000000 -0400
+++ curl-7.88.1/debian/tests/control 2023-07-25 08:11:34.000000000 -0400
@@ -9,3 +9,7 @@
Tests: upstream-tests-nss
Depends: @builddeps@
Restrictions: allow-stderr
+
+Test-Command: gcc debian/tests/LDAP-binattr.c $(pkgconf --cflags --libs ldap libcurl) -o "$AUTOPKGTEST_TMP"/ldap-test && "$AUTOPKGTEST_TMP"/ldap-test
+Depends: gcc, libc-dev, libcurl4-openssl-dev | libcurl-dev, libldap-dev
+Restrictions: needs-root, allow-stderr, isolation-container
diff -Nru curl-7.88.1/debian/tests/LDAP-binattr.c curl-7.88.1/debian/tests/LDAP-binattr.c
--- curl-7.88.1/debian/tests/LDAP-binattr.c 1969-12-31 19:00:00.000000000 -0500
+++ curl-7.88.1/debian/tests/LDAP-binattr.c 2023-07-25 08:11:34.000000000 -0400
@@ -0,0 +1,284 @@
+/* SPDX-FileCopyrightText: 2023 John Scott <jscott@posteo.net>
+ * SPDX-FileCopyrightText: 2019 Ryan Tandy <ryan@nardis.ca>
+ * SPDX-License-Identifier: OLDAP-2.8 */
+
+/* Here's what we're going to do.
+ * We're going to install slapd, the greatest directory server on the planet,
+ * and use routines from the OpenLDAP API to communicate with it. We'll add a
+ * binary attribute, then curl will come in. Some versions of curl choke on
+ * binary attributes, so the primary purpose of this test is to make sure this
+ * does not happen. We'll read the LDIF from curl, use the OpenLDAP LDIF parsing
+ * routines to examine what we got, and make sure our binary attribute is what
+ * we set it to earlier. */
+
+#define _POSIX_C_SOURCE 200809L
+#include <locale.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <uchar.h>
+#include <unistd.h>
+#include <curl/curl.h>
+#include <ldap.h>
+#include <ldap_utf8.h>
+#include <ldif.h>
+
+#if __STDC_NO_VLA__
+#error variable-length array support is required
+/* It doesn't make sense for us to have to fiddle with malloc() when handling error message strings,
+ * since the cause of the error could very well be that we're out of heap memory. */
+#endif
+#if !__STDC_UTF_32__
+#pragma GCC warning "char32_t isn't UTF-32; that's weird"
+#endif
+
+/* Note that this is a different prototype than what the deprecated API version offers. */
+static void ldap_perror(int code, const char msg[restrict static 1]) {
+ const char *u8str = ldap_err2string(code);
+ int s = ldap_x_utf8s_to_mbs(NULL, u8str, 0, NULL);
+ if(s == -1) {
+ fputs("ldap_x_utf8s_to_mbs failed on an ldap_err2string string!\n", stderr);
+ /* The strings from ldap_err2string() should always be valid UTF-8
+ * and the UTF-8 conversion routines do not require memory allocation or other resources. */
+ abort();
+ }
+ char buf[strlen(msg) + strlen(": ") + s + sizeof("")];
+ if(ldap_x_utf8s_to_mbs(stpcpy(stpcpy(buf, msg), ": "), u8str, s + 1, NULL) == -1) {
+ fputs("ldap_x_utf8s_to_mbs failed on an ldap_err2string string!\n", stderr);
+ abort();
+ }
+ if(fputs(buf, stderr) == EOF || putc('\n', stderr) == EOF) {
+ perror("Failed to print error message string");
+ /* If we can't print on stderr, this might be our only error indication. */
+ abort();
+ }
+}
+
+int main(void) {
+ if(!setlocale(LC_ALL, "")) {
+ fputs("Failed to enable default locale\n", stderr);
+ exit(EXIT_FAILURE);
+ }
+
+ /* First, get slapd setup. */
+ if(setenv("DEBIAN_FRONTEND", "noninteractive", true) == -1) {
+ perror("Failed to set DEBIAN_FRONTEND environment variable");
+ exit(EXIT_FAILURE);
+ }
+
+ FILE *const debconf = popen("debconf-set-selections", "w");
+ if(!debconf) {
+ perror("Failed to open pipe to debconf-set-selections");
+ exit(EXIT_FAILURE);
+ }
+
+ if(fputs("slapd slapd/password1 password secret\n"
+ "slapd slapd/password2 password secret\n"
+ "slapd slapd/domain string example.com\n"
+ "slapd slapd/organization string example.com\n", debconf) == EOF) {
+ perror("Failed to send slapd options over pipe");
+ if(pclose(debconf) == -1) {
+ perror("Failed to close pipe");
+ }
+ exit(EXIT_FAILURE);
+ }
+
+ int w = pclose(debconf);
+ if(w == -1) {
+ perror("Failed to close pipe");
+ exit(EXIT_FAILURE);
+ }
+ if(!WIFEXITED(w) || WEXITSTATUS(w) != EXIT_SUCCESS) {
+ fputs("debconf-set-selections terminated abnormally\n", stderr);
+ exit(EXIT_FAILURE);
+ }
+
+ w = system("dpkg-reconfigure --frontend=noninteractive --priority=critical --force slapd && service slapd restart");
+ if(w == -1) {
+ perror("Failed to invoke dpkg and restart slapd");
+ exit(EXIT_FAILURE);
+ }
+ if(!WIFEXITED(w) || WEXITSTATUS(w) != EXIT_SUCCESS) {
+ fputs("Our attempt to call dpkg-reconfigure and restart slapd failed\n", stderr);
+ exit(EXIT_FAILURE);
+ }
+
+ /* Now the server is (probably) set up, so let us start the OpenLDAP routines.
+ * Note that in the world of libldap, everything that goes in and out is UTF-8.
+ * Hence using UTF-8 string literals, for example, is necessary. */
+ int p = ldap_set_option(NULL, LDAP_OPT_PROTOCOL_VERSION, &(int){LDAP_VERSION3});
+ if(p) {
+ ldap_perror(p, "Failed to enable LDAPv3");
+ exit(EXIT_FAILURE);
+ }
+
+ LDAP *ldp;
+ p = ldap_initialize(&ldp, u8"ldapi:///");
+ if(p) {
+ ldap_perror(p, "Failed to connect to LDAP server");
+ exit(EXIT_FAILURE);
+ }
+
+ p = ldap_sasl_bind_s(ldp, u8"CN=admin,DC=example,DC=com", LDAP_SASL_SIMPLE, &(struct berval){.bv_len = strlen(u8"secret"), .bv_val = (char[]){u8"secret"}}, NULL, NULL, NULL);
+ if(p) {
+ ldap_perror(p, "Failed to bind");
+ if(p = ldap_unbind_ext(ldp, NULL, NULL)) {
+ ldap_perror(p, "Failed to deinitialize LDAP");
+ }
+ exit(EXIT_FAILURE);
+ }
+
+ /* We're in!
+ * All of the compound literals are necessary for const-correctness. */
+ LDAPMod *addorg[] = {
+ &(LDAPMod) {
+ .mod_type = u8"objectClass",
+ .mod_values = (char*[]) {
+ (char[]) {
+ u8"top"
+ },
+ (char[]) {
+ u8"organizationalUnit"
+ },
+ NULL
+ },
+ },
+ &(LDAPMod) {
+ .mod_op = LDAP_MOD_BVALUES,
+ .mod_type = u8"description",
+ .mod_bvalues = (struct berval *[]) {
+ /* Let's store our description in UTF-32 which has embedded null bytes */
+ &(struct berval) {
+ /* Exclude the null character */
+ .bv_len = sizeof(U"This is some part of some organization.") - sizeof(char32_t),
+ .bv_val = (char *) &(char32_t[]) {
+ U"This is some part of some organization."
+ }
+ },
+ NULL
+ }
+ },
+ NULL
+ };
+
+ p = ldap_add_ext_s(ldp, u8"OU=Accounts,DC=example,DC=com", addorg, NULL, NULL);
+ if(p) {
+ ldap_perror(p,"Failed to add an organization in the directory server");
+ if(p = ldap_unbind_ext(ldp, NULL, NULL)) {
+ ldap_perror(p, "Failed to unbind from the directory server");
+ }
+ exit(EXIT_FAILURE);
+ }
+
+ if(p = ldap_unbind_ext(ldp, NULL, NULL)) {
+ ldap_perror(p, "Failed to unbind from the directory server");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Alright! Here's where libcurl comes in. */
+ CURLcode s = curl_global_init(CURL_GLOBAL_DEFAULT);
+ if(s) {
+ fprintf(stderr, "Failed to initialize libcurl: %s\n", curl_easy_strerror(s));
+ exit(EXIT_FAILURE);
+ }
+ if(atexit(curl_global_cleanup)) {
+ fputs("Failed to register exit handler\n", stderr);
+ curl_global_cleanup();
+ exit(EXIT_FAILURE);
+ }
+
+ CURL *const c = curl_easy_init();
+ if(!c) {
+ fputs("Failed to get a libcurl easy handle\n", stderr);
+ exit(EXIT_FAILURE);
+ }
+
+ char *ldif;
+ size_t ldiflen;
+ FILE *const ldifstream = open_memstream(&ldif, &ldiflen);
+ if(!ldifstream) {
+ perror("Failed to open memory stream");
+ curl_easy_cleanup(c);
+ exit(EXIT_FAILURE);
+ }
+
+ char errbuf[CURL_ERROR_SIZE];
+ if((s = curl_easy_setopt(c, CURLOPT_VERBOSE, 1L))
+ || (s = curl_easy_setopt(c, CURLOPT_WRITEDATA, (void *)ldifstream))
+ || (s = curl_easy_setopt(c, CURLOPT_ERRORBUFFER, errbuf))
+ || (s = curl_easy_setopt(c, CURLOPT_DEFAULT_PROTOCOL, "ldap"))
+ || (s = curl_easy_setopt(c, CURLOPT_UNIX_SOCKET_PATH, "/run/slapd/ldapi"))
+ || (s = curl_easy_setopt(c, CURLOPT_URL, u8"ldap://localhost/DC=example,DC=com?description?one?(description=*)"))) {
+ fprintf(stderr, "Failed to set a libcurl option: %s\n", curl_easy_strerror(s));
+closestream:
+ if(fclose(ldifstream) == EOF) {
+ perror("Failed to close memory stream");
+ } else {
+ free(ldif);
+ }
+ curl_easy_cleanup(c);
+ exit(EXIT_FAILURE);
+ }
+
+ if(curl_easy_perform(c)) {
+ fprintf(stderr, "libcurl failed to fetch description attributes over LDAP: %s\n", errbuf);
+ goto closestream;
+ }
+
+ curl_easy_cleanup(c);
+ if(fclose(ldifstream) == EOF) {
+ perror("Failed to close memory stream");
+ exit(EXIT_FAILURE);
+ }
+ puts(ldif);
+
+ /* Now we have the LDIF. Let's see if there are any null bytes.
+ * What we're checking is that the LDIF is completely a text file with no binary data intertwined. */
+ if(strnlen(ldif, ldiflen) != ldiflen) {
+ fputs("The LDIF from libcurl contains a null byte; that's not supposed to happen!\n", stderr);
+ const char *artifactdir;
+makeartifact:
+ artifactdir = getenv("AUTOPKGTEST_ARTIFACTS");
+ if(artifactdir && chdir(artifactdir) != -1) {
+ FILE *const artifact = fopen("curl.ldif", "wb");
+ if(artifact && fwrite(ldif, 1, ldiflen, artifact) == ldiflen && fclose(artifact) != EOF) {
+ fprintf(stderr, "The obtained LDIF file has been saved as 'curl.ldif' in the %s directory.\n", artifactdir);
+ }
+ }
+ free(ldif);
+ exit(EXIT_FAILURE);
+ }
+
+ /* So the LDIF might be valid, let's see if we can parse it with OpenLDAP's own routines.
+ * The first line is just the DN, so we skip it. */
+ char *newline = strchr(ldif, u8"\n"[0]);
+ if(!newline) {
+ fputs("LDIF does not contain a newline\n", stderr);
+ goto makeartifact;
+ }
+ newline++;
+ struct berval attrname, attrval;
+ p = ldif_parse_line2(newline, &attrname, &attrval, NULL);
+ if(p) {
+ ldap_perror(p, "Failed to parse LDIF with OpenLDAP routines");
+ goto makeartifact;
+ }
+
+ if(strcmp(attrname.bv_val, u8"description")) {
+ fprintf(stderr, "Attribute is '%s', was expecting 'description'\n", attrname.bv_val);
+ ldap_memfree(attrname.bv_val);
+ ldap_memfree(attrval.bv_val);
+ goto makeartifact;
+ }
+
+ ldap_memfree(attrname.bv_val);
+ if(memcmp(attrval.bv_val, U"This is some part of some organization.", sizeof(U"This is some part of some organization.") - sizeof(char32_t))) {
+ fputs("We didn't get the UTF-32 string we were expecting from the LDIF.\n", stderr);
+ ldap_memfree(attrval.bv_val);
+ goto makeartifact;
+ }
+
+ free(ldif);
+ /* success */
+}
Attachment:
signature.asc
Description: This is a digitally signed message part