From d59faf22ed172a22cf701a283f4f58bd6d16892d Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 2 Apr 2013 10:55:26 +0200 Subject: [PATCH] Middleware list check fixed. [Issue#] N/A [Feature/Bug] N/A [Problem] Empty line in mw-lists matches everything. [Cause] N/A [Solution] Commandline replaced by executable name. Fixed comparison of executable name with mw-list entries. Empty line removed from mw-list. 50 chars limit removed [Verification] Run all security server tests Change-Id: I872ad45a4089b484a30fc4caa1759ce9d6a584e4 --- src/communication/security-server-comm.c | 173 ++++++++++++++++--------------- src/include/security-server-comm.h | 2 +- src/include/security-server-common.h | 3 +- src/server/security-server-cookie.c | 49 +++++---- 4 files changed, 114 insertions(+), 113 deletions(-) diff --git a/src/communication/security-server-comm.c b/src/communication/security-server-comm.c index eac1c55..3ea4780 100644 --- a/src/communication/security-server-comm.c +++ b/src/communication/security-server-comm.c @@ -32,6 +32,8 @@ #include #include #include +#include +#include #include "security-server-common.h" #include "security-server-comm.h" @@ -51,58 +53,49 @@ void printhex(const unsigned char *data, int size) printf("\n"); } -char *read_cmdline_from_proc(pid_t pid) +char *read_exe_path_from_proc(pid_t pid) { - int memsize = 32; - char path[32]; - char *cmdline = NULL, *tempptr = NULL; - FILE *fp = NULL; + char link[32]; + char *exe = NULL; + size_t size = 64; + ssize_t cnt = 0; - snprintf(path, sizeof(path), "/proc/%d/cmdline", pid); + // get link to executable + snprintf(link, sizeof(link), "/proc/%d/exe", pid); - fp = fopen(path, "r"); - if(fp == NULL) + for (;;) { - SEC_SVR_DBG("Cannot open cmdline on pid[%d]", pid); - return NULL; - } + exe = malloc(size); + if (exe == NULL) { + SEC_SVR_DBG("Out of memory"); + return NULL; + } - cmdline = malloc(32); - if(cmdline == NULL) - { - SEC_SVR_DBG("%s", "Out of memory"); - fclose(fp); - return NULL; - } + // read link target + cnt = readlink(link, exe, size); - bzero(cmdline, memsize); - if(fgets(cmdline, 32, fp) == NULL) - { - SEC_SVR_DBG("%s", "Cannot read cmdline"); - free(cmdline); - fclose(fp); - return NULL; - } + // error + if (cnt < 0 || (size_t)cnt > size) { + SEC_SVR_DBG("Can't locate process binary for pid[%d]", pid); + free(exe); + return NULL; + } - while(cmdline[memsize -2] != 0) - { - cmdline[memsize -1] = (char) fgetc(fp); - tempptr = realloc(cmdline, memsize + 32); - if(tempptr == NULL) - { - fclose(fp); - SEC_SVR_DBG("%s", "Out of memory"); - return NULL; - } - cmdline = tempptr; - bzero(cmdline + memsize, 32); - fgets(cmdline + memsize, 32, fp); - memsize += 32; - } + // read less than requested + if ((size_t)cnt < size) + break; - if(fp != NULL) - fclose(fp); - return cmdline; + // read exactly the number of bytes requested + free(exe); + if (size > (SIZE_MAX >> 1)) { + SEC_SVR_DBG("Exe path too long (more than %d characters)", size); + return NULL; + } + size <<= 1; + } + // readlink does not append null byte to buffer. + exe[cnt] = '\0'; + return exe; } /* Return code in packet is positive integer * @@ -314,7 +307,7 @@ int authenticate_server(int sockfd) int retval; struct ucred cr; unsigned int cl = sizeof(cr); -/* char *cmdline = NULL;*/ +/* char *exe = NULL;*/ /* get socket peer credential */ if(getsockopt(sockfd, SOL_SOCKET, SO_PEERCRED, &cr, &cl) != 0) @@ -336,22 +329,22 @@ int authenticate_server(int sockfd) /* Read command line of the PID from proc fs */ /* This is commented out because non root process cannot read link of /proc/pid/exe */ -/* cmdline = read_cmdline_from_proc(cr.pid); +/* exe = read_exe_path_from_proc(cr.pid); - if(strcmp(cmdline, SECURITY_SERVER_DAEMON_PATH) != 0) + if(strcmp(exe, SECURITY_SERVER_DAEMON_PATH) != 0) { retval = SECURITY_SERVER_ERROR_AUTHENTICATION_FAILED; - SEC_SVR_DBG("Cmdline is different. auth failed. cmdline=%s", cmdline); + SEC_SVR_DBG("Executable path is different. auth failed. Exe path=%s", exe); } else { retval = SECURITY_SERVER_SUCCESS; - SEC_SVR_DBG("Server authenticatd. %s, sockfd=%d", cmdline, sockfd); + SEC_SVR_DBG("Server authenticatd. %s, sockfd=%d", exe, sockfd); } */ error: -/* if(cmdline != NULL) - free(cmdline); +/* if(exe != NULL) + free(exe); */ return retval; } @@ -2318,13 +2311,16 @@ error: /* Checking client is pre-defined middleware daemons * * Check privilege API is only allowed to middleware daemons * - * cmd line list of middleware daemons are listed in + * list of middleware daemons' executables are listed in * /usr/share/security-server/mw-list */ -int search_middleware_cmdline(char *cmdline) +int search_middleware_exe_path(char *exe) { FILE *fp = NULL; - int ret; - char middleware[SECURITY_SERVER_MAX_PATH_LEN]; + int ret= SECURITY_SERVER_ERROR_AUTHENTICATION_FAILED; + size_t len = 0; + ssize_t cnt = 0; + char *middleware = NULL; + int cmp = 0; /* Open the list file */ fp = fopen(SECURITY_SERVER_MIDDLEWARE_LIST_PATH, "r"); @@ -2335,19 +2331,26 @@ int search_middleware_cmdline(char *cmdline) return SECURITY_SERVER_ERROR_FILE_OPERATION; } - /* Search each line */ - ret = SECURITY_SERVER_ERROR_AUTHENTICATION_FAILED; - while(fgets(middleware, SECURITY_SERVER_MAX_PATH_LEN, fp) != NULL) - { - if(strncmp(middleware, cmdline, strlen(middleware)-1) == 0) - { - /* found */ - SEC_SVR_DBG("%s", "found matching cmd line"); - ret = SECURITY_SERVER_SUCCESS; - break; - } + /* read file line by line */ + while ((cnt = getline(&middleware, &len, fp)) != -1) { - } + /* trim trailing whitespaces */ + while (cnt > 0 && isspace(middleware[cnt-1])!=0 ) + cnt--; + middleware[cnt]='\0'; + + /* compare middleware list entry with executable */ + cmp = strcmp(middleware, exe); + free(middleware); + middleware = NULL; + if (cmp == 0) + { + /* found */ + SEC_SVR_DBG("%s", "found matching executable"); + ret = SECURITY_SERVER_SUCCESS; + break; + } + } if(fp != NULL) fclose(fp); return ret; @@ -2361,7 +2364,7 @@ int authenticate_client_middleware(int sockfd, int *pid) int retval = SECURITY_SERVER_ERROR_AUTHENTICATION_FAILED; struct ucred cr; unsigned int cl = sizeof(cr); - char *cmdline = NULL; + char *exe = NULL; struct passwd pw, *ppw; size_t buf_size; char *buf; @@ -2403,22 +2406,22 @@ int authenticate_client_middleware(int sockfd, int *pid) } /* Read command line of the PID from proc fs */ - cmdline = read_cmdline_from_proc(cr.pid); - if(cmdline == NULL) + exe = read_exe_path_from_proc(cr.pid); + if(exe == NULL) { /* It's weired. no file in proc file system, */ retval = SECURITY_SERVER_ERROR_FILE_OPERATION; - SEC_SVR_DBG("Error on opening /proc/%d/cmdline", cr.pid); + SEC_SVR_DBG("Error on opening /proc/%d/exe", cr.pid); goto error; } - /* Search cmdline of the peer that is really middleware executable */ - retval = search_middleware_cmdline(cmdline); + /* Search executable of the peer that is really middleware executable */ + retval = search_middleware_exe_path(exe); *pid = cr.pid; error: - if(cmdline != NULL) - free(cmdline); + if(exe != NULL) + free(exe); return retval; } @@ -2539,7 +2542,7 @@ int authenticate_developer_shell(int sockfd) int retval = SECURITY_SERVER_ERROR_AUTHENTICATION_FAILED; struct ucred cr; unsigned int cl = sizeof(cr); - char *cmdline = NULL; + char *exe = NULL; /* get PID of socket peer */ if(getsockopt(sockfd, SOL_SOCKET, SO_PEERCRED, &cr, &cl) != 0) @@ -2557,20 +2560,20 @@ int authenticate_developer_shell(int sockfd) goto error; } - /* Read command line of the PID from proc fs */ - cmdline = read_cmdline_from_proc(cr.pid); - if(cmdline == NULL) + /* Read executable path of the PID from proc fs */ + exe = read_exe_path_from_proc(cr.pid); + if(exe == NULL) { /* It's weired. no file in proc file system, */ retval = SECURITY_SERVER_ERROR_FILE_OPERATION; - SEC_SVR_DBG("Error on opening /proc/%d/cmdline", cr.pid); + SEC_SVR_DBG("Error on opening /proc/%d/exe", cr.pid); goto error; } - /* Search cmdline of the peer that is really debug tool */ - if(strncmp(cmdline, SECURITY_SERVER_DEBUG_TOOL_PATH, strlen(SECURITY_SERVER_DEBUG_TOOL_PATH)) != 0) + /* Search exe of the peer that is really debug tool */ + if(strcmp(exe, SECURITY_SERVER_DEBUG_TOOL_PATH) != 0) { - SEC_SVR_DBG("Error: Wrong cmdline [%s]", cmdline); + SEC_SVR_DBG("Error: Wrong exe path [%s]", exe); retval = SECURITY_SERVER_ERROR_AUTHENTICATION_FAILED; goto error; } @@ -2578,8 +2581,8 @@ int authenticate_developer_shell(int sockfd) SEC_SVR_DBG("%s", "Client Authenticated"); error: - if(cmdline != NULL) - free(cmdline); + if(exe != NULL) + free(exe); return retval; } diff --git a/src/include/security-server-comm.h b/src/include/security-server-comm.h index cc2dd8f..8f2de9a 100644 --- a/src/include/security-server-comm.h +++ b/src/include/security-server-comm.h @@ -97,7 +97,7 @@ int authenticate_client_application(int sockfd, int *pid, int *uid); int authenticate_client_middleware(int sockfd, int *pid); int get_client_gid_list(int sockfd, int ** privileges); int authenticate_developer_shell(int sockfd); -char *read_cmdline_from_proc(pid_t pid); +char *read_exe_path_from_proc(pid_t pid); int send_generic_response (int sockfd, unsigned char msgid, unsigned char return_code); int send_cookie(int sockfd, unsigned char *cookie); int send_object_name(int sockfd, char *obj); diff --git a/src/include/security-server-common.h b/src/include/security-server-common.h index 7874384..fa2bded 100644 --- a/src/include/security-server-common.h +++ b/src/include/security-server-common.h @@ -62,7 +62,6 @@ #define MAX_MODE_STR_LEN 16 #define SECURITY_SERVER_MIDDLEWARE_LIST_PATH "/usr/share/security-server/mw-list" #define SECURITY_SERVER_MAX_OBJ_NAME 30 -#define SECURITY_SERVER_MAX_PATH_LEN 50 #define SECURITY_SERVER_MSG_VERSION 0x01 #define SECURITY_SERVER_ACCEPT_TIMEOUT_MILISECOND 10000 #define SECURITY_SERVER_SOCKET_TIMEOUT_MILISECOND 3000 @@ -95,7 +94,7 @@ typedef struct _cookie_list int path_len; /* Client process cmd line length */ int permission_len; /* Client process permissions (aka group IDs) */ pid_t pid; /* Client process's PID */ - char *path; /* Client process's cmd line string */ + char *path; /* Client process's executable path */ int *permissions; /* Array of GID that the client process has */ char *smack_label; /* SMACK label of the client process */ char is_roots_process; /* Is cookie belongs to roots process */ diff --git a/src/server/security-server-cookie.c b/src/server/security-server-cookie.c index 6ed2f30..25ce19d 100644 --- a/src/server/security-server-cookie.c +++ b/src/server/security-server-cookie.c @@ -126,7 +126,7 @@ cookie_list * garbage_collection(cookie_list *cookie) cookie_list *search_existing_cookie(int pid, const cookie_list *c_list) { cookie_list *current =(cookie_list *)c_list, *cookie = NULL; - char *cmdline = NULL, *debug_cmdline = NULL; + char *exe = NULL, *debug_cmdline = NULL; /* Search from the list */ while(current != NULL) @@ -141,27 +141,26 @@ cookie_list *search_existing_cookie(int pid, const cookie_list *c_list) { /* Found cookie for the pid. Check the cookie is reused by dirrent executable */ /* Check the path of the process */ - cmdline = (char*)read_cmdline_from_proc(pid); - if(cmdline == NULL) + exe = read_exe_path_from_proc(pid); + if(exe == NULL) { SEC_SVR_DBG("%s", "cannot read cmdline"); return NULL; } - /* Check the path is different */ - if(strncmp(cmdline, current->path, current->path_len) != 0 - || (int)strlen(cmdline) != current->path_len) + /* Check the path is different. */ + if(strcmp(exe, current->path) != 0) { - SEC_SVR_DBG("pid [%d] has been reused by %s. deleting the old cookie.", pid, cmdline); + SEC_SVR_DBG("pid [%d] has been reused by %s. deleting the old cookie.", pid, exe); debug_cmdline = malloc(current->path_len + 1); if(debug_cmdline == NULL) { SEC_SVR_DBG("%s", "out of memory error"); - free(cmdline); + free(exe); return NULL; } strncpy(debug_cmdline, current->path, current->path_len); debug_cmdline[current->path_len] = 0; - SEC_SVR_DBG("[%s] --> [%s]", cmdline, debug_cmdline); + SEC_SVR_DBG("[%s] --> [%s]", exe, debug_cmdline); if(debug_cmdline != NULL) { free(debug_cmdline); @@ -169,10 +168,10 @@ cookie_list *search_existing_cookie(int pid, const cookie_list *c_list) } /* Okay. delete current cookie */ current = delete_cookie_item(current); - if(cmdline != NULL) + if(exe != NULL) { - free(cmdline); - cmdline = NULL; + free(exe); + exe = NULL; } continue; } @@ -182,10 +181,10 @@ cookie_list *search_existing_cookie(int pid, const cookie_list *c_list) cookie = current; } - if(cmdline != NULL) + if(exe != NULL) { - free(cmdline); - cmdline = NULL; + free(exe); + exe = NULL; } } current = current->next; @@ -358,7 +357,7 @@ cookie_list *create_cookie_item(int pid, int sockfd, cookie_list *c_list) { int ret, tempint; cookie_list *added = NULL, *current = NULL; - char path[24], *cmdline = NULL; + char path[24], *exe = NULL; char *buf = NULL, inputed, *tempptr = NULL; char delim[] = ": ", *token = NULL; int *permissions = NULL, perm_num = 1, cnt, i, *tempperm = NULL; @@ -374,11 +373,11 @@ cookie_list *create_cookie_item(int pid, int sockfd, cookie_list *c_list) goto error; } - /* Read command line of the PID from proc fs */ - cmdline = (char *)read_cmdline_from_proc(pid); - if(cmdline == NULL) + /* Read executable name of the PID from proc fs */ + exe = (char *)read_exe_path_from_proc(pid); + if(exe == NULL) { - SEC_SVR_DBG("Error on reading /proc/%d/cmdline", pid); + SEC_SVR_DBG("Error on reading /proc/%d/exe", pid); goto error; } @@ -518,9 +517,9 @@ out_of_while: goto error; } - added->path_len = strlen(cmdline); - added->path = calloc(1, strlen(cmdline)); - memcpy(added->path, cmdline, strlen(cmdline)); + added->path_len = strlen(exe); + added->path = calloc(1, strlen(exe)); + memcpy(added->path, exe, strlen(exe)); added->permission_len = perm_num; added->pid = pid; @@ -531,8 +530,8 @@ out_of_while: added->next = NULL; error: - if(cmdline != NULL) - free(cmdline); + if(exe != NULL) + free(exe); if(fp != NULL) fclose(fp); if(buf != NULL) -- 2.7.4