On Sat, Feb 06, 2021 at 01:32:35AM +0100, [email protected] wrote:
> commit 6d7c7b6ff701fafc2a649b21a66a92a9ab626221
> Author:     Laslo Hunhold <[email protected]>
> AuthorDate: Sat Feb 6 01:15:53 2021 +0100
> Commit:     Laslo Hunhold <[email protected]>
> CommitDate: Sat Feb 6 01:32:17 2021 +0100
> 
>     Apply (D)DoS-Hardening
>     
>     Until now, if quark found that in case of an incoming connection it
>     didn't have any vacant connection slots left, it would just not
>     accept() and thus block any further new connections until a slot was
>     free.
>     
>     This may sound reasonable at first, but there are cases where the
>     connected clients are not benevolent and might firmly occupy all slots
>     with simple request flooding or more advanced attacks like slowloris,
>     R-U-Dead-Yet or Slow Read, which all boil down to sending or receiving
>     data really slowly. The latter attacks are very effective and require
>     very little resources on the attacker's side.
>     

>     Thus, the only way to keep things going is to bite the bullet and always
>     accept a connection, even if it means dropping another connection for
>     it. In this case, an algorithm determines which client has the most
>     connections and drops the least-advanced connection of it (i.e. a
>     connection in the earliest possible stage).
>     

I don't think this is the right approach for reliability sake.

Is all this added complexity worth it compared to the previous non-OS-specific
code (pre-kqueue)?  Have you even tested the kqueue code? Last time it did not
even compile.

This DDoS-hardening does not protect against a botnet attack anyway. Very basic
attacks can be worked around with firewall/pf rules.

The quark pre-FRIGN-era was much simpler and more readable.

>     Stress-tests with slowhttptest[0] and siege yielded excellent results,
>     where quark always remained responsive for the normal visitor despite
>     an active massive DoS-attack using the aforementioned methods.
>     
>     Side-note: In the spirit of not having any memory allocations in quark
>     during "runtime", the constant-space algorithm used to determine the
>     client with the most connections is quadratic in time over the number
>     of slots. This, however, is not a big deal, given the number of slots
>     is always relatively small and such an out-of-connection-scenario is
>     an edge-case.
>     
>     [0]:https://github.com/shekyan/slowhttptest
>     [1]:https://github.com/JoeDog/siege
>     
>     Signed-off-by: Laslo Hunhold <[email protected]>
> 
> diff --git a/main.c b/main.c
> index f366985..4506c3e 100644
> --- a/main.c
> +++ b/main.c
> @@ -45,15 +45,25 @@ logmsg(const struct connection *c)
>               inaddr_str[0] = '\0';
>       }
>  
> -     printf("%s\t%s\t%d\t%s\t%s%s%s%s%s\n", tstmp, inaddr_str, c->res.status,
> -            c->req.field[REQ_HOST], c->req.path, c->req.query[0] ? "?" : "",
> -            c->req.query, c->req.fragment[0] ? "#" : "", c->req.fragment);
> +     printf("%s\t%s\t%s%.*d\t%s\t%s%s%s%s%s\n",
> +            tstmp,
> +            inaddr_str,
> +            (c->res.status == 0) ? "dropped" : "",
> +            (c->res.status == 0) ? 0 : 3,
> +            c->res.status,
> +            c->req.field[REQ_HOST][0] ? c->req.field[REQ_HOST] : "-",
> +            c->req.path[0] ? c->req.path : "-",
> +            c->req.query[0] ? "?" : "",
> +            c->req.query,
> +            c->req.fragment[0] ? "#" : "",
> +            c->req.fragment);
>  }
>  
>  static void
> -close_connection(struct connection *c)
> +reset_connection(struct connection *c)
>  {
>       if (c != NULL) {
> +             shutdown(c->fd, SHUT_RDWR);
>               close(c->fd);
>               memset(c, 0, sizeof(*c));
>       }
> @@ -152,7 +162,53 @@ response:
>       }
>  err:
>       logmsg(c);
> -     close_connection(c);
> +     reset_connection(c);
> +}
> +
> +static struct connection *
> +get_connection_drop_candidate(struct connection *connection, size_t nslots)
> +{
> +     struct connection *c, *minc;
> +     size_t i, j, maxcnt, cnt;
> +
> +     /*
> +      * determine the most-unimportant connection 'minc' of the in-address
> +      * with most connections; this algorithm has a complexity of O(n²)
> +      * in time but is O(1) in space; there are algorithms with O(n) in
> +      * time and space, but this would require memory allocation,
> +      * which we avoid. Given the simplicity of the inner loop and
> +      * relatively small number of slots per thread, this is fine.
> +      */
> +     for (i = 0, minc = NULL, maxcnt = 0; i < nslots; i++) {
> +             /*
> +              * the c is used to minimize in regard to importance
> +              * within the same-address-group
> +              */
> +             c = &connection[i];
> +
> +             for (j = 0, cnt = 0; j < nslots; j++) {
> +                     if (!sock_same_addr(&connection[i].ia,
> +                                         &connection[j].ia)) {
> +                             continue;
> +                     }
> +                     cnt++;
> +
> +                     if (connection[j].state < c->state) {
> +                             /*
> +                              * we have found a connection with an
> +                              * even lower state and thus lower
> +                              * importance
> +                              */
> +                             c = &connection[j];
> +                     }
> +             }
> +             if (cnt > maxcnt) {
> +                     minc = c;
> +                     maxcnt = cnt;
> +             }
> +     }
> +
> +     return minc;
>  }
>  
>  struct connection *
> @@ -160,33 +216,61 @@ accept_connection(int insock, struct connection 
> *connection,
>                    size_t nslots)
>  {
>       struct connection *c = NULL;
> -     size_t j;
> +     size_t i;
>  
>       /* find vacant connection (i.e. one with no fd assigned to it) */
> -     for (j = 0; j < nslots; j++) {
> -             if (connection[j].fd == 0) {
> -                     c = &connection[j];
> +     for (i = 0; i < nslots; i++) {
> +             if (connection[i].fd == 0) {
> +                     c = &connection[i];
>                       break;
>               }
>       }
> -     if (j == nslots) {
> -             /* nothing available right now, return without accepting */
> -
> -             /* 
> -              * NOTE: This is currently still not the best option, but
> -              * at least we now have control over it and can reap a
> -              * connection from our pool instead of previously when
> -              * we were forking and were more or less on our own in
> -              * each process
> +     if (i == nslots) {
> +             /*
> +              * all our connection-slots are occupied and the only
> +              * way out is to drop another connection, because not
> +              * accepting this connection just kicks this can further
> +              * down the road (to the next queue_wait()) without
> +              * solving anything.
> +              *
> +              * This may sound bad, but this case can only be hit
> +              * either when there's a (D)DoS-attack or a massive
> +              * influx of requests. The latter is impossible to solve
> +              * at this moment without expanding resources, but the
> +              * former has certain characteristics allowing us to
> +              * handle this gracefully.
> +              *
> +              * During an attack (e.g. Slowloris, R-U-Dead-Yet, Slow
> +              * Read or just plain flooding) we can not see who is
> +              * waiting to be accept()ed.
> +              * However, an attacker usually already has many
> +              * connections open (while well-behaved clients could
> +              * do everything with just one connection using
> +              * keep-alive). Inferring a likely attacker-connection
> +              * is an educated guess based on which in-address is
> +              * occupying the most connection slots. Among those,
> +              * connections in early stages (receiving or sending
> +              * headers) are preferred over connections in late
> +              * stages (sending body).
> +              *
> +              * This quantitative approach effectively drops malicious
> +              * connections while preserving even long-running
> +              * benevolent connections like downloads.
>                */
> -             return NULL;
> +             c = get_connection_drop_candidate(connection, nslots);
> +             c->res.status = 0;
> +             logmsg(c);
> +             reset_connection(c);
>       }
>  
>       /* accept connection */
>       if ((c->fd = accept(insock, (struct sockaddr *)&c->ia,
>                           &(socklen_t){sizeof(c->ia)})) < 0) {
>               if (errno != EAGAIN && errno != EWOULDBLOCK) {
> -                     /* not much we can do here */
> +                     /*
> +                      * this should not happen, as we received the
> +                      * event that there are pending connections here
> +                      */
>                       warn("accept:");
>               }
>               return NULL;
> @@ -250,11 +334,11 @@ thread_method(void *data)
>                       if (queue_event_is_error(&event[i])) {
>                               if (c != NULL) {
>                                       queue_rem_fd(qfd, c->fd);
> -                                     close_connection(c);
> +                                     c->res.status = 0;
> +                                     logmsg(c);
> +                                     reset_connection(c);
>                               }
>  
> -                             printf("dropped a connection\n");
> -
>                               continue;
>                       }
>  
> @@ -301,7 +385,7 @@ thread_method(void *data)
>                                       if (queue_mod_fd(qfd, c->fd,
>                                                        QUEUE_EVENT_IN,
>                                                        c) < 0) {
> -                                             close_connection(c);
> +                                             reset_connection(c);
>                                               break;
>                                       }
>                                       break;
> @@ -310,7 +394,7 @@ thread_method(void *data)
>                                       if (queue_mod_fd(qfd, c->fd,
>                                                        QUEUE_EVENT_OUT,
>                                                        c) < 0) {
> -                                             close_connection(c);
> +                                             reset_connection(c);
>                                               break;
>                                       }
>                                       break;
> diff --git a/sock.c b/sock.c
> index fd16547..f160ecb 100644
> --- a/sock.c
> +++ b/sock.c
> @@ -222,3 +222,25 @@ sock_get_inaddr_str(const struct sockaddr_storage 
> *in_sa, char *str,
>  
>       return 0;
>  }
> +
> +int
> +sock_same_addr(const struct sockaddr_storage *sa1, const struct 
> sockaddr_storage *sa2)
> +{
> +     /* return early if address-families don't match */
> +     if (sa1->ss_family != sa2->ss_family) {
> +             return 0;
> +     }
> +
> +     switch (sa1->ss_family) {
> +     case AF_INET6:
> +             return memcmp(((struct sockaddr_in6 *)sa1)->sin6_addr.s6_addr,
> +                           ((struct sockaddr_in6 *)sa2)->sin6_addr.s6_addr,
> +                           sizeof(((struct sockaddr_in6 
> *)sa1)->sin6_addr.s6_addr));
> +     case AF_INET:
> +             return ntohl(((struct sockaddr_in *)sa1)->sin_addr.s_addr) ==
> +                    ntohl(((struct sockaddr_in *)sa2)->sin_addr.s_addr);
> +     default: /* AF_UNIX */
> +             return strcmp(((struct sockaddr_un *)sa1)->sun_path,
> +                           ((struct sockaddr_un *)sa2)->sun_path) == 0;
> +     }
> +}
> diff --git a/sock.h b/sock.h
> index 9834eda..c27141a 100644
> --- a/sock.h
> +++ b/sock.h
> @@ -12,5 +12,7 @@ int sock_get_uds_arr(const char *, uid_t, gid_t, int *, 
> size_t);
>  int sock_set_timeout(int, int);
>  int sock_set_nonblocking(int);
>  int sock_get_inaddr_str(const struct sockaddr_storage *, char *, size_t);
> +int sock_same_addr(const struct sockaddr_storage *,
> +                   const struct sockaddr_storage *);
>  
>  #endif /* SOCK_H */
> 

-- 
Kind regards,
Hiltjo

Reply via email to