From: Pawel Polawski Date: Tue, 26 Feb 2013 07:51:09 +0000 (+0100) Subject: Fix bug with searching cookie with no privileges. X-Git-Tag: submit/tizen_2.1/20130424.233001~5^2~3 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9134aabef18db396395c0eebeecd6de2c5f30e84;p=platform%2Fcore%2Fsecurity%2Fsecurity-server.git Fix bug with searching cookie with no privileges. [Issue#] SSDWSSP-102 [Bug] Bug with get_smack_label in security-server [Cause] Error in searching cookie with no privileges set [Soulution] Special trading cookies with no privileges set [Verification] Code compiles with success Change-Id: I25debbc88315f316ed08b1cda2895bd3d9d90116 --- diff --git a/src/communication/security-server-comm.c b/src/communication/security-server-comm.c index 56a4d14..e83a14a 100644 --- a/src/communication/security-server-comm.c +++ b/src/communication/security-server-comm.c @@ -2426,6 +2426,115 @@ error: return retval; } +/* Get app PID from socked and read its privilege (GID) list + * from /proc//status. + * + * param 1: socket descriptor + * param 2: pointer for hold returned array + * + * ret: size of array or -1 in case of error + * + * Notice that user must free space allocated in this function and + * returned by second parameter (int * privileges) + * */ +int get_client_gid_list(int sockfd, int ** privileges) +{ + int ret; + //for read socket options + struct ucred socopt; + unsigned int socoptSize = sizeof(socopt); + //privileges to be returned + int privilegesSize; + //buffer for store /proc//status filepath + const int PATHSIZE = 24; + char path[PATHSIZE]; + //file pointer + FILE * fp = NULL; + //buffer for filelines + const int LINESIZE = 128; + char fileLine[LINESIZE]; + //for parsing file + char delim[] = ": "; + char * token = NULL; + + + //clear pointer + *privileges = NULL; + + //read socket options + ret = getsockopt(sockfd, SOL_SOCKET, SO_PEERCRED, &socopt, &socoptSize); + if(ret != 0) + { + SEC_SVR_DBG("%s", "Error on getsockopt"); + return -1; + } + + //now we have PID in sockopt.pid + bzero(path, PATHSIZE); + snprintf(path, PATHSIZE, "/proc/%d/status", socopt.pid); + + fp = fopen(path, "r"); + if(fp == NULL) + { + SEC_SVR_DBG("%s", "Error on fopen"); + return -1; + } + + bzero(fileLine, LINESIZE); + + //search for line beginning with "Groups:" + while(strncmp(fileLine, "Groups:", 7) != 0) + { + ret = fgets(fileLine, LINESIZE, fp); + if(ret == NULL) + { + SEC_SVR_DBG("%s", "Error on fgets"); + fclose(fp); + return -1; + } + } + + fclose(fp); + + //now we have "Groups:" line in fileLine[] + ret = 0; + token = strtok(fileLine, delim); + while(token = strtok(NULL, delim)) + { + //add found GID + if(*privileges == NULL) + { + //first GID on list + *privileges = (int *)malloc(sizeof(int) * 1); + if(*privileges == NULL) + { + SEC_SVR_DBG("%s", "Error on malloc"); + return -1; + } + (*privileges)[0] = atoi(token); + } + else + { + *privileges = realloc(*privileges, sizeof(int) * (ret + 1)); + (*privileges)[ret] = atoi(token); + } + + ret++; + } + + //check if we found any GIDs for process + if(*privileges == NULL) + { + SEC_SVR_DBG("%s %d", "No GIDs found for PID:", socopt.pid); + } + else + { + SEC_SVR_DBG("%s %d", "Number of GIDs found:", ret); + } + + return ret; +} + /* Authenticate the application is middleware daemon * The middleware must run as root and the cmd line must be pre listed */ int authenticate_developer_shell(int sockfd) diff --git a/src/include/security-server-comm.h b/src/include/security-server-comm.h index b132caf..cc2dd8f 100644 --- a/src/include/security-server-comm.h +++ b/src/include/security-server-comm.h @@ -95,6 +95,7 @@ int connect_to_server(int *fd); int accept_client(int server_sockfd); 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); int send_generic_response (int sockfd, unsigned char msgid, unsigned char return_code); diff --git a/src/include/security-server-cookie.h b/src/include/security-server-cookie.h index 3acc720..c24fcda 100644 --- a/src/include/security-server-cookie.h +++ b/src/include/security-server-cookie.h @@ -27,7 +27,7 @@ int free_cookie_item(cookie_list *cookie); cookie_list *delete_cookie_item(cookie_list *cookie); cookie_list *search_existing_cookie(int pid, const cookie_list *c_list); -cookie_list *search_cookie(const cookie_list *c_list, const unsigned char *cookie, int privilege); +cookie_list *search_cookie(const cookie_list *c_list, const unsigned char *cookie, int * privileges, int privilegesSize); cookie_list *search_cookie_new(const cookie_list *c_list, const unsigned char *cookie, const char *object, diff --git a/src/server/security-server-cookie.c b/src/server/security-server-cookie.c index a084a11..b5571c0 100644 --- a/src/server/security-server-cookie.c +++ b/src/server/security-server-cookie.c @@ -219,10 +219,10 @@ finish: /* Search existing cookie from the cookie list for matching cookie and privilege */ /* If privilege is 0, just search cookie exists or not */ -cookie_list *search_cookie(const cookie_list *c_list, const unsigned char *cookie, int privilege) +cookie_list *search_cookie(const cookie_list *c_list, const unsigned char *cookie, int * privileges, int privilegesSize) { cookie_list *current = (cookie_list *)c_list, *retval = NULL; - int i; + int i, j; /* Search from the list */ while(current != NULL) @@ -236,9 +236,9 @@ cookie_list *search_cookie(const cookie_list *c_list, const unsigned char *cooki //searching for cookie if(memcmp(current->cookie, cookie, SECURITY_SERVER_COOKIE_LEN) == 0) { - SEC_SVR_DBG("%s", "cookie has been found"); + SEC_SVR_DBG("%s", "Cookie has been found"); - //check if this cookie belongs to root process + //check if this cookie belongs to root process (root process created it) if(current->is_roots_process == 1) { SEC_SVR_DBG("%s", "Root process cookie, special privileges"); @@ -247,15 +247,33 @@ cookie_list *search_cookie(const cookie_list *c_list, const unsigned char *cooki goto finish; } - for(i=0 ; i < current->permission_len ; i++) - { - if(privilege == current->permissions[i]) - { - SEC_SVR_DBG("Found privilege %d", privilege); - retval = current; - goto finish; - } - } + if((privileges == NULL) || (privilegesSize == 0)) + { + SEC_SVR_DBG("%s", "No privileges to search in cookie!"); + } + else if(current->permissions == NULL) + { + SEC_SVR_DBG("%s", "Cookie has no privileges inside!"); + } + else + { + SEC_SVR_DBG("%s", "Searching for privileges"); + SEC_SVR_DBG("%s %d", "Privileges in cookie:", current->permission_len); + SEC_SVR_DBG("%s %d", "Privileges to search:", privilegesSize); + + for(j = 0; j < privilegesSize; j++) + { + for(i = 0; i < current->permission_len; i++) + { + if(privileges[j] == current->permissions[i]) + { + SEC_SVR_DBG("Found privilege %d", privileges[j]); + retval = current; + goto finish; + } + } + } + } } current = current->next; } diff --git a/src/server/security-server-main.c b/src/server/security-server-main.c index 94fe1f5..34d4f2a 100644 --- a/src/server/security-server-main.c +++ b/src/server/security-server-main.c @@ -391,6 +391,7 @@ int process_check_privilege_request(int sockfd) { /* Authenticate client */ int retval, client_pid, requested_privilege; + int privileges[1]; unsigned char requested_cookie[SECURITY_SERVER_COOKIE_LEN]; cookie_list *search_result = NULL; @@ -438,7 +439,8 @@ int process_check_privilege_request(int sockfd) /* Search cookie list */ pthread_mutex_lock(&cookie_mutex); - search_result = search_cookie(c_list, requested_cookie, requested_privilege); + privileges[0] = requested_privilege; + search_result = search_cookie(c_list, requested_cookie, privileges, 1); pthread_mutex_unlock(&cookie_mutex); if(search_result != NULL) { @@ -714,6 +716,7 @@ int process_pid_request(int sockfd) { int retval, client_pid; unsigned char requested_cookie[SECURITY_SERVER_COOKIE_LEN]; + int * privileges; cookie_list *search_result = NULL; /* Authenticate client */ @@ -747,7 +750,17 @@ int process_pid_request(int sockfd) /* Search cookie list */ pthread_mutex_lock(&cookie_mutex); - search_result = search_cookie(c_list, requested_cookie, 0); + + retval = get_client_gid_list(sockfd, &privileges); + if(retval < 0) + { + SEC_SVR_DBG("ERROR: Cannot get GID list"); + goto error; + } + + search_result = search_cookie(c_list, requested_cookie, privileges, retval); + free(privileges); + pthread_mutex_unlock(&cookie_mutex); if(search_result != NULL) { @@ -779,9 +792,10 @@ error: int process_smack_request(int sockfd) { - int retval, client_pid; - unsigned char requested_cookie[SECURITY_SERVER_COOKIE_LEN]; - cookie_list *search_result = NULL; + int retval, client_pid; + int privileges[1]; + unsigned char requested_cookie[SECURITY_SERVER_COOKIE_LEN]; + cookie_list *search_result = NULL; //handler for SMACK label char * label = NULL; //buffer for storing file path @@ -804,29 +818,39 @@ int process_smack_request(int sockfd) goto error; } - retval = recv_smack_request(sockfd, requested_cookie); - if(retval == SECURITY_SERVER_ERROR_RECV_FAILED) - { - SEC_SVR_DBG("%s", "Receiving request failed"); - retval = send_generic_response(sockfd, - SECURITY_SERVER_MSG_TYPE_SMACK_RESPONSE, - SECURITY_SERVER_RETURN_CODE_BAD_REQUEST); - if(retval != SECURITY_SERVER_SUCCESS) - { - SEC_SVR_DBG("ERROR: Cannot send generic response: %d", retval); - } - goto error; - } + retval = recv_smack_request(sockfd, requested_cookie); + if(retval == SECURITY_SERVER_ERROR_RECV_FAILED) + { + SEC_SVR_DBG("%s", "Receiving request failed"); + retval = send_generic_response(sockfd, + SECURITY_SERVER_MSG_TYPE_SMACK_RESPONSE, + SECURITY_SERVER_RETURN_CODE_BAD_REQUEST); + if(retval != SECURITY_SERVER_SUCCESS) + { + SEC_SVR_DBG("ERROR: Cannot send generic response: %d", retval); + } + goto error; + } - /* Search cookie list */ - pthread_mutex_lock(&cookie_mutex); - search_result = search_cookie(c_list, requested_cookie, 0); - pthread_mutex_unlock(&cookie_mutex); - if(search_result != NULL) - { - /* We found */ - SEC_SVR_DBG("We found the cookie and pid:%d", search_result->pid); - SEC_SVR_DBG("%s", "Cookie comparison succeeded. Access granted."); + /* Search cookie list */ + pthread_mutex_lock(&cookie_mutex); + + retval = get_client_gid_list(sockfd, &privileges); + if(retval < 0) + { + SEC_SVR_DBG("ERROR: Cannot get GID list"); + goto error; + } + + search_result = search_cookie(c_list, requested_cookie, privileges, retval); + free(privileges); + + pthread_mutex_unlock(&cookie_mutex); + if(search_result != NULL) + { + /* We found */ + SEC_SVR_DBG("We found the cookie and pid:%d", search_result->pid); + SEC_SVR_DBG("%s", "Cookie comparison succeeded. Access granted."); //clearing buffer memset(path, 0x00, BUFFSIZE); diff --git a/src/util/security-server-util-common.c b/src/util/security-server-util-common.c index e28786a..675c729 100644 --- a/src/util/security-server-util-common.c +++ b/src/util/security-server-util-common.c @@ -336,6 +336,7 @@ int util_process_cookie_from_cookie(int sockfd, cookie_list* list) { unsigned char cookie[SECURITY_SERVER_COOKIE_LEN]; int ret; + int privileges[] = { 0 }; //only one privilege to check - root cookie_list *result = NULL; ret = read(sockfd, cookie, SECURITY_SERVER_COOKIE_LEN); @@ -344,7 +345,7 @@ int util_process_cookie_from_cookie(int sockfd, cookie_list* list) SEC_SVR_DBG("Received cookie size is too small: %d", ret); return SECURITY_SERVER_ERROR_RECV_FAILED; } - result = search_cookie(list, cookie, 0); + result = search_cookie(list, cookie, privileges, 1); if(result == NULL) { ret = send_generic_response(sockfd, SECURITY_SERVER_MSG_TYPE_GET_COOKIEINFO_RESPONSE,