Merge pull request #2751 from telefonicaid/hardening/2724_mhd_to_use_…

…poll

Hardening/2724 mhd to use poll

CHANGES_NEXT_RELEASE

@@ -0,0 +1,2 @@
+- Fix: libmicrohttpd now uses poll()/epoll() and not select() so fds greater than 1023 can be used for incoming connections (#2724, #2166)
+- Fix: modified the upper limit for CLI '-maxConnections', from 1020 to UNLIMITED

doc/manuals/admin/cli.md

@@ -138,8 +138,8 @@ The list of available options is the following:
     internal logic point of view.
 -   **-connectionMemory**. Sets the size of the connection memory buffer (in kB) per connection used internally
     by the HTTP server library. Default value is 64 kB.
--   **-maxConnections**. Maximum number of simultaneous connections. Default value is "unlimited" (limited by 
-    max file descriptors of operating system).
+-   **-maxConnections**. Maximum number of simultaneous connections. Default value is 1020, for legacy reasons,
+    while the lower limit is 1 and there is no upper limit (limited by max file descriptors of the operating system).
 -   **-reqPoolSize**. Size of thread pool for incoming connections. Default value is 0, meaning *no thread pool*.
 -   **-statCounters**, **-statSemWait**, **-statTiming** and **-statNotifQueue**. Enable statistics
     generation. See [statistics documentation](statistics.md).

src/app/contextBroker/contextBroker.cpp

@@ -333,16 +333,10 @@ bool            logForHumans;
 *
 * paArgs - option vector for the Parse CLI arguments library
 *
-* NOTE
-*   A note about 'FD_SETSIZE - 4', the default and max value for '-maxConnections':
-*   [ taken from https://www.gnu.org/software/libmicrohttpd/manual/libmicrohttpd.html ]
-*
-*   MHD_OPTION_CONNECTION_LIMIT
-*     Maximum number of concurrent connections to accept.
-*     The default is FD_SETSIZE - 4 (the maximum number of file descriptors supported by 
-*     select minus four for stdin, stdout, stderr and the server socket). In other words,
-*    the default is as large as possible.
-*
+* A note about the default value of -maxConnections.
+* In older implementations of the broker, select was used in MHD and not poll/epoll.
+* The old default value (1024 - 4), that was a recommendation by MHD, has been kept.
+* More info about this can be found in the documentation of MHD.
 */
 PaArgument paArgs[] =
 {
@@ -379,7 +373,7 @@ PaArgument paArgs[] =
   { "-subCacheIval",     &subCacheInterval, "SUBCACHE_IVAL",     PaInt,    PaOpt, 60,             0,     3600,     SUB_CACHE_IVAL_DESC    },
   { "-noCache",          &noCache,          "NOCACHE",           PaBool,   PaOpt, false,          false, true,     NO_CACHE               },
   { "-connectionMemory", &connectionMemory, "CONN_MEMORY",       PaUInt,   PaOpt, 64,             0,     1024,     CONN_MEMORY_DESC       },  
-  { "-maxConnections",   &maxConnections,   "MAX_CONN",          PaUInt,   PaOpt, FD_SETSIZE - 4, 0,     FD_SETSIZE - 4, MAX_CONN_DESC    },
+  { "-maxConnections",   &maxConnections,   "MAX_CONN",          PaUInt,   PaOpt, 1020,           1,     PaNL,     MAX_CONN_DESC          },
   { "-reqPoolSize",      &reqPoolSize,      "TRQ_POOL_SIZE",     PaUInt,   PaOpt, 0,              0,     1024,     REQ_POOL_SIZE          },
 
   { "-notificationMode",      &notificationMode,      "NOTIF_MODE", PaString, PaOpt, _i "transient", PaNL,  PaNL, NOTIFICATION_MODE_DESC },

src/lib/common/limits.h

@@ -105,16 +105,6 @@
 
 
 
-/* ****************************************************************************
-*
-* CONSTANTS RESTINIT - 
-*/ 
-#define   DEFAULT_CONNECTION_MEM        64
-#define   DEFAULT_MAX_CONNECTIONS       128
-#define   DEFAULT_MHD_THREAD_POOLSIZE   128
-
-
-
 /* ****************************************************************************
 *
 * HTTP header maximum lengths

src/lib/rest/rest.cpp

@@ -1421,12 +1421,18 @@ static int connectionTreat
 *   Number (unsigned int) of threads in thread pool. Enable thread pooling by setting this value to to something greater than 1.
 *   Currently, thread model must be MHD_USE_SELECT_INTERNALLY if thread pooling is enabled (MHD_start_daemon returns NULL for
 *   an unsupported thread model).
+*
+* MHD_USE_EPOLL:
+*   Use `epoll()` instead of `select()` or `poll()` for the event loop.
+*   This option is only available on some systems; using the option on
+*   systems without epoll will cause #MHD_start_daemon to fail.  Using
+*   this option is not supported with #MHD_USE_THREAD_PER_CONNECTION.
 */
 static int restStart(IpVersion ipVersion, const char* httpsKey = NULL, const char* httpsCertificate = NULL)
 {
   bool      mhdStartError  = true;
   size_t    memoryLimit    = connMemory * 1024; // Connection memory is expressed in kilobytes
-  MHD_FLAG  serverMode     = MHD_USE_THREAD_PER_CONNECTION;
+  int       serverMode     = MHD_USE_THREAD_PER_CONNECTION | MHD_USE_POLL;
 
   if (port == 0)
   {
@@ -1435,7 +1441,16 @@ static int restStart(IpVersion ipVersion, const char* httpsKey = NULL, const cha
 
   if (threadPoolSize != 0)
   {
-    serverMode = MHD_USE_SELECT_INTERNALLY;
+    //
+    // To use poll() instead of select(), MHD 0.9.48 has the define MHD_USE_EPOLL_LINUX_ONLY,
+    // while in MHD 0.9.51, the name of the define has changed to MHD_USE_EPOLL.
+    // So, to support both names, we need a ifdef/else cpp directive here.
+    //
+#ifdef MHD_USE_EPOLL
+    serverMode = MHD_USE_SELECT_INTERNALLY | MHD_USE_EPOLL;
+#else
+    serverMode = MHD_USE_SELECT_INTERNALLY | MHD_USE_EPOLL_LINUX_ONLY;
+#endif
   }
 
 
@@ -1452,8 +1467,9 @@ static int restStart(IpVersion ipVersion, const char* httpsKey = NULL, const cha
 
     if ((httpsKey != NULL) && (httpsCertificate != NULL))
     {
-      LM_T(LmtMhd, ("Starting HTTPS daemon on IPv4 %s port %d", bindIp, port));
-      mhdDaemon = MHD_start_daemon(serverMode | MHD_USE_SSL,
+      serverMode |= MHD_USE_SSL;
+      LM_T(LmtMhd, ("Starting HTTPS daemon on IPv4 %s port %d, serverMode: 0x%x", bindIp, port, serverMode));
+      mhdDaemon = MHD_start_daemon(serverMode,
                                    htons(port),
                                    NULL,
                                    NULL,
@@ -1470,7 +1486,7 @@ static int restStart(IpVersion ipVersion, const char* httpsKey = NULL, const cha
     }
     else
     {
-      LM_T(LmtMhd, ("Starting HTTP daemon on IPv4 %s port %d", bindIp, port));
+      LM_T(LmtMhd, ("Starting HTTP daemon on IPv4 %s port %d, serverMode: 0x%x", bindIp, port, serverMode));
       mhdDaemon = MHD_start_daemon(serverMode,
                                    htons(port),
                                    NULL,
@@ -1502,10 +1518,13 @@ static int restStart(IpVersion ipVersion, const char* httpsKey = NULL, const cha
     sad_v6.sin6_family = AF_INET6;
     sad_v6.sin6_port = htons(port);
 
+    serverMode |= MHD_USE_IPv6;
+
     if ((httpsKey != NULL) && (httpsCertificate != NULL))
     {
-      LM_T(LmtMhd, ("Starting HTTPS daemon on IPv6 %s port %d", bindIPv6, port));
-      mhdDaemon_v6 = MHD_start_daemon(serverMode | MHD_USE_IPv6 | MHD_USE_SSL,
+      serverMode |= MHD_USE_SSL;
+      LM_T(LmtMhd, ("Starting HTTPS daemon on IPv6 %s port %d, serverMode: 0x%x", bindIPv6, port, serverMode));
+      mhdDaemon_v6 = MHD_start_daemon(serverMode,
                                       htons(port),
                                       NULL,
                                       NULL,
@@ -1521,8 +1540,8 @@ static int restStart(IpVersion ipVersion, const char* httpsKey = NULL, const cha
     }
     else
     {
-      LM_T(LmtMhd, ("Starting HTTP daemon on IPv6 %s port %d", bindIPv6, port));
-      mhdDaemon_v6 = MHD_start_daemon(serverMode | MHD_USE_IPv6,
+      LM_T(LmtMhd, ("Starting HTTP daemon on IPv6 %s port %d, serverMode: 0x%x", bindIPv6, port, serverMode));
+      mhdDaemon_v6 = MHD_start_daemon(serverMode,
                                       htons(port),
                                       NULL,
                                       NULL,

src/lib/rest/rest.h

@@ -92,13 +92,13 @@ extern void restInit
    IpVersion           _ipVersion,
    const char*         _bindAddress, 
    unsigned short      _port,
-   bool                _multitenant       = false,
-   unsigned int        _connectionMemory  = DEFAULT_CONNECTION_MEM,
-   unsigned int        _maxConnections    = DEFAULT_MAX_CONNECTIONS,
-   unsigned int        _mhdThreadPoolSize = DEFAULT_MHD_THREAD_POOLSIZE,
-   const std::string&  _rushHost          = "",
-   unsigned short      _rushPort          = NO_PORT,
-   const char*         _allowedOrigin     = NULL,
+   bool                _multitenant,
+   unsigned int        _connectionMemory,
+   unsigned int        _maxConnections,
+   unsigned int        _mhdThreadPoolSize,
+   const std::string&  _rushHost,
+   unsigned short      _rushPort,
+   const char*         _allowedOrigin,
    const char*         _httpsKey          = NULL,
    const char*         _httpsCert         = NULL,
    RestServeFunction   _serveFunction     = NULL