diff -u nbd-2.9.25/nbd-server.c nbd-2.9.25/nbd-server.c --- nbd-2.9.25/nbd-server.c +++ nbd-2.9.25/nbd-server.c @@ -176,7 +176,7 @@ #define NEG_OLD (1 << 1) #define NEG_MODERN (1 << 2) -int modernsock=0; /**< Socket for the modern handler. Not used +int modernsock=-1; /**< Socket for the modern handler. Not used if a client was only specified on the command line; only port used if oldstyle is set to false (and then the @@ -187,6 +187,16 @@ bool logged_oversized=false; /**< whether we logged oversized requests already */ +static volatile sig_atomic_t is_sigchld_caught; /**< Flag set by + SIGCHLD handler + to mark a child + exit */ + +static volatile sig_atomic_t is_sigterm_caught; /**< Flag set by + SIGTERM handler + to mark a exit + request */ + /** * Types of virtuatlization **/ @@ -344,7 +354,7 @@ return 1; } } - if (strncmp(line,opts->clientname,strlen(opts->clientname))==0) { + if (strcmp(line,opts->clientname)==0) { fclose(f); return 1; } @@ -1030,27 +1040,16 @@ } /** - * Signal handler for SIGCHLD + * Handle SIGCHLD by setting atomically a flag which will be evaluated in the + * main loop of the root server process. This allows us to separate the signal + * catching from th actual task triggered by SIGCHLD and hence processing in the + * interrupt context is kept as minimial as possible. + * * @param s the signal we're handling (must be SIGCHLD, or something * is severely wrong) **/ -void sigchld_handler(int s) { - int status; - int* i; - pid_t pid; - - while((pid=waitpid(-1, &status, WNOHANG)) > 0) { - if(WIFEXITED(status)) { - msg3(LOG_INFO, "Child exited with %d", WEXITSTATUS(status)); - } - i=g_hash_table_lookup(children, &pid); - if(!i) { - msg3(LOG_INFO, "SIGCHLD received for an unknown child with PID %ld", (long)pid); - } else { - DEBUG("Removing %d from the list of children", pid); - g_hash_table_remove(children, &pid); - } - } +static void sigchld_handler(const int s G_GNUC_UNUSED) { + is_sigchld_caught = 1; } /** @@ -1063,27 +1062,21 @@ **/ void killchild(gpointer key, gpointer value, gpointer user_data) { pid_t *pid=value; - int *parent=user_data; kill(*pid, SIGTERM); - *parent=1; } /** - * Handle SIGTERM and dispatch it to our children + * Handle SIGTERM by setting atomically a flag which will be evaluated in the + * main loop of the root server process. This allows us to separate the signal + * catching from th actual task triggered by SIGTERM and hence processing in the + * interrupt context is kept as minimial as possible. + * * @param s the signal we're handling (must be SIGTERM, or something * is severely wrong). **/ -void sigterm_handler(int s) { - int parent=0; - - g_hash_table_foreach(children, killchild, &parent); - - if(parent) { - unlink(pidfname); - } - - exit(EXIT_SUCCESS); +static void sigterm_handler(const int s G_GNUC_UNUSED) { + is_sigterm_caught = 1; } /** @@ -2063,6 +2056,248 @@ g_free(data); } +static pid_t +spawn_child() +{ + pid_t pid; + sigset_t newset; + sigset_t oldset; + + sigemptyset(&newset); + sigaddset(&newset, SIGCHLD); + sigaddset(&newset, SIGTERM); + sigprocmask(SIG_BLOCK, &newset, &oldset); + pid = fork(); + if (pid < 0) { + msg3(LOG_ERR, "Could not fork (%s)", strerror(errno)); + goto out; + } + if (pid > 0) { /* Parent */ + pid_t *pidp; + + pidp = g_malloc(sizeof(pid_t)); + *pidp = pid; + g_hash_table_insert(children, pidp, pidp); + goto out; + } + /* Child */ + + /* Child's signal disposition is reset to default. */ + signal(SIGCHLD, SIG_DFL); + signal(SIGTERM, SIG_DFL); + signal(SIGHUP, SIG_DFL); + sigemptyset(&oldset); +out: + sigprocmask(SIG_SETMASK, &oldset, NULL); + return pid; +} + +static int +socket_accept(const int sock) +{ + struct sockaddr_storage addrin; + socklen_t addrinlen = sizeof(addrin); + int net; + + net = accept(sock, (struct sockaddr *) &addrin, &addrinlen); + if (net < 0) { + err_nonfatal("Failed to accept socket connection: %m"); + } + + return net; +} + +static void +handle_modern_connection(GArray *const servers, const int sock) +{ + int net; + pid_t pid; + CLIENT *client = NULL; + int sock_flags_old; + int sock_flags_new; + + net = socket_accept(sock); + if (net < 0) + return; + + if (!dontfork) { + pid = spawn_child(); + if (pid) { + if (pid > 0) + msg2(LOG_INFO, "Spawned a child process"); + if (pid < 0) + msg2(LOG_ERR, "Failed to spawn a child process"); + close(net); + return; + } + /* Child just continues. */ + } + + client = negotiate(net, NULL, servers, NEG_INIT | NEG_MODERN); + if (!client) { + msg2(LOG_ERR, "Modern initial negotiation failed"); + goto handler_err; + } + + if (client->server->max_connections > 0 && + g_hash_table_size(children) >= client->server->max_connections) { + msg3(LOG_ERR, "Max connections (%d) reached", + client->server->max_connections); + goto handler_err; + } + + sock_flags_old = fcntl(net, F_GETFL, 0); + if (sock_flags_old == -1) { + msg2(LOG_ERR, "Failed to get socket flags"); + goto handler_err; + } + + sock_flags_new = sock_flags_old & ~O_NONBLOCK; + if (sock_flags_new != sock_flags_old && + fcntl(net, F_SETFL, sock_flags_new) == -1) { + msg2(LOG_ERR, "Failed to set socket to blocking mode"); + goto handler_err; + } + + set_peername(net, client); + + if (!authorized_client(client)) { + msg3(LOG_INFO, "Client '%s' is not authorized to access", + client->clientname); + goto handler_err; + } + + if (!dontfork) { + int i; + + /* Free all root server resources here, because we are + * currently in the child process serving one specific + * connection. These are not simply needed anymore. */ + g_hash_table_destroy(children); + children = NULL; + close(modernsock); + + /* Now that we are in the child process after a + * succesful negotiation, we do not need the list of + * servers anymore, get rid of it.*/ + + for (i = 0; i < servers->len; i++) { + const SERVER *const server = &g_array_index(servers, SERVER, i); + close(server->socket); + } + + /* FALSE does not free the + actual data. This is required, + because the client has a + direct reference into that + data, and otherwise we get a + segfault... */ + g_array_free(servers, FALSE); + } + + msg2(LOG_INFO, "Starting to serve"); + serveconnection(client); + exit(EXIT_SUCCESS); + +handler_err: + g_free(client); + close(net); + + if (!dontfork) { + exit(EXIT_FAILURE); + } +} + +static void +handle_connection(GArray *servers, int net, SERVER *serve, CLIENT *client) +{ + int sock_flags_old; + int sock_flags_new; + + if(serve->max_connections > 0 && + g_hash_table_size(children) >= serve->max_connections) { + msg2(LOG_INFO, "Max connections reached"); + goto handle_connection_out; + } + if((sock_flags_old = fcntl(net, F_GETFL, 0)) == -1) { + err("fcntl F_GETFL"); + } + sock_flags_new = sock_flags_old & ~O_NONBLOCK; + if (sock_flags_new != sock_flags_old && + fcntl(net, F_SETFL, sock_flags_new) == -1) { + err("fcntl F_SETFL ~O_NONBLOCK"); + } + if(!client) { + client = g_new0(CLIENT, 1); + client->server=serve; + client->exportsize=OFFT_MAX; + client->net=net; + client->transactionlogfd = -1; + } + set_peername(net, client); + if (!authorized_client(client)) { + msg2(LOG_INFO,"Unauthorized client") ; + goto handle_connection_out; + } + msg2(LOG_INFO,"Authorized client") ; + + if (!dontfork) { + pid_t pid; + int i; + sigset_t newset; + sigset_t oldset; + + sigemptyset(&newset); + sigaddset(&newset, SIGCHLD); + sigaddset(&newset, SIGTERM); + sigprocmask(SIG_BLOCK, &newset, &oldset); + if ((pid = fork()) < 0) { + msg3(LOG_INFO,"Could not fork (%s)",strerror(errno)) ; + sigprocmask(SIG_SETMASK, &oldset, NULL); + goto handle_connection_out; + } + if (pid > 0) { /* parent */ + pid_t *pidp; + + pidp = g_malloc(sizeof(pid_t)); + *pidp = pid; + g_hash_table_insert(children, pidp, pidp); + sigprocmask(SIG_SETMASK, &oldset, NULL); + goto handle_connection_out; + } + /* child */ + + /* Child's signal disposition is reset to default. */ + signal(SIGCHLD, SIG_DFL); + signal(SIGTERM, SIG_DFL); + sigemptyset(&oldset); + sigprocmask(SIG_SETMASK, &oldset, NULL); + + g_hash_table_destroy(children); + children = NULL; + for(i=0;ilen;i++) { + serve=&g_array_index(servers, SERVER, i); + close(serve->socket); + } + /* FALSE does not free the + actual data. This is required, + because the client has a + direct reference into that + data, and otherwise we get a + segfault... */ + g_array_free(servers, FALSE); + close(modernsock); + } + + msg2(LOG_INFO,"Starting to serve"); + serveconnection(client); + exit(EXIT_SUCCESS); + +handle_connection_out: + g_free(client); + close(net); +} + /** * Loop through the available servers, and serve them. Never returns. **/ @@ -2074,6 +2309,8 @@ int sock; fd_set mset; fd_set rset; + sigset_t blocking_mask; + sigset_t original_mask; /* * Set up the master fd_set. The set of descriptors we need @@ -2090,99 +2327,78 @@ max=sock>max?sock:max; } } - if(modernsock) { + if(modernsock >= 0) { FD_SET(modernsock, &mset); max=modernsock>max?modernsock:max; } + + /* Construct a signal mask which is used to make signal testing and + * receiving an atomic operation to ensure no signal is received between + * tests and blocking pselect(). */ + if (sigemptyset(&blocking_mask) == -1) + err("failed to initialize blocking_mask: %m"); + + if (sigaddset(&blocking_mask, SIGCHLD) == -1) + err("failed to add SIGCHLD to blocking_mask: %m"); + + if (sigaddset(&blocking_mask, SIGHUP) == -1) + err("failed to add SIGHUP to blocking_mask: %m"); + + if (sigaddset(&blocking_mask, SIGTERM) == -1) + err("failed to add SIGTERM to blocking_mask: %m"); + + if (sigprocmask(SIG_BLOCK, &blocking_mask, &original_mask) == -1) + err("failed to block signals: %m"); + for(;;) { - CLIENT *client = NULL; - pid_t *pid; + if (is_sigterm_caught) { + is_sigterm_caught = 0; - memcpy(&rset, &mset, sizeof(fd_set)); - if(select(max+1, &rset, NULL, NULL, NULL)>0) { - int net = 0; - SERVER* serve=NULL; + g_hash_table_foreach(children, killchild, NULL); + unlink(pidfname); - DEBUG("accept, "); - if(FD_ISSET(modernsock, &rset)) { - if((net=accept(modernsock, (struct sockaddr *) &addrin, &addrinlen)) < 0) - err("accept: %m"); - client = negotiate(net, NULL, servers, NEG_INIT | NEG_MODERN); - if(!client) { - err_nonfatal("negotiation failed"); - close(net); - net=0; - continue; + exit(EXIT_SUCCESS); + } + + if (is_sigchld_caught) { + int status; + int* i; + pid_t pid; + + is_sigchld_caught = 0; + + while ((pid=waitpid(-1, &status, WNOHANG)) > 0) { + if (WIFEXITED(status)) { + msg3(LOG_INFO, "Child exited with %d", WEXITSTATUS(status)); } - serve = client->server; - } - for(i=0;ilen && !net;i++) { - serve=&(g_array_index(servers, SERVER, i)); - if(FD_ISSET(serve->socket, &rset)) { - if ((net=accept(serve->socket, (struct sockaddr *) &addrin, &addrinlen)) < 0) - err("accept: %m"); + i = g_hash_table_lookup(children, &pid); + if (!i) { + msg3(LOG_INFO, "SIGCHLD received for an unknown child with PID %ld", (long)pid); + } else { + DEBUG("Removing %d from the list of children", pid); + g_hash_table_remove(children, &pid); } } - if(net) { - int sock_flags; + } - if(serve->max_connections > 0 && - g_hash_table_size(children) >= serve->max_connections) { - msg2(LOG_INFO, "Max connections reached"); - close(net); - continue; - } - if((sock_flags = fcntl(net, F_GETFL, 0))==-1) { - err("fcntl F_GETFL"); - } - if(fcntl(net, F_SETFL, sock_flags &~O_NONBLOCK)==-1) { - err("fcntl F_SETFL ~O_NONBLOCK"); - } - if(!client) { - client = g_new0(CLIENT, 1); - client->server=serve; - client->exportsize=OFFT_MAX; - client->net=net; - client->transactionlogfd = -1; - } - set_peername(net, client); - if (!authorized_client(client)) { - msg2(LOG_INFO,"Unauthorized client") ; - close(net); - continue; - } - msg2(LOG_INFO,"Authorized client") ; - pid=g_malloc(sizeof(pid_t)); + memcpy(&rset, &mset, sizeof(fd_set)); + if (pselect(max + 1, &rset, NULL, NULL, NULL, &original_mask) > 0) { + DEBUG("accept, "); + if(modernsock >= 0 && FD_ISSET(modernsock, &rset)) { + handle_modern_connection(servers, modernsock); + } + for(i=0; i < servers->len; i++) { + int net; + SERVER *serve; - if (!dontfork) { - if ((*pid=fork())<0) { - msg3(LOG_INFO,"Could not fork (%s)",strerror(errno)) ; - close(net); - continue; - } - if (*pid>0) { /* parent */ - close(net); - g_hash_table_insert(children, pid, pid); + serve=&(g_array_index(servers, SERVER, i)); + if(FD_ISSET(serve->socket, &rset)) { + if ((net=accept(serve->socket, (struct sockaddr *) &addrin, &addrinlen)) < 0) { + err_nonfatal("accept: %m"); continue; } - /* child */ - g_hash_table_destroy(children); - for(i=0;ilen;i++) { - serve=&g_array_index(servers, SERVER, i); - close(serve->socket); - } - /* FALSE does not free the - actual data. This is required, - because the client has a - direct reference into that - data, and otherwise we get a - segfault... */ - g_array_free(servers, FALSE); + handle_connection(servers, net, serve, NULL); } - - msg2(LOG_INFO,"Starting to serve"); - serveconnection(client); - exit(EXIT_SUCCESS); } } } @@ -2337,11 +2553,14 @@ sa.sa_handler = sigchld_handler; sigemptyset(&sa.sa_mask); + sigaddset(&sa.sa_mask, SIGTERM); sa.sa_flags = SA_RESTART; if(sigaction(SIGCHLD, &sa, NULL) == -1) err("sigaction: %m"); + sa.sa_handler = sigterm_handler; sigemptyset(&sa.sa_mask); + sigaddset(&sa.sa_mask, SIGCHLD); sa.sa_flags = SA_RESTART; if(sigaction(SIGTERM, &sa, NULL) == -1) err("sigaction: %m"); @@ -2499,7 +2718,7 @@ #endif client=g_malloc(sizeof(CLIENT)); client->server=serve; - client->net=0; + client->net=-1; client->exportsize=OFFT_MAX; set_peername(0,client); serveconnection(client); diff -u nbd-2.9.25/debian/changelog nbd-2.9.25/debian/changelog --- nbd-2.9.25/debian/changelog +++ nbd-2.9.25/debian/changelog @@ -1,3 +1,32 @@ +nbd (1:2.9.25-2ubuntu1.1) precise-security; urgency=medium + + * SECURITY UPDATE: access restriction bypass via IP partial match + - nbd-server.c: use strcmp instead of strncmp to compare clients. + - df890c99337a255979e608d71f42401c0cddd5e0 + - CVE-2013-6410 + * SECURITY UPDATE: denial of service in modern style negotiation + - nbd-server.c: backport commits to refactor code and handle + modern-style negotiation in a child process + - 7fdf3f6531e3f1f61f11bbbc185cc4cf12f86ff9 + - f958e6563bd9e365c8adf6d2cc2aa023ae132681 + - 59c25aa8e743ad0b1ab9aec8837d15181670e057 + - 2a62394c64734f32d4d8205c80ac935f59f3f873 + - abe1977070e2c71d82f473c6d3aa807b489c7fb0 + - 43fa145cc7f0b50cd74c318c27bc00415c6a8499 + - d072873d5756ba52c4cac4d13857e2acab98539f + - 0b019ad559f6a664fa6f15c429c0bf6ea99ed564 + - e68de3612ac5b58bf64134216e22b131995e62a7 + - 463c8bcf4e638bceb75e33ae3a419f93f1e52a68 + - 22b693e410b0c314f879f437d654c76aeadb97e5 + - 741495cb08503fd32a9d22648e63b64390c601f4 + - CVE-2013-7441 + * SECURITY UPDATE: denial of service via incorrect signal handling + - nbd-server.c: fix unsafe signal handling + - 412defe42d03be842c80d21dccf405c435b18432 + - CVE-2015-0847 + + -- Marc Deslauriers Tue, 14 Jul 2015 07:44:02 -0400 + nbd (1:2.9.25-2ubuntu1) precise; urgency=low * Merge from Debian unstable. Remaining changes (LP: #594595):