]> git.ipfire.org Git - thirdparty/cups.git/commitdiff
Fix unresponsive cupsd process caused by a slow client
authorMichael R Sweet <msweet@msweet.org>
Tue, 18 Nov 2025 16:14:27 +0000 (17:14 +0100)
committerZdenek Dohnal <zdohnal@redhat.com>
Tue, 18 Nov 2025 16:14:27 +0000 (17:14 +0100)
If client is very slow, it will slow cupsd process for other clients.
The fix is the best effort without turning scheduler cupsd into
multithreaded process which would be too complex and error-prone when
backporting to 2.4.x series.

The fix for unencrypted communication is to follow up on communication
only if there is the whole line on input, and the waiting time is
guarded by timeout.

Encrypted communication now starts after we have the whole client hello
packet, which conflicts with optional upgrade support to HTTPS via
methods other than method OPTIONS, so this optional support defined in
RFC 2817, section 3.1 is removed. Too slow or incomplete requests are
handled by connection timeout.

Fixes CVE-2025-58436

cups/http-private.h
cups/http.c
cups/tls-gnutls.c
cups/tls-openssl.c
scheduler/client.c
scheduler/client.h
scheduler/select.c

index aa949e614fc6511d3c640cd69668da40835ae731..532a138c12537f810614cf3957113b428203362c 100644 (file)
@@ -83,6 +83,7 @@ extern "C" {
 // Constants...
 //
 
+#  define _HTTP_MAX_BUFFER     32768   /* Size of read buffer */
 #  define _HTTP_MAX_SBUFFER    65536   /* Size of (de)compression buffer */
 
 #  define _HTTP_TLS_NONE       0       /* No TLS options */
@@ -157,8 +158,8 @@ struct _http_s                              // HTTP connection structure
   http_encoding_t      data_encoding;  /* Chunked or not */
   int                  _data_remaining;/* Number of bytes left (deprecated) */
   int                  used;           /* Number of bytes used in buffer */
-  char                 buffer[HTTP_MAX_BUFFER];
-                                       /* Buffer for incoming data */
+  char                 _buffer[HTTP_MAX_BUFFER];
+                                       /* Old read buffer (deprecated) */
   int                  _auth_type;     /* Authentication in use (deprecated) */
   unsigned char                _md5_state[88]; /* MD5 state (deprecated) */
   char                 nonce[HTTP_MAX_VALUE];
@@ -231,6 +232,8 @@ struct _http_s                              // HTTP connection structure
                        *default_fields[HTTP_FIELD_MAX];
                                        /* Default field values, if any */
   /**** New in CUPS 2.5 ****/
+  char                 buffer[_HTTP_MAX_BUFFER];
+                                       /* Read buffer */
   char                 qop[HTTP_MAX_VALUE];
                                        /* Quality of Protection (qop) value from WWW-Authenticate */
 };
index a469fde2ba0dd40f8217c18af5fff9f70a2aa083..5cc41c3c05c8730f66c14e20c4ae52d8b4a08e9c 100644 (file)
@@ -37,7 +37,7 @@ static http_t         *http_create(const char *host, int port, http_addrlist_t *addrlis
 #ifdef DEBUG
 static void            http_debug_hex(const char *prefix, const char *buffer, int bytes);
 #endif // DEBUG
-static ssize_t         http_read(http_t *http, char *buffer, size_t length);
+static ssize_t         http_read(http_t *http, char *buffer, size_t length, int timeout);
 static ssize_t         http_read_buffered(http_t *http, char *buffer, size_t length);
 static ssize_t         http_read_chunk(http_t *http, char *buffer, size_t length);
 static bool            http_send(http_t *http, http_state_t request, const char *uri);
@@ -1435,7 +1435,7 @@ httpGets2(http_t *http,                   // I - HTTP connection
         return (NULL);
       }
 
-      bytes = http_read(http, http->buffer + http->used, (size_t)(HTTP_MAX_BUFFER - http->used));
+      bytes = http_read(http, http->buffer + http->used, (size_t)(_HTTP_MAX_BUFFER - http->used), http->wait_value);
 
       DEBUG_printf("4httpGets2: read " CUPS_LLFMT " bytes.", CUPS_LLCAST bytes);
 
@@ -1919,24 +1919,13 @@ httpPeek(http_t *http,                  // I - HTTP connection
     // Buffer small reads for better performance...
     ssize_t    buflen;                 // Length of read for buffer
 
-    if (!http->blocking)
-    {
-      while (!httpWait(http, http->wait_value))
-      {
-       if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
-         continue;
-
-       return (0);
-      }
-    }
-
     if ((size_t)http->data_remaining > sizeof(http->buffer))
       buflen = sizeof(http->buffer);
     else
       buflen = (ssize_t)http->data_remaining;
 
     DEBUG_printf("2httpPeek: Reading %d bytes into buffer.", (int)buflen);
-    bytes = http_read(http, http->buffer, (size_t)buflen);
+    bytes = http_read(http, http->buffer, (size_t)buflen, http->wait_value);
 
     DEBUG_printf("2httpPeek: Read " CUPS_LLFMT " bytes into buffer.", CUPS_LLCAST bytes);
     if (bytes > 0)
@@ -1954,9 +1943,9 @@ httpPeek(http_t *http,                    // I - HTTP connection
     int                zerr;                   // Decompressor error
     z_stream   stream;                 // Copy of decompressor stream
 
-    if (http->used > 0 && ((z_stream *)http->stream)->avail_in < HTTP_MAX_BUFFER)
+    if (http->used > 0 && ((z_stream *)http->stream)->avail_in < _HTTP_MAX_BUFFER)
     {
-      size_t buflen = HTTP_MAX_BUFFER - ((z_stream *)http->stream)->avail_in;
+      size_t buflen = _HTTP_MAX_BUFFER - ((z_stream *)http->stream)->avail_in;
                                        // Number of bytes to copy
 
       if (((z_stream *)http->stream)->avail_in > 0 && ((z_stream *)http->stream)->next_in > http->sbuffer)
@@ -2205,7 +2194,7 @@ httpRead2(http_t *http,                   // I - HTTP connection
 
       if (bytes == 0)
       {
-        ssize_t buflen = HTTP_MAX_BUFFER - (ssize_t)((z_stream *)http->stream)->avail_in;
+        ssize_t buflen = _HTTP_MAX_BUFFER - (ssize_t)((z_stream *)http->stream)->avail_in;
                                        // Additional bytes for buffer
 
         if (buflen > 0)
@@ -2904,7 +2893,7 @@ int                                       // O - 1 to continue, 0 to stop
 _httpUpdate(http_t        *http,       // I - HTTP connection
             http_status_t *status)     // O - Current HTTP status
 {
-  char         line[32768],            // Line from connection...
+  char         line[_HTTP_MAX_BUFFER], // Line from connection...
                *value;                 // Pointer to value on line
   http_field_t field;                  // Field index
   int          major, minor;           // HTTP version numbers
@@ -2912,9 +2901,43 @@ _httpUpdate(http_t        *http, // I - HTTP connection
 
   DEBUG_printf("_httpUpdate(http=%p, status=%p), state=%s", (void *)http, (void *)status, httpStateString(http->state));
 
+  // When doing non-blocking I/O, make sure we have a whole line...
+  if (!http->blocking)
+  {
+    ssize_t    bytes;                  // Bytes "peeked" from connection
+
+    // See whether our read buffer is full...
+    DEBUG_printf("2_httpUpdate: used=%d", http->used);
+
+    if (http->used > 0 && !memchr(http->buffer, '\n', (size_t)http->used) && (size_t)http->used < sizeof(http->buffer))
+    {
+      // No, try filling in more data...
+      if ((bytes = http_read(http, http->buffer + http->used, sizeof(http->buffer) - (size_t)http->used, /*timeout*/0)) > 0)
+      {
+       DEBUG_printf("2_httpUpdate: Read %d bytes.", (int)bytes);
+       http->used += (int)bytes;
+      }
+    }
+
+    // Peek at the incoming data...
+    if (!http->used || !memchr(http->buffer, '\n', (size_t)http->used))
+    {
+      // Don't have a full line, tell the reader to try again when there is more data...
+      DEBUG_puts("1_htttpUpdate: No newline in buffer yet.");
+      if ((size_t)http->used == sizeof(http->buffer))
+       *status = HTTP_STATUS_ERROR;
+      else
+       *status = HTTP_STATUS_CONTINUE;
+      return (0);
+    }
+
+    DEBUG_puts("2_httpUpdate: Found newline in buffer.");
+  }
+
   // Grab a single line from the connection...
   if (!httpGets2(http, line, sizeof(line)))
   {
+    DEBUG_puts("1_httpUpdate: Error reading request line.");
     *status = HTTP_STATUS_ERROR;
     return (0);
   }
@@ -4100,7 +4123,8 @@ http_debug_hex(const char *prefix,        // I - Prefix for line
 static ssize_t                         // O - Number of bytes read or -1 on error
 http_read(http_t *http,                        // I - HTTP connection
           char   *buffer,              // I - Buffer
-          size_t length)               // I - Maximum bytes to read
+          size_t length,               // I - Maximum bytes to read
+          int    timeout)              // I - Wait timeout
 {
   ssize_t      bytes;                  // Bytes read
 
@@ -4109,7 +4133,7 @@ http_read(http_t *http,                   // I - HTTP connection
 
   if (!http->blocking || http->timeout_value > 0.0)
   {
-    while (!httpWait(http, http->wait_value))
+    while (!_httpWait(http, timeout, 1))
     {
       if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
        continue;
@@ -4212,7 +4236,7 @@ http_read_buffered(http_t *http,  // I - HTTP connection
     else
       bytes = (ssize_t)length;
 
-    DEBUG_printf("8http_read: Grabbing %d bytes from input buffer.", (int)bytes);
+    DEBUG_printf("8http_read_buffered: Grabbing %d bytes from input buffer.", (int)bytes);
 
     memcpy(buffer, http->buffer, (size_t)bytes);
     http->used -= (int)bytes;
@@ -4222,7 +4246,7 @@ http_read_buffered(http_t *http,  // I - HTTP connection
   }
   else
   {
-    bytes = http_read(http, buffer, length);
+    bytes = http_read(http, buffer, length, http->wait_value);
   }
 
   return (bytes);
@@ -4535,16 +4559,14 @@ http_set_timeout(int    fd,             // I - File descriptor
 static void
 http_set_wait(http_t *http)            // I - HTTP connection
 {
-  if (http->blocking)
-  {
-    http->wait_value = (int)(http->timeout_value * 1000);
+  http->wait_value = (int)(http->timeout_value * 1000);
 
-    if (http->wait_value <= 0)
-      http->wait_value = 60000;
-  }
-  else
+  if (http->wait_value <= 0)
   {
-    http->wait_value = 10000;
+    if (http->blocking)
+      http->wait_value = 60000;
+    else
+      http->wait_value = 1000;
   }
 }
 
index b2a1094f049bb49913c8b8c288fb030e3b7a5f46..2fe7ef1965e1147ef21e9c9b036da49c7daf2d66 100644 (file)
@@ -1832,7 +1832,7 @@ _httpTLSStart(http_t *http)               // I - Connection to server
 
       if (!cupsCreateCredentials(tls_keypath, false, CUPS_CREDPURPOSE_SERVER_AUTH, CUPS_CREDTYPE_DEFAULT, CUPS_CREDUSAGE_DEFAULT_TLS, NULL, NULL, NULL, NULL, NULL, cn, /*email*/NULL, 0, NULL, NULL, time(NULL) + 3650 * 86400))
       {
-       DEBUG_puts("4_httpTLSStart: cupsCreateCredentials failed.");
+       DEBUG_printf("4_httpTLSStart: cupsCreateCredentials failed: %s", cupsGetErrorString());
        http->error  = errno = EINVAL;
        http->status = HTTP_STATUS_ERROR;
        cupsMutexUnlock(&tls_mutex);
index b4a654841ba9643d53f9630f4846dc15a2994477..2886a3aa8fd9f611be50a3c18c6281cde02d7074 100644 (file)
@@ -1900,7 +1900,7 @@ _httpTLSStart(http_t *http)               // I - Connection to server
 
       if (!cupsCreateCredentials(tls_keypath, false, CUPS_CREDPURPOSE_SERVER_AUTH, CUPS_CREDTYPE_DEFAULT, CUPS_CREDUSAGE_DEFAULT_TLS, NULL, NULL, NULL, NULL, NULL, cn, NULL, 0, NULL, NULL, time(NULL) + 3650 * 86400))
       {
-       DEBUG_puts("4_httpTLSStart: cupsCreateCredentials failed.");
+       DEBUG_printf("4_httpTLSStart: cupsCreateCredentials failed: %s", cupsGetErrorString());
        http->error  = errno = EINVAL;
        http->status = HTTP_STATUS_ERROR;
        SSL_CTX_free(context);
@@ -2190,11 +2190,14 @@ http_bio_read(BIO  *h,                  // I - BIO data
   http = (http_t *)BIO_get_data(h);
   DEBUG_printf("9http_bio_read: http=%p", (void *)http);
 
-  if (!http->blocking)
+  if (!http->blocking || http->timeout_value > 0.0)
   {
     // Make sure we have data before we read...
-    if (!_httpWait(http, 10000, 0))
+    while (!_httpWait(http, http->wait_value, 0))
     {
+      if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
+       continue;
+
 #ifdef WIN32
       http->error = WSAETIMEDOUT;
 #else
index dbb82c293c2a94e9f02b0a92180d9324d0dd5a91..065e873ee853248f237492e2b520bffcca82aa77 100644 (file)
@@ -30,8 +30,8 @@
 
 static int             check_if_modified(cupsd_client_t *con,
                                          struct stat *filestats);
+static int             check_start_tls(cupsd_client_t *con);
 static int             compare_clients(cupsd_client_t *a, cupsd_client_t *b, void *data);
-static int             cupsd_start_tls(cupsd_client_t *con, http_encryption_t e);
 static char            *get_file(cupsd_client_t *con, struct stat *filestats, char *filename, size_t len);
 static http_status_t   install_cupsd_conf(cupsd_client_t *con);
 static int             is_cgi(cupsd_client_t *con, const char *filename, struct stat *filestats, mime_type_t *type);
@@ -354,14 +354,20 @@ cupsdAcceptClient(cupsd_listener_t *lis)/* I - Listener socket */
   if (lis->encryption == HTTP_ENCRYPTION_ALWAYS)
   {
    /*
-    * https connection; go secure...
+    * HTTPS connection, force TLS negotiation...
     */
 
-    if (cupsd_start_tls(con, HTTP_ENCRYPTION_ALWAYS))
-      cupsdCloseClient(con);
+    con->tls_start     = time(NULL);
+    con->encryption = HTTP_ENCRYPTION_ALWAYS;
   }
   else
+  {
+   /*
+    * HTTP connection, but check for HTTPS negotiation on first data...
+    */
+
     con->auto_ssl = 1;
+  }
 }
 
 
@@ -597,17 +603,46 @@ cupsdReadClient(cupsd_client_t *con)      /* I - Client to read from */
 
     con->auto_ssl = 0;
 
-    if (recv(httpGetFd(con->http), buf, 1, MSG_PEEK) == 1 &&
-        (!buf[0] || !strchr("DGHOPT", buf[0])))
+    if (recv(httpGetFd(con->http), buf, 5, MSG_PEEK) == 5 && buf[0] == 0x16 && buf[1] == 3 && buf[2])
     {
      /*
-      * Encrypt this connection...
+      * Client hello record, encrypt this connection...
       */
 
-      cupsdLogClient(con, CUPSD_LOG_DEBUG2, "Saw first byte %02X, auto-negotiating SSL/TLS session.", buf[0] & 255);
+      cupsdLogClient(con, CUPSD_LOG_DEBUG2, "Saw client hello record, auto-negotiating TLS session.");
+      con->tls_start = time(NULL);
+      con->encryption = HTTP_ENCRYPTION_ALWAYS;
+    }
+  }
+
+  if (con->tls_start)
+  {
+   /*
+    * Try negotiating TLS...
+    */
+
+    int tls_status = check_start_tls(con);
 
-      if (cupsd_start_tls(con, HTTP_ENCRYPTION_ALWAYS))
-        cupsdCloseClient(con);
+    if (tls_status < 0)
+    {
+     /*
+      * TLS negotiation failed, close the connection.
+      */
+
+      cupsdCloseClient(con);
+      return;
+    }
+    else if (tls_status == 0)
+    {
+     /*
+      * Nothing to do yet...
+      */
+
+      if ((time(NULL) - con->tls_start) > 5)
+      {
+       // Timeout, close the connection...
+       cupsdCloseClient(con);
+      }
 
       return;
     }
@@ -775,9 +810,7 @@ cupsdReadClient(cupsd_client_t *con)        /* I - Client to read from */
         * Parse incoming parameters until the status changes...
        */
 
-        while ((status = httpUpdate(con->http)) == HTTP_STATUS_CONTINUE)
-         if (!httpGetReady(con->http))
-           break;
+       status = httpUpdate(con->http);
 
        if (status != HTTP_STATUS_OK && status != HTTP_STATUS_CONTINUE)
        {
@@ -940,11 +973,10 @@ cupsdReadClient(cupsd_client_t *con)      /* I - Client to read from */
          return;
        }
 
-        if (cupsd_start_tls(con, HTTP_ENCRYPTION_REQUIRED))
-        {
-         cupsdCloseClient(con);
-         return;
-       }
+       con->tls_start = time(NULL);
+       con->tls_upgrade = 1;
+       con->encryption = HTTP_ENCRYPTION_REQUIRED;
+       return;
       }
 
       httpClearFields(con->http);
@@ -974,30 +1006,6 @@ cupsdReadClient(cupsd_client_t *con)      /* I - Client to read from */
     }
     else
     {
-      if (!_cups_strcasecmp(httpGetField(con->http, HTTP_FIELD_CONNECTION),
-                            "Upgrade") && !httpIsEncrypted(con->http))
-      {
-       /*
-        * Do encryption stuff...
-       */
-
-        httpClearFields(con->http);
-        httpClearCookie(con->http);
-
-       if (!cupsdSendHeader(con, HTTP_STATUS_SWITCHING_PROTOCOLS, NULL,
-                            CUPSD_AUTH_NONE))
-       {
-         cupsdCloseClient(con);
-         return;
-       }
-
-        if (cupsd_start_tls(con, HTTP_ENCRYPTION_REQUIRED))
-        {
-         cupsdCloseClient(con);
-         return;
-       }
-      }
-
       if ((status = cupsdIsAuthorized(con, NULL)) != HTTP_STATUS_OK)
       {
        cupsdSendError(con, status, CUPSD_AUTH_NONE);
@@ -2702,6 +2710,67 @@ check_if_modified(
 }
 
 
+/*
+ * 'check_start_tls()' - Start encryption on a connection.
+ */
+
+static int                             /* O - 0 to continue, 1 on success, -1 on error */
+check_start_tls(cupsd_client_t *con)   /* I - Client connection */
+{
+  unsigned char        chello[4096];           /* Client hello record */
+  ssize_t      chello_bytes;           /* Bytes read/peeked */
+  int          chello_len;             /* Length of record */
+
+
+ /*
+  * See if we have a good and complete client hello record...
+  */
+
+  if ((chello_bytes = recv(httpGetFd(con->http), (char *)chello, sizeof(chello), MSG_PEEK)) < 5)
+    return (0);                                /* Not enough bytes (yet) */
+
+  if (chello[0] != 0x016 || chello[1] != 3 || chello[2] == 0)
+    return (-1);                       /* Not a TLS Client Hello record */
+
+  chello_len = (chello[3] << 8) | chello[4];
+
+  if ((chello_len + 5) > chello_bytes)
+    return (0);                                /* Not enough bytes yet */
+
+ /*
+  * OK, we do, try negotiating...
+  */
+
+  con->tls_start = 0;
+
+  if (!httpSetEncryption(con->http, con->encryption))
+  {
+    cupsdLogClient(con, CUPSD_LOG_ERROR, "Unable to encrypt connection: %s", cupsGetErrorString());
+    return (-1);
+  }
+
+  cupsdLogClient(con, CUPSD_LOG_DEBUG, "Connection now encrypted.");
+
+  if (con->tls_upgrade)
+  {
+    // Respond to the original OPTIONS command...
+    con->tls_upgrade = 0;
+
+    httpClearFields(con->http);
+    httpClearCookie(con->http);
+    httpSetField(con->http, HTTP_FIELD_CONTENT_LENGTH, "0");
+
+    if (!cupsdSendHeader(con, HTTP_STATUS_OK, NULL, CUPSD_AUTH_NONE))
+    {
+      cupsdCloseClient(con);
+      return (-1);
+    }
+  }
+
+  return (1);
+}
+
+
 /*
  * 'compare_clients()' - Compare two client connections.
  */
@@ -2722,26 +2791,6 @@ compare_clients(cupsd_client_t *a, /* I - First client */
 }
 
 
-/*
- * 'cupsd_start_tls()' - Start encryption on a connection.
- */
-
-static int                             /* O - 0 on success, -1 on error */
-cupsd_start_tls(cupsd_client_t    *con,        /* I - Client connection */
-                http_encryption_t e)   /* I - Encryption mode */
-{
-  if (!httpSetEncryption(con->http, e))
-  {
-    cupsdLogClient(con, CUPSD_LOG_ERROR, "Unable to encrypt connection: %s",
-                   cupsGetErrorString());
-    return (-1);
-  }
-
-  cupsdLogClient(con, CUPSD_LOG_DEBUG, "Connection now encrypted.");
-  return (0);
-}
-
-
 /*
  * 'get_file()' - Get a filename and state info.
  */
index 0db5e8f4977d221dfe1ab6cd632196522d53733a..218238e29acef6a42e0bc366c9e2a7304441e943 100644 (file)
@@ -57,6 +57,9 @@ struct cupsd_client_s
   char                 header[4096];   /* Header from CGI program */
   cups_lang_t          *language;      /* Language to use */
   int                  auto_ssl;       /* Automatic test for SSL/TLS */
+  time_t               tls_start;      /* Start time for TLS negotiation */
+  int                  tls_upgrade;    /* Doing TLS upgrade via OPTIONS? */
+  http_encryption_t    encryption;     /* Type of TLS negotiation */
   http_addr_t          clientaddr;     /* Client's server address */
   char                 clientname[256];/* Client's server name for connection */
   int                  clientport;     /* Client's server port for connection */
index be225a9f91d5fa46c9ba48b5c6a49d77966e91cd..b05100191e5ed8538fc752826367e96b5998673f 100644 (file)
@@ -296,6 +296,9 @@ cupsdDoSelect(long timeout)         // I - Timeout in seconds
     }
   }
 
+  // Prevent 100% CPU by releasing control before the poll call...
+  usleep(1);
+
   if (timeout >= 0 && timeout < 86400)
     nfds = poll(cupsd_pollfds, (nfds_t)count, timeout * 1000);
   else