=== modified file 'Makefile' --- Makefile 2014-03-10 06:55:54 +0000 +++ Makefile 2014-03-23 19:24:40 +0000 @@ -1,11 +1,15 @@ -WARN=-O -Wall -Wformat=2 -Winit-self -Wmissing-include-dirs \ - -Wswitch-default -Wswitch-enum -Wunused-parameter \ - -Wstrict-aliasing=1 -Wextra -Wfloat-equal -Wundef -Wshadow \ +WARN=-O -Wall -Wextra -Wdouble-promotion -Wformat=2 -Winit-self \ + -Wmissing-include-dirs -Wswitch-default -Wswitch-enum \ + -Wunused -Wuninitialized -Wstrict-overflow=5 \ + -Wsuggest-attribute=pure -Wsuggest-attribute=const \ + -Wsuggest-attribute=noreturn -Wfloat-equal -Wundef -Wshadow \ -Wunsafe-loop-optimizations -Wpointer-arith \ -Wbad-function-cast -Wcast-qual -Wcast-align -Wwrite-strings \ - -Wconversion -Wstrict-prototypes -Wold-style-definition \ - -Wpacked -Wnested-externs -Winline -Wvolatile-register-var \ - -Wunreachable-code + -Wconversion -Wlogical-op -Waggregate-return \ + -Wstrict-prototypes -Wold-style-definition \ + -Wmissing-format-attribute -Wnormalized=nfc -Wpacked \ + -Wredundant-decls -Wnested-externs -Winline -Wvla \ + -Wvolatile-register-var -Woverlength-strings #DEBUG=-ggdb3 # For info about _FORTIFY_SOURCE, see feature_test_macros(7) # and . === modified file 'plugin-runner.c' --- plugin-runner.c 2014-03-06 02:26:04 +0000 +++ plugin-runner.c 2014-03-23 19:24:40 +0000 @@ -105,6 +105,7 @@ /* Gets an existing plugin based on name, or if none is found, creates a new one */ +__attribute__((warn_unused_result)) static plugin *getplugin(char *name){ /* Check for existing plugin with that name */ for(plugin *p = plugin_list; p != NULL; p = p->next){ @@ -171,7 +172,7 @@ } /* Helper function for add_argument and add_environment */ -__attribute__((nonnull)) +__attribute__((nonnull, warn_unused_result)) static bool add_to_char_array(const char *new, char ***array, int *len){ /* Resize the pointed-to array to hold one more pointer */ @@ -202,7 +203,7 @@ } /* Add to a plugin's argument vector */ -__attribute__((nonnull(2))) +__attribute__((nonnull(2), warn_unused_result)) static bool add_argument(plugin *p, const char *arg){ if(p == NULL){ return false; @@ -211,7 +212,7 @@ } /* Add to a plugin's environment */ -__attribute__((nonnull(2))) +__attribute__((nonnull(2), warn_unused_result)) static bool add_environment(plugin *p, const char *def, bool replace){ if(p == NULL){ return false; @@ -244,6 +245,7 @@ * Descriptor Flags". | [[info:libc:Descriptor%20Flags][File Descriptor Flags]] | */ +__attribute__((warn_unused_result)) static int set_cloexec_flag(int fd){ int ret = (int)TEMP_FAILURE_RETRY(fcntl(fd, F_GETFD, 0)); /* If reading the flags failed, return error indication now. */ @@ -291,7 +293,7 @@ } /* Prints out a password to stdout */ -__attribute__((nonnull)) +__attribute__((nonnull, warn_unused_result)) static bool print_out_password(const char *buffer, size_t length){ ssize_t ret; for(size_t written = 0; written < length; written += (size_t)ret){ @@ -437,10 +439,13 @@ break; } } + errno = 0; } break; case 'G': /* --global-env */ - add_environment(getplugin(NULL), arg, true); + if(add_environment(getplugin(NULL), arg, true)){ + errno = 0; + } break; case 'o': /* --options-for */ { @@ -463,6 +468,7 @@ break; } } + errno = 0; } break; case 'E': /* --env-for */ @@ -480,7 +486,9 @@ errno = EINVAL; break; } - add_environment(getplugin(arg), envdef, true); + if(add_environment(getplugin(arg), envdef, true)){ + errno = 0; + } } break; case 'd': /* --disable */ @@ -488,6 +496,7 @@ plugin *p = getplugin(arg); if(p != NULL){ p->disabled = true; + errno = 0; } } break; @@ -496,12 +505,16 @@ plugin *p = getplugin(arg); if(p != NULL){ p->disabled = false; + errno = 0; } } break; case 128: /* --plugin-dir */ free(plugindir); plugindir = strdup(arg); + if(plugindir != NULL){ + errno = 0; + } break; case 129: /* --config-file */ /* This is already done by parse_opt_config_file() */ @@ -515,6 +528,7 @@ break; } uid = (uid_t)tmp_id; + errno = 0; break; case 131: /* --groupid */ tmp_id = strtoimax(arg, &tmp, 10); @@ -525,6 +539,7 @@ break; } gid = (gid_t)tmp_id; + errno = 0; break; case 132: /* --debug */ debug = true; @@ -578,6 +593,9 @@ case 129: /* --config-file */ free(argfile); argfile = strdup(arg); + if(argfile != NULL){ + errno = 0; + } break; case 130: /* --userid */ case 131: /* --groupid */ === modified file 'plugins.d/mandos-client.c' --- plugins.d/mandos-client.c 2014-03-10 10:11:37 +0000 +++ plugins.d/mandos-client.c 2014-03-23 19:24:40 +0000 @@ -183,7 +183,7 @@ perror(print_text); } -__attribute__((format (gnu_printf, 2, 3))) +__attribute__((format (gnu_printf, 2, 3), nonnull)) int fprintf_plus(FILE *stream, const char *format, ...){ va_list ap; va_start (ap, format); @@ -198,6 +198,7 @@ * bytes. "buffer_capacity" is how much is currently allocated, * "buffer_length" is how much is already used. */ +__attribute__((nonnull, warn_unused_result)) size_t incbuffer(char **buffer, size_t buffer_length, size_t buffer_capacity){ if(buffer_length + BUFFER_SIZE > buffer_capacity){ @@ -216,6 +217,7 @@ } /* Add server to set of servers to retry periodically */ +__attribute__((nonnull, warn_unused_result)) bool add_server(const char *ip, in_port_t port, AvahiIfIndex if_index, int af, server **current_server){ int ret; @@ -242,8 +244,8 @@ new_server->next = new_server; new_server->prev = new_server; *current_server = new_server; - /* Place the new server last in the list */ } else { + /* Place the new server last in the list */ new_server->next = *current_server; new_server->prev = (*current_server)->prev; new_server->prev->next = new_server; @@ -255,6 +257,7 @@ /* * Initialize GPGME. */ +__attribute__((nonnull, warn_unused_result)) static bool init_gpgme(const char *seckey, const char *pubkey, const char *tempdir, mandos_context *mc){ gpgme_error_t rc; @@ -350,6 +353,7 @@ * Decrypt OpenPGP data. * Returns -1 on error */ +__attribute__((nonnull, warn_unused_result)) static ssize_t pgp_packet_decrypt(const char *cryptotext, size_t crypto_size, char **plaintext, @@ -475,7 +479,8 @@ return plaintext_length; } -static const char * safer_gnutls_strerror(int value){ +__attribute__((warn_unused_result)) +static const char *safer_gnutls_strerror(int value){ const char *ret = gnutls_strerror(value); if(ret == NULL) ret = "(unknown)"; @@ -483,11 +488,13 @@ } /* GnuTLS log function callback */ +__attribute__((nonnull)) static void debuggnutls(__attribute__((unused)) int level, const char* string){ fprintf_plus(stderr, "GnuTLS: %s", string); } +__attribute__((nonnull, warn_unused_result)) static int init_gnutls_global(const char *pubkeyfilename, const char *seckeyfilename, mandos_context *mc){ @@ -567,6 +574,7 @@ return -1; } +__attribute__((nonnull, warn_unused_result)) static int init_gnutls_session(gnutls_session_t *session, mandos_context *mc){ int ret; @@ -629,6 +637,7 @@ __attribute__((unused)) const char *txt){} /* Called when a Mandos server is found */ +__attribute__((nonnull, warn_unused_result)) static int start_mandos_communication(const char *ip, in_port_t port, AvahiIfIndex if_index, int af, mandos_context *mc){ @@ -740,7 +749,7 @@ goto mandos_end; } if(af == AF_INET6){ - ((struct sockaddr_in6 *)&to)->sin6_port = htons(port); + ((struct sockaddr_in6 *)&to)->sin6_port = htons(port); if(IN6_IS_ADDR_LINKLOCAL (&((struct sockaddr_in6 *)&to)->sin6_addr)){ if(if_index == AVAHI_IF_UNSPEC){ @@ -1030,6 +1039,7 @@ return retval; } +__attribute__((nonnull)) static void resolve_callback(AvahiSServiceResolver *r, AvahiIfIndex interface, AvahiProtocol proto, @@ -1043,7 +1053,7 @@ AVAHI_GCC_UNUSED AvahiStringList *txt, AVAHI_GCC_UNUSED AvahiLookupResultFlags flags, - void* mc){ + void *mc){ if(r == NULL){ return; } @@ -1102,7 +1112,7 @@ const char *domain, AVAHI_GCC_UNUSED AvahiLookupResultFlags flags, - void* mc){ + void *mc){ if(b == NULL){ return; } @@ -1168,6 +1178,7 @@ errno = old_errno; } +__attribute__((nonnull, warn_unused_result)) bool get_flags(const char *ifname, struct ifreq *ifr){ int ret; error_t ret_errno; @@ -1192,6 +1203,7 @@ return true; } +__attribute__((nonnull, warn_unused_result)) bool good_flags(const char *ifname, const struct ifreq *ifr){ /* Reject the loopback device */ @@ -1239,6 +1251,7 @@ * corresponds to an acceptable network device. * (This function is passed to scandir(3) as a filter function.) */ +__attribute__((nonnull, warn_unused_result)) int good_interface(const struct dirent *if_entry){ if(if_entry->d_name[0] == '.'){ return 0; @@ -1262,6 +1275,7 @@ /* * This function determines if a network interface is up. */ +__attribute__((nonnull, warn_unused_result)) bool interface_is_up(const char *interface){ struct ifreq ifr; if(not get_flags(interface, &ifr)){ @@ -1278,6 +1292,7 @@ /* * This function determines if a network interface is running */ +__attribute__((nonnull, warn_unused_result)) bool interface_is_running(const char *interface){ struct ifreq ifr; if(not get_flags(interface, &ifr)){ @@ -1291,6 +1306,7 @@ return (bool)(ifr.ifr_flags & IFF_RUNNING); } +__attribute__((nonnull, pure, warn_unused_result)) int notdotentries(const struct dirent *direntry){ /* Skip "." and ".." */ if(direntry->d_name[0] == '.' @@ -1303,6 +1319,7 @@ } /* Is this directory entry a runnable program? */ +__attribute__((nonnull, warn_unused_result)) int runnable_hook(const struct dirent *direntry){ int ret; size_t sret; @@ -1363,6 +1380,7 @@ return 1; } +__attribute__((nonnull, warn_unused_result)) int avahi_loop_with_timeout(AvahiSimplePoll *s, int retry_interval, mandos_context *mc){ int ret; @@ -1437,6 +1455,7 @@ } /* Set effective uid to 0, return errno */ +__attribute__((warn_unused_result)) error_t raise_privileges(void){ error_t old_errno = errno; error_t ret_errno = 0; @@ -1449,6 +1468,7 @@ } /* Set effective and real user ID to 0. Return errno. */ +__attribute__((warn_unused_result)) error_t raise_privileges_permanently(void){ error_t old_errno = errno; error_t ret_errno = raise_privileges(); @@ -1465,6 +1485,7 @@ } /* Set effective user ID to unprivileged saved user ID */ +__attribute__((warn_unused_result)) error_t lower_privileges(void){ error_t old_errno = errno; error_t ret_errno = 0; @@ -1477,6 +1498,7 @@ } /* Lower privileges permanently */ +__attribute__((warn_unused_result)) error_t lower_privileges_permanently(void){ error_t old_errno = errno; error_t ret_errno = 0; @@ -1488,7 +1510,8 @@ return ret_errno; } -bool run_network_hooks(const char *mode, const char *interface, +__attribute__((nonnull)) +void run_network_hooks(const char *mode, const char *interface, const float delay){ struct dirent **direntries; int numhooks = scandir(hookdir, &direntries, runnable_hook, @@ -1522,22 +1545,39 @@ if(hook_pid == 0){ /* Child */ /* Raise privileges */ - raise_privileges_permanently(); + if(raise_privileges_permanently() != 0){ + perror_plus("Failed to raise privileges"); + _exit(EX_NOPERM); + } /* Set group */ errno = 0; ret = setgid(0); if(ret == -1){ perror_plus("setgid"); + _exit(EX_NOPERM); } /* Reset supplementary groups */ errno = 0; ret = setgroups(0, NULL); if(ret == -1){ perror_plus("setgroups"); - } - dup2(devnull, STDIN_FILENO); - close(devnull); - dup2(STDERR_FILENO, STDOUT_FILENO); + _exit(EX_NOPERM); + } + ret = dup2(devnull, STDIN_FILENO); + if(ret == -1){ + perror_plus("dup2(devnull, STDIN_FILENO)"); + _exit(EX_OSERR); + } + ret = close(devnull); + if(ret == -1){ + perror_plus("close"); + _exit(EX_OSERR); + } + ret = dup2(STDERR_FILENO, STDOUT_FILENO); + if(ret == -1){ + perror_plus("dup2(STDERR_FILENO, STDOUT_FILENO)"); + _exit(EX_OSERR); + } ret = setenv("MANDOSNETHOOKDIR", hookdir, 1); if(ret == -1){ perror_plus("setenv"); @@ -1559,7 +1599,7 @@ _exit(EX_OSERR); } char *delaystring; - ret = asprintf(&delaystring, "%f", delay); + ret = asprintf(&delaystring, "%f", (double)delay); if(ret == -1){ perror_plus("asprintf"); _exit(EX_OSERR); @@ -1618,15 +1658,13 @@ } close(devnull); } - return true; } +__attribute__((nonnull, warn_unused_result)) error_t bring_up_interface(const char *const interface, const float delay){ - int sd = -1; error_t old_errno = errno; - error_t ret_errno = 0; - int ret, ret_setflags; + int ret; struct ifreq network; unsigned int if_index = if_nametoindex(interface); if(if_index == 0){ @@ -1641,24 +1679,29 @@ } if(not interface_is_up(interface)){ - if(not get_flags(interface, &network) and debug){ + error_t ret_errno = 0, ioctl_errno = 0; + if(not get_flags(interface, &network)){ ret_errno = errno; fprintf_plus(stderr, "Failed to get flags for interface " "\"%s\"\n", interface); + errno = old_errno; return ret_errno; } - network.ifr_flags |= IFF_UP; + network.ifr_flags |= IFF_UP; /* set flag */ - sd = socket(PF_INET6, SOCK_DGRAM, IPPROTO_IP); - if(sd < 0){ + int sd = socket(PF_INET6, SOCK_DGRAM, IPPROTO_IP); + if(sd == -1){ ret_errno = errno; perror_plus("socket"); errno = old_errno; return ret_errno; } - + if(quit_now){ - close(sd); + ret = (int)TEMP_FAILURE_RETRY(close(sd)); + if(ret == -1){ + perror_plus("close"); + } errno = old_errno; return EINTR; } @@ -1668,21 +1711,27 @@ interface); } - /* Raise priviliges */ - raise_privileges(); - + /* Raise privileges */ + ret_errno = raise_privileges(); + bool restore_loglevel = false; + if(ret_errno != 0){ + perror_plus("Failed to raise privileges"); + } #ifdef __linux__ - /* Lower kernel loglevel to KERN_NOTICE to avoid KERN_INFO - messages about the network interface to mess up the prompt */ - int ret_linux = klogctl(8, NULL, 5); - bool restore_loglevel = true; - if(ret_linux == -1){ - restore_loglevel = false; - perror_plus("klogctl"); + int ret_linux; + if(ret_errno == 0){ + /* Lower kernel loglevel to KERN_NOTICE to avoid KERN_INFO + messages about the network interface to mess up the prompt */ + ret_linux = klogctl(8, NULL, 5); + if(ret_linux == -1){ + perror_plus("klogctl"); + } else { + restore_loglevel = true; + } } #endif /* __linux__ */ - ret_setflags = ioctl(sd, SIOCSIFFLAGS, &network); - ret_errno = errno; + int ret_setflags = ioctl(sd, SIOCSIFFLAGS, &network); + ioctl_errno = errno; #ifdef __linux__ if(restore_loglevel){ ret_linux = klogctl(7, NULL, 0); @@ -1692,8 +1741,15 @@ } #endif /* __linux__ */ - /* Lower privileges */ - lower_privileges(); + /* If raise_privileges() succeeded above */ + if(ret_errno == 0){ + /* Lower privileges */ + ret_errno = lower_privileges(); + if(ret_errno != 0){ + errno = ret_errno; + perror_plus("Failed to lower privileges"); + } + } /* Close the socket */ ret = (int)TEMP_FAILURE_RETRY(close(sd)); @@ -1702,10 +1758,10 @@ } if(ret_setflags == -1){ - errno = ret_errno; + errno = ioctl_errno; perror_plus("ioctl SIOCSIFFLAGS +IFF_UP"); errno = old_errno; - return ret_errno; + return ioctl_errno; } } else if(debug){ fprintf_plus(stderr, "Interface \"%s\" is already up; good\n", @@ -1729,6 +1785,7 @@ return 0; } +__attribute__((nonnull, warn_unused_result)) error_t take_down_interface(const char *const interface){ error_t old_errno = errno; struct ifreq network; @@ -1739,17 +1796,18 @@ return ENXIO; } if(interface_is_up(interface)){ - error_t ret_errno = 0; + error_t ret_errno = 0, ioctl_errno = 0; if(not get_flags(interface, &network) and debug){ ret_errno = errno; fprintf_plus(stderr, "Failed to get flags for interface " "\"%s\"\n", interface); + errno = old_errno; return ret_errno; } network.ifr_flags &= ~(short)IFF_UP; /* clear flag */ int sd = socket(PF_INET6, SOCK_DGRAM, IPPROTO_IP); - if(sd < 0){ + if(sd == -1){ ret_errno = errno; perror_plus("socket"); errno = old_errno; @@ -1761,14 +1819,23 @@ interface); } - /* Raise priviliges */ - raise_privileges(); - + /* Raise privileges */ + ret_errno = raise_privileges(); + if(ret_errno != 0){ + perror_plus("Failed to raise privileges"); + } int ret_setflags = ioctl(sd, SIOCSIFFLAGS, &network); - ret_errno = errno; + ioctl_errno = errno; - /* Lower privileges */ - lower_privileges(); + /* If raise_privileges() succeeded above */ + if(ret_errno == 0){ + /* Lower privileges */ + ret_errno = lower_privileges(); + if(ret_errno != 0){ + errno = ret_errno; + perror_plus("Failed to lower privileges"); + } + } /* Close the socket */ int ret = (int)TEMP_FAILURE_RETRY(close(sd)); @@ -1777,10 +1844,10 @@ } if(ret_setflags == -1){ - errno = ret_errno; + errno = ioctl_errno; perror_plus("ioctl SIOCSIFFLAGS -IFF_UP"); errno = old_errno; - return ret_errno; + return ioctl_errno; } } else if(debug){ fprintf_plus(stderr, "Interface \"%s\" is already down; odd\n", @@ -1794,7 +1861,7 @@ int main(int argc, char *argv[]){ mandos_context mc = { .server = NULL, .dh_bits = 1024, .priority = "SECURE256:!CTYPE-X.509:" - "+CTYPE-OPENPGP", .current_server = NULL, + "+CTYPE-OPENPGP", .current_server = NULL, .interfaces = NULL, .interfaces_size = 0 }; AvahiSServiceBrowser *sb = NULL; error_t ret_errno; @@ -1993,8 +2060,12 @@ /* Work around Debian bug #633582: */ - /* Re-raise priviliges */ - if(raise_privileges() == 0){ + /* Re-raise privileges */ + ret_errno = raise_privileges(); + if(ret_errno != 0){ + errno = ret_errno; + perror_plus("Failed to raise privileges"); + } else { struct stat st; if(strcmp(seckey, PATHDIR "/" SECKEY) == 0){ @@ -2040,7 +2111,11 @@ } /* Lower privileges */ - lower_privileges(); + ret_errno = lower_privileges(); + if(ret_errno != 0){ + errno = ret_errno; + perror_plus("Failed to lower privileges"); + } } } @@ -2072,10 +2147,8 @@ memcpy(interfaces_hooks, mc.interfaces, mc.interfaces_size); argz_stringify(interfaces_hooks, mc.interfaces_size, (int)','); } - if(not run_network_hooks("start", interfaces_hooks != NULL ? - interfaces_hooks : "", delay)){ - goto end; - } + run_network_hooks("start", interfaces_hooks != NULL ? + interfaces_hooks : "", delay); } if(not debug){ @@ -2209,17 +2282,15 @@ break; } bool interface_was_up = interface_is_up(interface); - ret = bring_up_interface(interface, delay); + errno = bring_up_interface(interface, delay); if(not interface_was_up){ - if(ret != 0){ - errno = ret; + if(errno != 0){ perror_plus("Failed to bring up interface"); } else { - ret_errno = argz_add(&interfaces_to_take_down, - &interfaces_to_take_down_size, - interface); - if(ret_errno != 0){ - errno = ret_errno; + errno = argz_add(&interfaces_to_take_down, + &interfaces_to_take_down_size, + interface); + if(errno != 0){ perror_plus("argz_add"); } } @@ -2449,33 +2520,39 @@ } } - /* Re-raise priviliges */ + /* Re-raise privileges */ { - raise_privileges(); - - /* Run network hooks */ - run_network_hooks("stop", interfaces_hooks != NULL ? - interfaces_hooks : "", delay); - - /* Take down the network interfaces which were brought up */ - { - char *interface = NULL; - while((interface=argz_next(interfaces_to_take_down, - interfaces_to_take_down_size, - interface))){ - ret_errno = take_down_interface(interface); - if(ret_errno != 0){ - errno = ret_errno; - perror_plus("Failed to take down interface"); - } - } - if(debug and (interfaces_to_take_down == NULL)){ - fprintf_plus(stderr, "No interfaces needed to be taken" - " down\n"); - } - } - - lower_privileges_permanently(); + ret_errno = raise_privileges(); + if(ret_errno != 0){ + perror_plus("Failed to raise privileges"); + } else { + + /* Run network hooks */ + run_network_hooks("stop", interfaces_hooks != NULL ? + interfaces_hooks : "", delay); + + /* Take down the network interfaces which were brought up */ + { + char *interface = NULL; + while((interface=argz_next(interfaces_to_take_down, + interfaces_to_take_down_size, + interface))){ + ret_errno = take_down_interface(interface); + if(ret_errno != 0){ + errno = ret_errno; + perror_plus("Failed to take down interface"); + } + } + if(debug and (interfaces_to_take_down == NULL)){ + fprintf_plus(stderr, "No interfaces needed to be taken" + " down\n"); + } + } + } + ret_errno = lower_privileges_permanently(); + if(ret_errno != 0){ + perror_plus("Failed to lower privileges permanently"); + } } free(interfaces_to_take_down);