On 23 Apr 2008, at 16:33, Perki Pat wrote: > Perki Pat escribió: >> Alvaro Lopez Ortega escribió: >>> On 23 Apr 2008, at 01:59, Perki Pat wrote: >>> The patch is good, congrats! However, there is a couple of changes >>> that would improve the patch before we apply it: >>> >>> - I would move the checking code to the initialize_server_socket() >>> function in cherokee/server.c file. In that way the port would >>> always be checked, not only when the -p parameter is used; for >>> instance, it would ensure that a port read from a configuration >>> file is correct as well. >> initialize_server_socket() receives an unsigned short argument for >> the port: it's always in range (gcc told me). >> It's called from a couple places (server.c:940 and server.c:1070), >> and in both places it takes its value from a cherokee_server_t >> struct, which has a couple field (port and port_tls) which are also >> unsigned short. >> I'm looking where that fields are assigned. I've found where >> they're filled from config file. Now I'll look for more places. > > ok, I think this choice is done. I'll paste the diffs below. > > We can check that if I'm root the port or port_tls is greater than > 1024. > (Windows also?) Or maybe it's better to "outsource" it to bind() > +perror()??
No, that is not a good idea. We have been discussing something similar these days, actually. The idea is that we must let the operating system to do its jobs instead of trying to do it twice in the server side. As you said, at the first look checking that the port is higher than 1024 when it isn't run as root may look like a good idea. However, there are systems that will allow the server to bind to a low port even if it is not run as root. For instance, Windows or Solaris with RBAC would do it. So, the underneath idea here is that we must not assume anything; the best approach is to let the operating system make the decision on what is allowed and what is not. In this case, whether to open a low port is permitted. > I've changed severals cuint_t port; with cushort_t port; > > I've seen several atoi()s with no boundaries checking > (server.c:configure_server_property(), lines circa 1480). is right? It depends on each case. It might be something that we should let the operating system to validate. > Also, when I do cherokee -p 67, if the config file says server! > port=81, > cherokee listens in port 81. Maybe command line arguments should > "overwrite" config file statements. No, that is the expected behavior actually. The -p parameter is only meant to be used in combination with -r, otherwise it should not affect the 'server!port' configuration entry. > ps: cc to list? Yeah, please sent everything to the list. Public revision is one of the most interesting advantages of F/OSS. -- Greetings, alo. _______________________________________________ Cherokee mailing list [email protected] http://cherokee-project.com/cgi-bin/mailman/listinfo/cherokee
