On Sat, Feb 06, 2021 at 01:32:35AM +0100, git_AT_suckless.org wrote:
> commit 6d7c7b6ff701fafc2a649b21a66a92a9ab626221
> Author: Laslo Hunhold <dev_AT_frign.de>
> AuthorDate: Sat Feb 6 01:15:53 2021 +0100
> Commit: Laslo Hunhold <dev_AT_frign.de>
> 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 <dev_AT_frign.de>
>
> diff --git a/main.c b/main.c
> index f366985..4506c3e 100644
> --- a/main.c
> +++ b/main.c
> _AT_@ -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));
> }
> _AT_@ -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 *
> _AT_@ -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;
> _AT_@ -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;
> }
>
> _AT_@ -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;
> _AT_@ -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
> _AT_@ -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
> _AT_@ -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
Received on Sat Feb 06 2021 - 11:28:20 CET