From 935ed847bcf9c4909a5198a232b15b3adb607bdf Mon Sep 17 00:00:00 2001 From: Yunjin Lee Date: Fri, 26 Jun 2020 19:01:20 +0900 Subject: [PATCH] Modify privilege verification messaging - Adjust function/variable name to its role. - Remove redunant code. - Remove build warning bypass trick. - Make messaging logic simpler. Change-Id: Ib5e113f147bbbaf4597da3ccb2798467f1ddbae3 Signed-off-by: Yunjin Lee --- capi/include/privilege_manager_types.h | 40 ++-- capi/src/privilege_manager.c | 243 ++++++++++--------------- 2 files changed, 110 insertions(+), 173 deletions(-) diff --git a/capi/include/privilege_manager_types.h b/capi/include/privilege_manager_types.h index 5b5f751..5565ba2 100644 --- a/capi/include/privilege_manager_types.h +++ b/capi/include/privilege_manager_types.h @@ -114,33 +114,27 @@ typedef enum { * @brief Enumerations of privilege verification error type for guide message */ typedef enum { - G_WEB, - G_NATIVE, - G_METADATA, - E_DPM_BANNED_PRIVILEGE, - E_DEPRECATED_AND_CHANGED, - E_DEPRECATED, - E_NOT_EXIST_YET, - E_NOT_EXIST, - E_CERT_LEVEL_MISMATCHED, - MAX_VERIFICATION_INFO, -} privilege_verification_error_type_e; + MSG_GUIDE_WEB, + MSG_GUIDE_NATIVE, + MSG_GUIDE_METADATA, + MSG_ERROR_DPM_BANNED_PRIVILEGE, + MSG_HEADER_CERT_LEVEL_MISMATCHED, + MSG_ERROR_CERT_LEVEL_MISMATCHED, + MAX_PRIVILEGE_VERIFICATION_MSG_TYPE, +} privilege_verification_msg_type_e; typedef struct { - privilege_verification_error_type_e type; + privilege_verification_msg_type_e type; const char* msg; -} privilege_verification_error_s; +} privilege_verification_msg_s; -static const privilege_verification_error_s prvmgr_msg_table[MAX_VERIFICATION_INFO] = { - {G_WEB, "Check config.xml| - Current required_version(=api version) = %s, | certificate signature level = %s||"}, - {G_NATIVE, "Check tizen-manifest.xml| - Current api-version = %s, | certificate signature level = %s||"}, - {G_METADATA, "Check tizen-manifest.xml or config.xml| - Current api-version = %s, | certificate signature level = %s||"}, - {E_DPM_BANNED_PRIVILEGE, "Application manifest contains banned privilege(s) declared by the DPM."}, - {E_DEPRECATED_AND_CHANGED, " - %s| >> Use %s instead of it or use api version lower than %s.|"}, - {E_DEPRECATED, " - %s| >> Remove the privilege.|"}, - {E_NOT_EXIST_YET, "- %s| >> Use at least api version %s or remove the privilege.|"}, - {E_NOT_EXIST, " - %s| >> Check spelling or remove the privilege.|"}, - {E_CERT_LEVEL_MISMATCHED, " - %s| >> Use at least %s signatured certificate.|"}, +static const privilege_verification_msg_s prvmgr_msg_table[MAX_PRIVILEGE_VERIFICATION_MSG_TYPE] = { + {MSG_GUIDE_WEB, "Check config.xml| - Current certificate signature level = %s||"}, + {MSG_GUIDE_NATIVE, "Check tizen-manifest.xml| - Current certificate signature level = %s||"}, + {MSG_GUIDE_METADATA, "Check tizen-manifest.xml or config.xml| - Current certificate signature level = %s||"}, + {MSG_ERROR_DPM_BANNED_PRIVILEGE, "App manifest contains banned privilege(s) declared by the DPM."}, + {MSG_HEADER_CERT_LEVEL_MISMATCHED, "[MISMATCHED_PRIVILEGE_LEVEL]|"}, + {MSG_ERROR_CERT_LEVEL_MISMATCHED, " - %s| >> Use at least %s signatured certificate.|"}, }; #ifdef __cplusplus diff --git a/capi/src/privilege_manager.c b/capi/src/privilege_manager.c index 30cf9a6..b2761e5 100755 --- a/capi/src/privilege_manager.c +++ b/capi/src/privilege_manager.c @@ -45,19 +45,6 @@ #define _LOGI(fmt, arg...) #endif -#ifdef __GNUC__ -#if __GNUC__ >= 8 -#define PUSH_IGNORE_STRING_WARNINGS \ - _Pragma("GCC diagnostic push") \ - _Pragma("GCC diagnostic ignored \"-Wstringop-truncation\"") \ - _Pragma("GCC diagnostic ignored \"-Wstringop-overflow\"") -#define POP_IGNORE_STRING_WARNINGS _Pragma("GCC diagnostic pop") -#else -#define PUSH_IGNORE_STRING_WARNINGS -#define POP_IGNORE_STRING_WARNINGS -#endif -#endif - #define TryReturn(condition, expr, returnValue, ...)\ if (!(condition)) { \ _LOGE(__VA_ARGS__); \ @@ -65,6 +52,13 @@ return returnValue; \ } +#define SafeFree(var) { \ + if (var != NULL) { \ + free(var); \ + var = NULL; \ + } \ +} + int __get_api_version_code(const char *api_version, api_version_code_t *api_version_code) { TryReturn(api_version != NULL, , PRVMGR_ERR_INVALID_PARAMETER, "[PRVMGR_ERR_INVALID_PARAMETER] api_version is NULL"); @@ -107,8 +101,7 @@ int __get_api_version_code(const char *api_version, api_version_code_t *api_vers static void __free_privilege_info(privilege_info_db_row_s *privilege_info) { - if (privilege_info->privilege_name != NULL) - free(privilege_info->privilege_name); + SafeFree(privilege_info->privilege_name); } static void __free_privilege_list(gpointer privilege_list) @@ -116,71 +109,41 @@ static void __free_privilege_list(gpointer privilege_list) __free_privilege_info((privilege_info_db_row_s*)privilege_list); } -static int __privilege_manager_check_privilege_list(const char *privilege, GList *valid_privilege_list, int *privilege_level) + +static int __comp_privilege_name(gconstpointer a, gconstpointer b) { - int ret_val = PRVMGR_ERR_NO_EXIST_PRIVILEGE; - GList *l = NULL; - for (l = valid_privilege_list; l != NULL; l = l->next) { - privilege_info_db_row_s *privilege_info_db_row = (privilege_info_db_row_s *)l->data; - if (strcmp(privilege_info_db_row->privilege_name, privilege) == 0) { - *privilege_level = privilege_info_db_row->privilege_level_id; - ret_val = PRVMGR_ERR_NONE; - break; - } - } - return ret_val; + privilege_info_db_row_s *info = (privilege_info_db_row_s*)a; + + return strcmp(info->privilege_name, b); } -static const char *__get_privilege_level_string(privilege_manager_visibility_e visibility) +static int __get_privilege_level(const char *privilege, GList *privilege_list, int *privilege_level) { - switch (visibility) { - case PRVMGR_PACKAGE_VISIBILITY_PUBLIC: - return "public"; - case PRVMGR_PACKAGE_VISIBILITY_PARTNER: - return "partner"; - case PRVMGR_PACKAGE_VISIBILITY_PLATFORM: - return "platform"; - default: - _LOGE("__get_package_type_string() failed. No matched privilege level string."); - return ""; + int ret = PRVMGR_ERR_NO_EXIST_PRIVILEGE; + + GList *matched = g_list_find_custom(privilege_list, privilege, __comp_privilege_name); + if (matched != NULL) { + *privilege_level = ((privilege_info_db_row_s*)matched->data)->privilege_level_id; + ret = PRVMGR_ERR_NONE; } + return ret; } -const char *__get_package_type_string(privilege_manager_package_type_e type) +static const char *__convert_privilege_level_to_string(privilege_manager_visibility_e level) { - switch (type) { - case PRVMGR_PACKAGE_TYPE_WRT: - return "Web"; - case PRVMGR_PACKAGE_TYPE_CORE: - return "Native"; - case PRVMGR_PACKAGE_TYPE_METADATA: - return "Metadata"; - default: - _LOGE("__get_package_type_string() failed. No matched package type string."); - return ""; - } + if (level == PRVMGR_PACKAGE_VISIBILITY_PUBLIC) + return "public"; + else if (level == PRVMGR_PACKAGE_VISIBILITY_PARTNER) + return "partner"; + else + return "platform"; } -static const char *__get_format(privilege_verification_error_type_e type) +static const char *__get_format(privilege_verification_msg_type_e type) { return prvmgr_msg_table[type].msg; } -static char *__make_message_from_type(privilege_verification_error_type_e type, ...) -{ - char *buf = NULL; - va_list ap; - va_start(ap, type); - int ret = g_vasprintf(&buf, __get_format(type), ap); - va_end(ap); - if (ret == -1) { - _LOGE("[PRVMGR_INTERNAL_ERROR] g_vasprintf failed"); - if (buf != NULL) - free(buf); - return NULL; - } - return buf; -} static char *__make_message_from_format(const char *fmt, ...) { @@ -189,12 +152,7 @@ static char *__make_message_from_format(const char *fmt, ...) va_start(ap, fmt); int ret = g_vasprintf(&buf, fmt, ap); va_end(ap); - if (ret == -1) { - _LOGE("[PRVMGR_INTERNAL_ERROR] g_vasprintf failed"); - if (buf != NULL) - free(buf); - return NULL; - } + TryReturn(ret != -1, SafeFree(buf), NULL, "[PRVMGR_INTERNAL_ERROR] g_vasprintf failed."); return buf; } @@ -205,16 +163,21 @@ static int __is_valid_package_type(privilege_manager_package_type_e package_type return 1; } -static char *__make_guide_message(privilege_manager_package_type_e package_type, const char *api_version, privilege_manager_visibility_e level) +static int __is_valid_cert_level(privilege_manager_visibility_e level) { - switch (package_type) { - case PRVMGR_PACKAGE_TYPE_WRT: - return __make_message_from_type(G_WEB, api_version, __get_privilege_level_string(level)); - case PRVMGR_PACKAGE_TYPE_CORE: - return __make_message_from_type(G_NATIVE, api_version, __get_privilege_level_string(level)); - default: - return __make_message_from_type(G_METADATA, api_version, __get_privilege_level_string(level)); - } + if (level != PRVMGR_PACKAGE_VISIBILITY_PUBLIC && level != PRVMGR_PACKAGE_VISIBILITY_PARTNER && level != PRVMGR_PACKAGE_VISIBILITY_PLATFORM) + return 0; + return 1; +} + +static const char *__get_guide_format(privilege_manager_package_type_e package_type) +{ + if (package_type == PRVMGR_PACKAGE_TYPE_WRT) + return __get_format(MSG_GUIDE_WEB); + else if (package_type == PRVMGR_PACKAGE_TYPE_CORE) + return __get_format(MSG_GUIDE_NATIVE); + else + return __get_format(MSG_GUIDE_METADATA); } void privilege_manager_list_free(GList* list) @@ -224,49 +187,42 @@ void privilege_manager_list_free(GList* list) int privilege_manager_verify_privilege(uid_t uid, const char *api_version, privilege_manager_package_type_e package_type, GList * privilege_list, privilege_manager_visibility_e visibility, char **error_message) { - GList *l; + TryReturn(api_version != NULL, , PRVMGR_ERR_INVALID_PARAMETER, "[PRVMGR_ERR_INVALID_PARAMETER] api_version is NULL"); + TryReturn(privilege_list != NULL, , PRVMGR_ERR_INVALID_PARAMETER, "[PRVMGR_ERR_INVALID_PARAMETER] privilege_list is NULL"); + TryReturn(__is_valid_package_type(package_type), , PRVMGR_ERR_INVALID_PARAMETER, "[PRVMGR_ERR_INVALID_PARAMETER] package_type is not a PRVMGR_PACKAGE_TYPE_WRT or PRVMGR_PACKAGE_TYPE_CORE"); + TryReturn(__is_valid_cert_level(visibility), , PRVMGR_ERR_INVALID_PARAMETER, "[PRVMGR_ERR_INVALID_PARAMETER] visibility is not PRVMGR_PACKAGE_VISIBILITY_PUBLIC, PRVMGR_PACKAGE_VISIBILITY_PARTNER, or PRVMGR_PACKAGE_VISIBILITY_PLATFORM"); + int ret; int ret_val = PRVMGR_ERR_NONE; - char *message = NULL; - char *message_list = NULL; - char *mismatched_message = NULL; - GList *valid_privilege_list; - api_version_code_t api_version_code = 0; - - /* Check invalid parameters */ - if (api_version == NULL) { - _LOGE("[PRVMGR_ERR_INVALID_PARAMETER] api_version is NULL"); - return PRVMGR_ERR_INVALID_PARAMETER; - } - if (privilege_list == NULL) { - _LOGE("[PRVMGR_ERR_INVALID_PARAMETER] privilege_list is NULL"); - return PRVMGR_ERR_INVALID_PARAMETER; - } + char* msg_list[g_list_length(privilege_list)]; + for (unsigned int i = 0; i < g_list_length(privilege_list); ++i) msg_list[i] = NULL; + int msg_idx = 0; + unsigned int total_msg_size = 0; + + api_version_code_t api_version_code; + /* FIXME: Should we do api-version check? */ if (__get_api_version_code(api_version, &api_version_code) != PRVMGR_ERR_NONE) { _LOGE("[PRVMGR_ERR_INVALID_PARAMETER] The api_version is invalid. api_version should be consist of digit(0 <= x <= 255) and dot(.) in form of x, x.x, x.x.x, or x.x.x.x."); return PRVMGR_ERR_INVALID_PARAMETER; } - if (!__is_valid_package_type(package_type)) { - _LOGE("[PRVMGR_ERR_INVALID_PARAMETER] package_type is not a PRVMGR_PACKAGE_TYPE_WRT or PRVMGR_PACKAGE_TYPE_CORE"); - return PRVMGR_ERR_INVALID_PARAMETER; - } - /* Check black list */ ret = privilege_db_manager_check_black_list(uid, package_type, privilege_list); if (ret == PRIVILEGE_DB_MANAGER_ERR_DB_NOENTRY) { _LOGE("[PRVMGR_ERR_INTERNAL_ERROR] black list policy db cannot be found"); } else if (ret > 0) { - *error_message = __make_message_from_type(E_DPM_BANNED_PRIVILEGE); - TryReturn(error_message != NULL, , PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERROR_OUT_OF_MEMORY] __make_message_from_type failed"); + *error_message = __make_message_from_format(__get_format(MSG_ERROR_DPM_BANNED_PRIVILEGE)); + TryReturn(error_message != NULL, , PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERROR_OUT_OF_MEMORY] __make_message_from_format failed"); + _LOGE("%s", *error_message); return PRVMGR_ERR_USING_BANNED_PRIVILEGE; } else if (ret < 0) { _LOGE("[PRVMGR_ERR_INTERNAL_ERROR] privilege_db_manager_check_black_list failed. ret = %d", ret); return PRVMGR_ERR_INTERNAL_ERROR; } + GList *valid_privilege_list = NULL; /* Get valid privilege list */ ret = privilege_db_manager_get_privilege_list(package_type, &valid_privilege_list); if (ret != PRIVILEGE_DB_MANAGER_ERR_NONE) { @@ -277,38 +233,32 @@ int privilege_manager_verify_privilege(uid_t uid, const char *api_version, privi } return PRVMGR_ERR_INTERNAL_ERROR; } + /* Compare received privilege with valid privilege list */ + GList* l = NULL; for (l = privilege_list; l != NULL; l = l->next) { int privilege_level_id = PRVMGR_PACKAGE_VISIBILITY_PUBLIC; char *privilege_name = (char *)l->data; - if (message != NULL) { - free(message); - message = NULL; - } + if (strstr(privilege_name, "/internal/") != NULL) { - _LOGE("[PRVMGR_ERR_INVALID_PRIVILEGE] Invalid privilege name %s", privilege_name); - if (mismatched_message != NULL) - free(mismatched_message); - g_list_free_full(valid_privilege_list, __free_privilege_list); - return PRVMGR_ERR_INVALID_PRIVILEGE; + ret_val = PRVMGR_ERR_INVALID_PRIVILEGE; + _LOGE("[PRVMGR_ERR_INVALID_PRIVILEGE] Invalid privilege %s. DO NOT declare internal privilege in app manifest.", privilege_name); + goto FINISH; } - ret = __privilege_manager_check_privilege_list(privilege_name, valid_privilege_list, &privilege_level_id); + ret = __get_privilege_level(privilege_name, valid_privilege_list, &privilege_level_id); if (ret == PRVMGR_ERR_NONE) { if (visibility < privilege_level_id) { - _LOGD("[MISMATCHED_PRIVILEGE_LEVEL] %s %s requires certificate level: %s and current certificate level: %s. Use at least certificate with signature level %s.", __get_package_type_string(package_type), privilege_name, __get_privilege_level_string(privilege_level_id), __get_privilege_level_string(visibility), __get_privilege_level_string(privilege_level_id)); - message = __make_message_from_type(E_CERT_LEVEL_MISMATCHED, privilege_name, __get_privilege_level_string(privilege_level_id)); - TryReturn(message != NULL, ret_val = PRVMGR_ERR_INTERNAL_ERROR; goto FINISH, PRVMGR_ERR_INTERNAL_ERROR, "[PRVMGR_ERR_INTERNAL_ERROR] __make_message_from_type failed"); - if (mismatched_message == NULL) { - mismatched_message = strdup(message); - } else { - char *tmp = __make_message_from_format("%s%s", mismatched_message, message); - TryReturn(message != NULL, ret_val = PRVMGR_ERR_INTERNAL_ERROR; goto FINISH, PRVMGR_ERR_INTERNAL_ERROR, "[PRVMGR_ERR_INTERNAL_ERROR] __make_message_from_format failed"); - free(mismatched_message); - mismatched_message = NULL; - mismatched_message = tmp; - } + _LOGD("[MISMATCHED_PRIVILEGE_LEVEL] %s requires certificate level '%s' and current certificate level is '%s'. " \ + "Use at least certificate with signature level %s.", + privilege_name, __convert_privilege_level_to_string(privilege_level_id), __convert_privilege_level_to_string(visibility), + __convert_privilege_level_to_string(privilege_level_id)); + + msg_list[msg_idx] = __make_message_from_format(__get_format(MSG_ERROR_CERT_LEVEL_MISMATCHED), privilege_name, __convert_privilege_level_to_string(privilege_level_id)); + TryReturn(msg_list[msg_idx] != NULL, ret_val = PRVMGR_ERR_INTERNAL_ERROR; goto FINISH, PRVMGR_ERR_INTERNAL_ERROR, "[PRVMGR_ERR_INTERNAL_ERROR] __make_message_from_format() failed"); + total_msg_size += strlen(msg_list[msg_idx]); + msg_idx++; ret_val = PRVMGR_ERR_INVALID_PRIVILEGE; } } else if (ret == PRVMGR_ERR_INTERNAL_ERROR) { @@ -317,33 +267,26 @@ int privilege_manager_verify_privilege(uid_t uid, const char *api_version, privi goto FINISH; } } - char *newline = "|"; - if (ret_val != PRVMGR_ERR_NONE) { - message_list = __make_guide_message(package_type, api_version, visibility); - TryReturn(message_list != NULL, ret_val = PRVMGR_ERR_OUT_OF_MEMORY; goto FINISH, PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERR_OUT_OF_MEMORY] message_list's strdup is failed."); - if (mismatched_message != NULL) { - size_t new_size = snprintf(0, 0, "%s[MISMATCHED_PRIVILEGE_LEVEL]|%s", message_list, mismatched_message) + 1; - char *tmp_message_list = realloc(message_list, new_size); - TryReturn(tmp_message_list != NULL, ret_val = PRVMGR_ERR_OUT_OF_MEMORY; goto FINISH, PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERR_OUT_OF_MEMORY] message_list's realloc is failed."); - message_list = tmp_message_list; - PUSH_IGNORE_STRING_WARNINGS - strncat(message_list, "[MISMATCHED_PRIVILEGE_LEVEL]|", strlen("[MISMATCHED_PRIVILEGE_LEVEL]|")); - strncat(message_list, mismatched_message, strlen(mismatched_message)); - POP_IGNORE_STRING_WARNINGS + if (ret_val == PRVMGR_ERR_INVALID_PRIVILEGE) { + char *msg_header = __make_message_from_format(__get_guide_format(package_type), __convert_privilege_level_to_string(visibility)); + TryReturn(msg_header != NULL, ret_val = PRVMGR_ERR_OUT_OF_MEMORY; goto FINISH, PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERR_OUT_OF_MEMORY] __make_guide_message() failed."); + total_msg_size += strlen(msg_header) + strlen(__get_format(MSG_HEADER_CERT_LEVEL_MISMATCHED)) + 1; + + char *err_msg = (char*)calloc(total_msg_size, sizeof(char)); + TryReturn(err_msg != NULL, ret_val = PRVMGR_ERR_OUT_OF_MEMORY; SafeFree(msg_header); goto FINISH, PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERR_OUT_OF_MEMORY] calloc() of err_msg failed."); + + g_strlcat(err_msg, msg_header, total_msg_size); + SafeFree(msg_header); + g_strlcat(err_msg, __get_format(MSG_HEADER_CERT_LEVEL_MISMATCHED), total_msg_size); + for (int i = 0; i < msg_idx; ++i) { + g_strlcat(err_msg, msg_list[i], total_msg_size); } - size_t total_size = snprintf(0, 0, "%s%s", message_list, newline) + 1; - char* tmp_message_list = realloc(message_list, total_size); - TryReturn(tmp_message_list != NULL, ret_val = PRVMGR_ERR_OUT_OF_MEMORY; goto FINISH, PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERR_OUT_OF_MEMORY] message_list's realloc is failed."); - message_list = tmp_message_list; - strncat(message_list, newline, total_size - strlen(message_list) - 1); - *error_message = strdup(message_list); - TryReturn(error_message != NULL, ret_val = PRVMGR_ERR_OUT_OF_MEMORY; goto FINISH, PRVMGR_ERR_OUT_OF_MEMORY, "[PRVMGR_ERR_OUT_OF_MEMORY] error_message's strdup is failed."); + + *error_message = err_msg; } - FINISH: - free(message); - free(message_list); - free(mismatched_message); +FINISH: + for (unsigned int i = 0; i < g_list_length(privilege_list); ++i) SafeFree(msg_list[i]); g_list_free_full(valid_privilege_list, __free_privilege_list); return ret_val; } -- 2.34.1