Fix coding style in server code

This commit is contained in:
Gergely Polonkai 2016-05-31 14:51:06 +02:00
parent 5c790813df
commit 227a9f7b47
4 changed files with 423 additions and 354 deletions

View File

@ -1,3 +1,2 @@
all: all:
gcc -g -Wall -o server server.c gcc -g -Wall -o server server.c

View File

@ -1,8 +1,22 @@
// TODO: Put these values into a config file // TODO: Put these values into a config file
#define PORT "2884" // Port number to listen on. It must be a string!
#define DROP_AFTER 10 // Drop clients who don't send any packets in this many seconds // Port number to listen on. It must be a string!
#define CURRENT_LOG_LEVEL LOG_LEVEL_DEBUG // Logging level. See LOG_LEVEL_* defines in server.h #define PORT "2884"
#define LOGFILE "auth.log" // Place of the log file
#define CLIENT_CONNECT_SCRIPT "/usr/local/sbin/ip_allow" // Script to run when a client connects // Drop clients who don't send any packets in this many seconds
#define CLIENT_DISCONNECT_SCRIPT "/usr/local/sbin/ip_block" // Script to run when a client disconnects #define DROP_AFTER 10
// Logging level. See LOG_LEVEL_* defines in server.h
#define CURRENT_LOG_LEVEL LOG_LEVEL_DEBUG
// Place of the log file
#define LOGFILE "auth.log"
// Script to run when a client connects
#define CLIENT_CONNECT_SCRIPT "/usr/local/sbin/ip_allow"
// Script to run when a client disconnects
#define CLIENT_DISCONNECT_SCRIPT "/usr/local/sbin/ip_block"
// Number of concurrent incoming (non-accepted) connections
#define BACKLOG 10 #define BACKLOG 10

View File

@ -29,14 +29,17 @@ fd_set master;
/* /*
* log_message(level, format, ...) * log_message(level, format, ...)
* Puts a message in the log file. If level is less than CURRENT_LOG_LEVEL (less means more importance), the message gets logged. *
* Puts a message in the log file. If level is less than
* CURRENT_LOG_LEVEL (less means more importance), the message gets
* logged.
*
* The log message should not end with a newline character. * The log message should not end with a newline character.
*/ */
static void static void
log_message(int level, const char *format, ...) log_message(int level, const char *format, ...)
{ {
if (CURRENT_LOG_LEVEL >= level) if (CURRENT_LOG_LEVEL >= level) {
{
va_list ap; va_list ap;
char *message; char *message;
char date[100]; char date[100];
@ -47,15 +50,16 @@ log_message(int level, const char *format, ...)
tmp = localtime(&t); tmp = localtime(&t);
/* This should never happen */ /* This should never happen */
if (tmp == NULL) if (tmp == NULL) {
{ // Cannot log the error here, as we caught error in the
// Cannot log the error here, as we caught error in the logger. Simply exit with an error status. // logger. Simply exit with an error status.
exit(1); exit(1);
} }
// Start handling the variable-length parameters // Start handling the variable-length parameters
va_start(ap, format); va_start(ap, format);
/* XXX: This is a bit unportable, maybe it should be changed to something else. */ /* TODO: This is a bit unportable, maybe it should be changed
* to something else. */
// Create the formatted message string // Create the formatted message string
vasprintf(&message, format, ap); vasprintf(&message, format, ap);
// End handling the variable-length parameters // End handling the variable-length parameters
@ -74,25 +78,31 @@ log_message(int level, const char *format, ...)
/* /*
* sigchld_handler(signal) * sigchld_handler(signal)
* This is the signal handler for the SIGCHLD signal. It gets called every time a child finishes its work (even with failure) *
* This is the signal handler for the SIGCHLD signal. It gets called
* every time a child finishes its work (even with failure)
*/ */
void void
sigchld_handler(int s) sigchld_handler(int s)
{ {
// Log a debug message about the finished child // Log a debug message about the finished child
log_message(LOG_LEVEL_DEBUG, "Found a hung child."); log_message(LOG_LEVEL_DEBUG, "Found a hung child.");
// Clean up all the dead children (otherwise they turn into zombie processes) // Clean up all the dead children (otherwise they turn into zombie
// processes)
while (waitpid(-1, NULL, WNOHANG) > 0); while (waitpid(-1, NULL, WNOHANG) > 0);
} }
/* /*
* sigterm_handler(signal) * sigterm_handler(signal)
* This is the signal handler for the SIGTERM signal. It gets called if someone kills the process with SIGTERM. *
* This is the signal handler for the SIGTERM signal. It gets called
* if someone kills the process with SIGTERM.
*/ */
void void
sigterm_handler(int s) sigterm_handler(int s)
{ {
// Log this event as an information (it's not a real error, and shouldn't be a debug-only message) // Log this event as an information (it's not a real error, and
// shouldn't be a debug-only message)
log_message(LOG_LEVEL_INFO, "Got SIGTERM, shutting down."); log_message(LOG_LEVEL_INFO, "Got SIGTERM, shutting down.");
// TODO: clean shutdown! (Close client sockets, etc.) // TODO: clean shutdown! (Close client sockets, etc.)
// Exit after the shutdown. // Exit after the shutdown.
@ -101,6 +111,7 @@ sigterm_handler(int s)
/* /*
* execute(command, parameter) * execute(command, parameter)
*
* Fork and execute the given program with exactly one parameter * Fork and execute the given program with exactly one parameter
*/ */
void void
@ -111,11 +122,14 @@ execute(char *command, char *parameter)
/* Do the fork() */ /* Do the fork() */
pid = fork(); pid = fork();
if (pid == 0) if (pid == 0) {
{
// If fork() returns zero, we are the child // If fork() returns zero, we are the child
// Try to execute the script. This will give control to the executed script. exec() returns only when an error occurs (e.g the command cannot be found). // Try to execute the script. This will give control to the
log_message(LOG_LEVEL_DEBUG, "Executing '%s \"%s\"'...", command, parameter); // executed script. exec() returns only when an error occurs
// (e.g the command cannot be found).
log_message(LOG_LEVEL_DEBUG,
"Executing '%s \"%s\"'...",
command, parameter);
execl(command, command, parameter, (char *)NULL); execl(command, command, parameter, (char *)NULL);
// If we get here, we got an error, which should be logged // If we get here, we got an error, which should be logged
log_message(LOG_LEVEL_ERROR, "execl: %s", strerror(errno)); log_message(LOG_LEVEL_ERROR, "execl: %s", strerror(errno));
@ -128,6 +142,7 @@ execute(char *command, char *parameter)
/* /*
* client_new(socket, remote_addr) * client_new(socket, remote_addr)
*
* Create a new client structure with the given data, and fully reset timer * Create a new client structure with the given data, and fully reset timer
*/ */
void void
@ -139,8 +154,7 @@ client_new(int socket, struct sockaddr_in *remote_addr)
// Allocate memory for the new client's data // Allocate memory for the new client's data
client_data = malloc(sizeof(client_t)); client_data = malloc(sizeof(client_t));
if (client_data == NULL) if (client_data == NULL) {
{
// Log an error message if allocation fails and exit with failure code // Log an error message if allocation fails and exit with failure code
log_message(LOG_LEVEL_ERROR, "malloc: %s", strerror(errno)); log_message(LOG_LEVEL_ERROR, "malloc: %s", strerror(errno));
exit(1); exit(1);
@ -148,20 +162,28 @@ client_new(int socket, struct sockaddr_in *remote_addr)
// Allocate memory for the new client's IP address // Allocate memory for the new client's IP address
tmp_addr = malloc(16); tmp_addr = malloc(16);
if (tmp_addr == NULL) if (tmp_addr == NULL) {
{ // Log an error message if allocation fails and exit with
// Log an error message if allocation fails and exit with failure code. Here we also free the previously allocated client_data // failure code. Here we also free the previously allocated
// client_data
log_message(LOG_LEVEL_ERROR, "malloc: %s", strerror(errno)); log_message(LOG_LEVEL_ERROR, "malloc: %s", strerror(errno));
free(client_data); free(client_data);
exit(1); exit(1);
} }
// Zero-fill the address location, so the string will be surely nul-terminated
// Zero-fill the address location, so the string will be surely
// nul-terminated
memset(tmp_addr, 0, 16); memset(tmp_addr, 0, 16);
// Get the numeric hostname (IP address) of the remote side // Get the numeric hostname (IP address) of the remote side
getnameinfo((const struct sockaddr *)remote_addr, sizeof(struct sockaddr_in), (char *)tmp_addr, 15, NULL, 0, NI_NUMERICHOST); getnameinfo((const struct sockaddr *)remote_addr,
sizeof(struct sockaddr_in),
(char *)tmp_addr,
15, NULL, 0, NI_NUMERICHOST);
// Log the connection // Log the connection
log_message(LOG_LEVEL_INFO, "New connection: %d (IP: %s)", socket, tmp_addr); log_message(LOG_LEVEL_INFO,
"New connection: %d (IP: %s)",
socket, tmp_addr);
// Fill the client_data struct // Fill the client_data struct
client_data->socket = socket; client_data->socket = socket;
@ -171,19 +193,19 @@ client_new(int socket, struct sockaddr_in *remote_addr)
client_data->next = NULL; client_data->next = NULL;
// If the client list is empty (this is the first client) // If the client list is empty (this is the first client)
if (!client_list) if (!client_list) {
{
// The client_list should point to the newly allocated struct // The client_list should point to the newly allocated struct
client_list = client_data; client_list = client_data;
} }
// Otherwise (this is not the first client // Otherwise (this is not the first client
else else {
{
// Set temp to the last element of the client list // Set temp to the last element of the client list
for (temp = client_list; temp->next; temp = temp->next); for (temp = client_list; temp->next; temp = temp->next);
// Set the last element's next pointer to point to the newly allocated struct // Set the last element's next pointer to point to the newly
// allocated struct
temp->next = client_data; temp->next = client_data;
// Set the newly allocated struct's previous pointer to the (till here) last client's struct // Set the newly allocated struct's previous pointer to the
// (till here) last client's struct
client_data->previous = temp; client_data->previous = temp;
} }
@ -193,6 +215,7 @@ client_new(int socket, struct sockaddr_in *remote_addr)
/* /*
* client_remove(socket) * client_remove(socket)
*
* Remove a client identified by its local socket number * Remove a client identified by its local socket number
*/ */
void void
@ -200,32 +223,44 @@ client_remove(int socket)
{ {
client_t *temp; client_t *temp;
// If we don't have an allocated client_list, we simply return, as we won't find the named client. However, this should never happen // If we don't have an allocated client_list, we simply return, as
if (!client_list) // we won't find the named client. However, this should never
// happen
if (!client_list) {
return; return;
}
// Walk through the client list // Walk through the client list
for (temp = client_list; temp; temp = temp->next) for (temp = client_list; temp; temp = temp->next) {
{
// If this element's socket number is the one we are looking for // If this element's socket number is the one we are looking for
if (temp->socket == socket) if (temp->socket == socket) {
{
// Logging a message about the disconnection // Logging a message about the disconnection
log_message(LOG_LEVEL_INFO, "Connection lost: %d (IP: %s)", temp->socket, temp->ip); log_message(LOG_LEVEL_INFO,
"Connection lost: %d (IP: %s)",
temp->socket, temp->ip);
// Remove this client from the linked list // Remove this client from the linked list
if (temp->previous) if (temp->previous) {
temp->previous->next = temp->next; temp->previous->next = temp->next;
if (temp->next) }
if (temp->next) {
temp->next->previous = temp->previous; temp->next->previous = temp->previous;
}
// If this is the first client, set the client_list to point to the second item (or to NULL if this is also the last client) // If this is the first client, set the client_list to
if (temp == client_list) // point to the second item (or to NULL if this is also
// the last client)
if (temp == client_list) {
client_list = temp->next; client_list = temp->next;
}
// If we don't have a previous, nor a next client in the list, then this was the last client, so client_list should be NULL // If we don't have a previous, nor a next client in the
// This is only a paranoia check, this should already happen in the previous instruction // list, then this was the last client, so client_list
if ((!temp->previous) && (!temp->next)) // should be NULL This is only a paranoia check, this
// should already happen in the previous instruction
if ((!temp->previous) && (!temp->next)) {
client_list = NULL; client_list = NULL;
}
// Execute the disconnect script // Execute the disconnect script
execute(CLIENT_DISCONNECT_SCRIPT, temp->ip); execute(CLIENT_DISCONNECT_SCRIPT, temp->ip);
@ -245,6 +280,7 @@ client_remove(int socket)
/* /*
* client_reset_timer(socket) * client_reset_timer(socket)
*
* Reset a client's timer, identified by the local socket number * Reset a client's timer, identified by the local socket number
*/ */
void void
@ -253,16 +289,20 @@ client_reset_timer(int socket)
client_t *temp; client_t *temp;
// Walk through the client list // Walk through the client list
for (temp = client_list; temp; temp = temp->next) for (temp = client_list; temp; temp = temp->next) {
// If the current client is the one we are looking for // If the current client is the one we are looking for
if (temp->socket == socket) if (temp->socket == socket) {
// Set its last reset time to the current timestamp // Set its last reset time to the current timestamp
temp->last_reset = time(NULL); temp->last_reset = time(NULL);
} }
}
}
/* /*
* check_timers() * check_timers()
* Check all clients if they have sent data in the near past, and disconnect (thus, deauthenticate) them if not *
* Check all clients if they have sent data in the near past, and
* disconnect (thus, deauthenticate) them if not
*/ */
void void
check_timers(void) check_timers(void)
@ -270,19 +310,22 @@ check_timers(void)
client_t *temp; client_t *temp;
// Walk through the client list // Walk through the client list
for (temp = client_list; temp; temp = temp->next) for (temp = client_list; temp; temp = temp->next) {
// If this client hasn't sent data in DROP_AFTER seconds // If this client hasn't sent data in DROP_AFTER seconds
if (time(NULL) - temp->last_reset > DROP_AFTER) if (time(NULL) - temp->last_reset > DROP_AFTER) {
{
// Log the timeout event // Log the timeout event
log_message(LOG_LEVEL_INFO, "Client timeout, dropping connection %d (IP: %s).", temp->socket, temp->ip); log_message(LOG_LEVEL_INFO,
"Client timeout, dropping connection %d (IP: %s).",
temp->socket, temp->ip);
// And remove the client from the client list // And remove the client from the client list
client_remove(temp->socket); client_remove(temp->socket);
} }
} }
}
/* /*
* main() * main()
*
* The main function of the server program * The main function of the server program
*/ */
int int
@ -305,9 +348,9 @@ main(void)
sa.sa_handler = sigchld_handler; sa.sa_handler = sigchld_handler;
sigemptyset(&sa.sa_mask); sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESTART; sa.sa_flags = SA_RESTART;
if (sigaction(SIGCHLD, &sa, NULL) < 0) if (sigaction(SIGCHLD, &sa, NULL) < 0) {
{
perror("sigaction"); perror("sigaction");
return 1; return 1;
} }
@ -315,9 +358,9 @@ main(void)
sa.sa_handler = sigterm_handler; sa.sa_handler = sigterm_handler;
sigemptyset(&sa.sa_mask); sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESTART; sa.sa_flags = SA_RESTART;
if (sigaction(SIGTERM, &sa, NULL) < 0) if (sigaction(SIGTERM, &sa, NULL) < 0) {
{
perror("sigaction"); perror("sigaction");
return 1; return 1;
} }
@ -328,50 +371,49 @@ main(void)
hints.ai_flags = AI_PASSIVE; // use my IP hints.ai_flags = AI_PASSIVE; // use my IP
// Check if our port number is already in use // Check if our port number is already in use
if ((rv = getaddrinfo (NULL, PORT, &hints, &servinfo)) != 0) if ((rv = getaddrinfo (NULL, PORT, &hints, &servinfo)) != 0) {
{
fprintf(stderr, "getaddrinfo: %s", gai_strerror(rv)); fprintf(stderr, "getaddrinfo: %s", gai_strerror(rv));
return 1; return 1;
} }
for (p = servinfo; p != NULL; p = p->ai_next) for (p = servinfo; p != NULL; p = p->ai_next) {
{
if ((sock_listen = socket (p->ai_family, p->ai_socktype, if ((sock_listen = socket (p->ai_family, p->ai_socktype,
p->ai_protocol)) == -1) p->ai_protocol)) == -1) {
{
perror("socket"); perror("socket");
continue; continue;
} }
if (setsockopt (sock_listen, SOL_SOCKET, SO_REUSEADDR, &yes, if (setsockopt (sock_listen, SOL_SOCKET, SO_REUSEADDR, &yes,
sizeof (int)) == -1) sizeof (int)) == -1) {
{
perror("setsockopt"); perror("setsockopt");
exit (1); exit (1);
} }
if (bind (sock_listen, p->ai_addr, p->ai_addrlen) == -1) if (bind (sock_listen, p->ai_addr, p->ai_addrlen) == -1) {
{
close(sock_listen); close(sock_listen);
perror("bind"); perror("bind");
continue; continue;
} }
break; break;
} }
if (p == NULL) if (p == NULL) {
{
perror("bind"); perror("bind");
return 2; return 2;
} }
freeaddrinfo(servinfo); freeaddrinfo(servinfo);
// Start listening on the listener socket // Start listening on the listener socket
if (listen(sock_listen, BACKLOG) == -1) if (listen(sock_listen, BACKLOG) == -1) {
{
perror("listen"); perror("listen");
exit (1); exit (1);
} }
@ -384,23 +426,22 @@ main(void)
fdmax = sock_listen; fdmax = sock_listen;
// Try to open (or create) the log file // Try to open (or create) the log file
if ((log_fd = fopen(LOGFILE, "a")) == NULL) if ((log_fd = fopen(LOGFILE, "a")) == NULL) {
{
perror("fopen"); perror("fopen");
exit(1); exit(1);
} }
// Try to go into the background // Try to go into the background
if (daemon(0, 0) < 0) if (daemon(0, 0) < 0) {
{
perror("daemon"); perror("daemon");
exit(1); exit(1);
} }
log_message(LOG_LEVEL_INFO, "Started."); log_message(LOG_LEVEL_INFO, "Started.");
while (1) while (1) {
{
struct timeval tv; struct timeval tv;
int t; int t;
@ -411,76 +452,90 @@ main(void)
tv.tv_sec = 1; tv.tv_sec = 1;
tv.tv_usec = 0; tv.tv_usec = 0;
// Wait for incoming connection or incoming data. "Modified" sockets (ones with incoming connections or incoming data) will be put in read_fds // Wait for incoming connection or incoming data. "Modified"
// sockets (ones with incoming connections or incoming data)
// will be put in read_fds
t = select(fdmax + 1, &read_fds, NULL, NULL, &tv); t = select(fdmax + 1, &read_fds, NULL, NULL, &tv);
// If select() returns a negative, it means an error // If select() returns a negative, it means an error
if (t < 0) if (t < 0) {
{ // However, if select() was only interrupted by a signal,
// However, if select() was only interrupted by a signal, simply continue // simply continue
if (errno == EINTR) if (errno == EINTR) {
continue; continue;
}
// Otherwise log an error and exit // Otherwise log an error and exit
log_message(LOG_LEVEL_ERROR, "select: %s", strerror(errno)); log_message(LOG_LEVEL_ERROR, "select: %s", strerror(errno));
return 1; return 1;
} }
// If select() returns a positive, it means that there are incoming connections or incoming data
else if (t != 0) // If select() returns a positive, it means that there are
{ // incoming connections or incoming data
else if (t != 0) {
int sock; int sock;
// Walk through the watched sockets (the lazy way, later this can cause some microseconds of hang up, as not all sockets are really monitored in this set [0..fdmax]) // Walk through the watched sockets (the lazy way, later
for (sock = 0; sock <= fdmax; sock++) // this can cause some microseconds of hang up, as not all
{ // sockets are really monitored in this set [0..fdmax])
// If the current socket exists in read_fds (it has data to read) for (sock = 0; sock <= fdmax; sock++) {
if (FD_ISSET(sock, &read_fds)) // If the current socket exists in read_fds (it has
{ // data to read)
if (FD_ISSET(sock, &read_fds)) {
// If the socket we found is the listener // If the socket we found is the listener
if (sock == sock_listen) if (sock == sock_listen) {
{
int new_socket; int new_socket;
struct sockaddr_in remote_addr; struct sockaddr_in remote_addr;
socklen_t addrlen = sizeof(struct sockaddr_in); socklen_t addrlen = sizeof(struct sockaddr_in);
// Accept the new connection // Accept the new connection
new_socket = accept(sock_listen, (struct sockaddr *)&remote_addr, &addrlen); new_socket = accept(sock_listen,
(struct sockaddr *)&remote_addr,
&addrlen);
// Add the new connection to the watched sockets // Add the new connection to the watched sockets
FD_SET(new_socket, &master); FD_SET(new_socket, &master);
if (new_socket > fdmax) if (new_socket > fdmax) {
fdmax = new_socket; fdmax = new_socket;
}
// Create a new client entry for the new connection // Create a new client entry for the new connection
client_new(new_socket, &remote_addr); client_new(new_socket, &remote_addr);
} } else {
else // Otherwise it's an already existing socket
{ // which has data to read
// Otherwise it's an already existing socket which has data to read
size_t read_len; size_t read_len;
char buf[128]; char buf[128];
// Read the data from the socket (in 128-bytes chunks) // Read the data from the socket (in 128-bytes chunks)
read_len = recv(sock, (char *)&buf, 128, 0); read_len = recv(sock, (char *)&buf, 128, 0);
// If recv() returns a negative value, this means an error, so we should remove this client // If recv() returns a negative value, this
if (read_len < 0) // means an error, so we should remove this
{ // client
if (read_len < 0) {
// In this case we also log an error // In this case we also log an error
log_message(LOG_LEVEL_ERROR, "recv: %s", strerror(errno)); log_message(LOG_LEVEL_ERROR,
"recv: %s",
strerror(errno));
client_remove(sock); client_remove(sock);
} }
// Otherwise if recv() returns 0, we just simply remove the client (0 means the client already closed the connection) // Otherwise if recv() returns 0, we just
else if (read_len == 0) // simply remove the client (0 means the
{ // client already closed the connection)
else if (read_len == 0) {
client_remove(sock); client_remove(sock);
} }
// If recv() returns a positive, the client sent some data, which will be discarded, but the timer of the client is reset
else // If recv() returns a positive, the client
{ // sent some data, which will be discarded,
// but the timer of the client is reset
else {
// Log a debugging message about the reset timer // Log a debugging message about the reset timer
log_message(LOG_LEVEL_DEBUG, "Connection timer reset: %d", sock); log_message(LOG_LEVEL_DEBUG,
"Connection timer reset: %d",
sock);
client_reset_timer(sock); client_reset_timer(sock);
} }
} }
@ -488,7 +543,9 @@ main(void)
} }
} }
// After we checked all the sockets, or there was no sockets to check (select() returned 0), we go and check if any clients timed out // After we checked all the sockets, or there was no sockets
// to check (select() returned 0), we go and check if any
// clients timed out
check_timers(); check_timers();
} }
@ -496,4 +553,3 @@ main(void)
return 0; return 0;
} }

View File

@ -6,7 +6,8 @@
#define LOG_LEVEL_INFO 1 #define LOG_LEVEL_INFO 1
#define LOG_LEVEL_DEBUG 2 #define LOG_LEVEL_DEBUG 2
/* The client_t struct. With this struct full client data can be stored in a doubly-linked list */ /* The client_t struct. With this struct full client data can be
* stored in a doubly-linked list */
typedef struct _client_t { typedef struct _client_t {
int socket; int socket;
char *ip; char *ip;
@ -16,4 +17,3 @@ typedef struct _client_t {
} client_t; } client_t;
#endif /* _AUTH_SERVER_H */ #endif /* _AUTH_SERVER_H */