Copyright © https://mongoose-os.com

Mongoose OS Forum

frame

IP access control is in the wrong place

In mongoose version 4, IP-address based access control was done right after the accept() syscall - connection from prohibited addresses was closed without reading and processing any data - this is the most safe thing to do.

In mongoose version 6, access control is done in mg_serve_http(), after reading some data (and maybe processing it). This is not safe. For connections from explicitly prohibited addresses, we cannot not know if they will send well-formed data, or even if they will send a data stream of finite length. If we did not want to block such things we would not need IP-based ACLs. (yes, I did hear about OS-level and network/hardware-level firewalls).

In addition, for our application, we need a custom access control handler - we have a custom database of permitted hostnames and IP addresses.

This code (from mongoose.c) works well for us:

static void accept_new_connection(...) {
  extern int check_midas_acl(const struct sockaddr *sa, int len);

  if ((so.sock = accept(...)) == INVALID_SOCKET) {
  } else if ((!check_acl(...) || (!check_midas_acl(&so.rsa.sa, len))) {
    closesocket(so.sock);
  }
}

I suspect similar effect can be achieved by using the MG_EV_ACCEPT event,
but it is not clear from the documentation how to force a connection closure
from the MG_EV_ACCEPT event handler, and for multi-threaded server,
the MG_EV_ACCEPT events are not delivered to the user anyway.

K.O.

Comments

  • rojerrojer Dublin, Ireland

    yes, as you correctly pointed out since ip acl is configured in http serving options, it's only checked when mg_serve_http is actually invoked.
    i think we could add ip_acl to mg_bind_opts.

    as for implementing custom ACLs, MG_EV_ACCEPT is the way to go. closing connection is the same as everywhere - set the CLOSE_IMMEDIATELY flag.
    the fact that accept event is not delivered in multithreaded mode is a bug that should be fixed (i believe we already have an issue tracking it).

  • Yes, thanks. I think I know how to kludge the MG_EV_ACCEPT delivery, shall try it with CLOSE_IMMEDIATELY. (My current kludge violates layering, I do not like that). K.O.

  • SergeySergey Dublin, Ireland

    @dd1 does it work as expected with MG_EV_ACCEPT ?

  • New mongoose is out, is MG_EV_ACCEPT delivered to the user now? K.O.

  • SergeySergey Dublin, Ireland

    @dd1 could you elaborate on your multithreaded usage, please?
    we are thinking about killing built-in multithreading support, and want to gather feedback on that.

Sign In or Register to comment.