From 18a8f69e9a990279215f6f5de8a05765a875b5f1 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Thu, 8 Apr 2021 14:19:37 +0200 Subject: [CLIENT] Use SO_GETPEERCRED instead of braindead setuid crap If you need daemon mode, run as root with --daemon, normal users can then request devices to be connected using the same binary WITHOUT havind the suid bit set on it. --- src/client/client.c | 125 +++++++++++++++++++++++++++++----------------------- 1 file changed, 69 insertions(+), 56 deletions(-) diff --git a/src/client/client.c b/src/client/client.c index 41e6c32..c458ac1 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -38,7 +38,7 @@ #include #include -#define SOCK_PATH "/var/run/dnbd3.socket" +#define SOCK_PATH "/run/dnbd3.socket" #define SOCK_BUFFER 1000 #define DEV_LEN 15 #define MAX_DEVS 50 @@ -68,7 +68,7 @@ static const struct option longOpts[] = { static int dnbd3_ioctl(const char *dev, const int command, dnbd3_ioctl_t * const msg); static void dnbd3_client_daemon(); -static void dnbd3_daemon_action(int client, int argc, char **argv); +static void dnbd3_daemon_action(int client, int uid, int argc, char **argv); static int dnbd3_daemon_ioctl(int uid, char *device, int action, const char *actionName, char *host); static char* dnbd3_daemon_open(int uid, char *host, char *image, int rid, int readAhead, const bool doLearnNewServers); static int dnbd3_daemon_send(int argc, char **argv); @@ -86,11 +86,11 @@ static char host_to_string(const dnbd3_host_t *host, char *target, size_t target if ( targetlen < 10 ) return false; if ( host->type == HOST_IP6 ) { *target++ = '['; - inet_ntop( AF_INET6, host->addr, target, targetlen - 10 ); + inet_ntop( AF_INET6, host->addr, target, (socklen_t)targetlen - 10 ); target += strlen( target ); *target++ = ']'; } else if ( host->type == HOST_IP4 ) { - inet_ntop( AF_INET, host->addr, target, targetlen - 8 ); + inet_ntop( AF_INET, host->addr, target, (socklen_t)targetlen - 8 ); target += strlen( target ); } else { snprintf( target, targetlen, "", (int)host->type ); @@ -137,8 +137,9 @@ static char parse_address(char *string, dnbd3_host_t *host) // Scan for port char *portpos = NULL, *ptr = string; while ( *ptr ) { - if ( *ptr == ':' ) - portpos = ptr; + if ( *ptr == ':' ) { + portpos = ptr; + } ++ptr; } if ( portpos == NULL ) return 0; // No port in string @@ -205,19 +206,20 @@ static int dnbd3_get_resolved_hosts(char *hosts_str, dnbd3_host_t *hosts, const do { /* get next host from string */ - while (*hosts_current_token == ' ') { + while ( *hosts_current_token == ' ' ) { hosts_current_token++; } /* buffer substring of host to get ip from it */ - hosts_last_host = strchr(hosts_current_token, ' '); + hosts_last_host = strchr( hosts_current_token, ' ' ); host_str_len = (hosts_last_host == NULL ? TMP_STR_LEN : (size_t)(hosts_last_host - hosts_current_token) + 1); - if ( host_str_len > TMP_STR_LEN ) + if ( host_str_len > TMP_STR_LEN ) { host_str_len = TMP_STR_LEN; + } - snprintf(host_str, host_str_len, "%s", hosts_current_token); + snprintf( host_str, host_str_len, "%s", hosts_current_token ); - if (!dnbd3_get_ip(host_str, &hosts[hosts_index])) + if ( !dnbd3_get_ip( host_str, &hosts[hosts_index] ) ) return false; hosts_index++; @@ -239,9 +241,7 @@ int main(int argc, char *argv[]) bool learnNewServers = true; int active_device_num = 0; - dnbd3_ioctl_t msg; - memset( &msg, 0, sizeof(dnbd3_ioctl_t) ); - msg.len = (uint16_t)sizeof(dnbd3_ioctl_t); + dnbd3_ioctl_t msg = { .len = (uint16_t)sizeof(msg) }; msg.hosts_num = 0; msg.read_ahead_kb = DEFAULT_READ_AHEAD_KB; msg.imgname = NULL; @@ -249,13 +249,22 @@ int main(int argc, char *argv[]) int opt = 0; int longIndex = 0; + // In case the client was invoked as a suid binary, change uid back to original user + // and warn the user as this was legacy mode + if ( geteuid() == 0 && getuid() != 0 ) { + fprintf( stderr, "Warning! %s is a setuid binary. This is deprecated and not needed anymore.\n", argv[0] ); + fprintf( stderr, "Switching back o user %d\n", (int)getuid() ); + setgid( getgid() ); + setuid( getuid() ); + } + opt = getopt_long( argc, argv, optString, longOpts, &longIndex ); while ( opt != -1 ) { switch ( opt ) { case 'h': - msg.hosts_num = dnbd3_get_resolved_hosts(optarg, msg.hosts, MAX_HOSTS_PER_IOCTL); - if (!msg.hosts_num) + msg.hosts_num = (uint8_t)dnbd3_get_resolved_hosts( optarg, msg.hosts, MAX_HOSTS_PER_IOCTL ); + if ( !msg.hosts_num ) exit( EXIT_FAILURE ); break; case 'i': @@ -330,13 +339,6 @@ int main(int argc, char *argv[]) // Direct requests - // In case the client was invoked as a suid binary, change uid back to original user - // when being used for direct ioctl, so that the device's permissions are taken into account - if ( geteuid() == 0 ) { - setgid( getgid() ); - setuid( getuid() ); - } - // close device if ( action == IOCTL_CLOSE && msg.hosts_num == 0 && dev && (msg.imgname == NULL )) { printf( "INFO: Closing device %s\n", dev ); @@ -377,14 +379,16 @@ int main(int argc, char *argv[]) static int dnbd3_ioctl(const char *dev, const int command, dnbd3_ioctl_t * const msg) { const int fd = open( dev, O_WRONLY ); - if ( fd < 0 ) { - printf( "open() for %s failed.\n", dev ); + if ( fd == -1 ) { + perror( "open() failed" ); return -ENODEV; } - if ( msg != NULL && msg->imgname != NULL ) msg->imgnamelen = (uint16_t)strlen( msg->imgname ); + if ( msg != NULL && msg->imgname != NULL ) { + msg->imgnamelen = (uint16_t)strlen( msg->imgname ); + } const int ret = ioctl( fd, command, msg ); if ( ret < 0 ) { - printf( "ioctl() failed.\n" ); + perror( "ioctl() failed" ); } close( fd ); return ret; @@ -398,11 +402,8 @@ static void dnbd3_client_daemon() struct timeval tv; int done, ret, len; socklen_t socklen; - - if ( geteuid() != 0 ) { - printf( "Only root can run the dnbd3-client in daemon mode!\n" ); - exit( 1 ); - } + struct ucred ucred; + int fdTest; if ( (listener = socket( AF_UNIX, SOCK_STREAM, 0 )) == -1 ) { perror( "socket" ); @@ -416,12 +417,21 @@ static void dnbd3_client_daemon() perror( "bind" ); exit( 1 ); } - chmod( addrLocal.sun_path, 0600 ); + fchmod( listener, 0666 ); + chmod( SOCK_PATH, 0666 ); if ( listen( listener, 5 ) == -1 ) { perror( "listen" ); + unlink( addrLocal.sun_path ); exit( 1 ); } + fdTest = open( "/dev/dnbd0", O_RDWR ); + if ( fdTest == -1 ) { + perror( "Opening /dev/dnbd0 failed. Daemon will probably not work" ); + } else { + close( fdTest ); + } + memset( openDevices, -1, sizeof(openDevices) ); for (;;) { @@ -432,6 +442,14 @@ static void dnbd3_client_daemon() continue; } + socklen = sizeof(ucred); + if ( getsockopt( client, SOL_SOCKET, SO_PEERCRED, &ucred, &socklen ) == -1 ) { + perror( "Could not get credentials of connection" ); + close( client ); + continue; + } + printf("Call from user %d\n", (int)ucred.uid ); + tv.tv_sec = 1; tv.tv_usec = 0; setsockopt( client, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv) ); @@ -458,24 +476,24 @@ static void dnbd3_client_daemon() } if ( pos >= end ) break; argv[argc++] = pos; - printf("Arg %d: '%s'\n", argc, pos); + //printf("Arg %d: '%s'\n", argc, pos); while ( *pos != '\0' ) { // This will always be in bounds because of -4 above if ( ++pos >= end ) break; } } - dnbd3_daemon_action( client, argc, argv ); + dnbd3_daemon_action( client, (int)ucred.uid, argc, argv ); } close( client ); } } -static void dnbd3_daemon_action(int client, int argc, char **argv) +static void dnbd3_daemon_action(int client, int uid, int argc, char **argv) { int opt = 0; int longIndex = 0; char *host = NULL, *image = NULL, *device = NULL; - int rid = 0, uid = 0, killMe = false, ahead = 512; + int rid = 0, killMe = false, ahead = 512; int len; int action = -1; const char *actionName = NULL; @@ -500,9 +518,6 @@ static void dnbd3_daemon_action(int client, int argc, char **argv) case 'r': rid = atoi( optarg ); break; - case 'U': - uid = atoi( optarg ); - break; case 'c': action = IOCTL_CLOSE; actionName = "Close"; @@ -529,14 +544,14 @@ static void dnbd3_daemon_action(int client, int argc, char **argv) } if ( killMe ) { - if ( uid != 0 ) { + if ( uid != geteuid() ) { printf( "Ignoring kill request by user %d\n", uid ); close( client ); return; } printf( "Received kill request; exiting.\n" ); - close( client ); unlink( SOCK_PATH ); + close( client ); exit( 0 ); } @@ -573,9 +588,7 @@ static int dnbd3_daemon_ioctl(int uid, char *device, int action, const char *act } else { index = atoi( device ); } - dnbd3_ioctl_t msg; - memset( &msg, 0, sizeof(msg) ); - msg.len = (uint16_t)sizeof(msg); + dnbd3_ioctl_t msg = { .len = (uint16_t)sizeof(msg) }; if ( host != NULL ) { dnbd3_get_ip( host, &msg.hosts[0] ); } @@ -608,16 +621,19 @@ static char* dnbd3_daemon_open(int uid, char *host, char *image, int rid, int re static char dev[DEV_LEN]; printf( "Opening a device for %s on %s\n", image, host ); // Check number of open devices - for (i = 0; i < MAX_DEVS; ++i) { - if ( openDevices[i] == uid ) sameUser++; - } - if ( sameUser > 1 ) { - printf( "Ignoring request by %d as there are already %d open devices for that user.\n", uid, sameUser ); - return NULL ; + if ( uid != 0 ) { + for ( i = 0; i < MAX_DEVS; ++i ) { + if ( openDevices[i] == uid ) sameUser++; + } + if ( sameUser > 1 ) { + printf( "Ignoring request by %d as there are already %d open devices for that user.\n", uid, sameUser ); + return NULL; + } } // Find free device - for (i = 0; i < MAX_DEVS; ++i) { - if ( openDevices[i] != -1 ) continue; + for ( i = 0; i < MAX_DEVS; ++i ) { + if ( openDevices[i] != -1 ) + continue; snprintf( dev, DEV_LEN, "/dev/dnbd%d", i ); if ( stat( dev, &st ) == -1 ) { break; @@ -648,7 +664,6 @@ static char* dnbd3_daemon_open(int uid, char *host, char *image, int rid, int re static int dnbd3_daemon_send(int argc, char **argv) { - const int uid = getuid(); int s, i, len; struct sockaddr_un remote; char buffer[SOCK_BUFFER]; @@ -668,7 +683,6 @@ static int dnbd3_daemon_send(int argc, char **argv) // (Re)build argument string into a single one, arguments separated by null chars char *pos = buffer; char *end = buffer + SOCK_BUFFER; - pos += snprintf( pos, end - pos, "--user%c%d", (int)'\0', uid ) + 1; for (i = 1; i < argc && pos < end; ++i) { pos += snprintf( pos, end - pos, "%s", argv[i] ) + 1; } @@ -725,7 +739,6 @@ static void dnbd3_print_help(char *argv_0) printf( " --daemon \t\t Run as helper daemon\n" ); printf( " --kill \t\t Kill running helper daemon\n\n" ); printf( "The helper daemon makes it possible for normal users to connect dnbd3 devices.\n" ); - printf( "The client binary needs to be a setuid program for this to work!\n" ); } void dnbd3_print_version() -- cgit v1.2.3-55-g7522