From de5e2c48136659025f1093bfb0f1122d366b3a96 Mon Sep 17 00:00:00 2001 From: ligongshao Date: Wed, 30 Aug 2023 19:00:46 +0800 Subject: [PATCH] fix code_signature cleancode Signed-off-by: ligongshao --- .../code_sign_utils/include/stat_utils.h | 2 +- .../code_sign_utils/src/code_sign_utils.cpp | 6 +- .../src/local_code_sign_client.cpp | 6 -- services/key_enable/utils/include/key_utils.h | 1 - test/unittest/code_sign_utils_test.cpp | 101 +++++++++--------- test/unittest/local_code_sign_test.cpp | 11 +- .../unittest/multi_thread_local_sign_test.cpp | 6 +- test/unittest/sign_and_enforce_test.cpp | 23 ++-- utils/src/cert_utils.cpp | 1 - utils/src/fsverity_utils_helper.cpp | 4 +- utils/src/signer_info.cpp | 6 +- 11 files changed, 80 insertions(+), 87 deletions(-) diff --git a/interfaces/innerkits/code_sign_utils/include/stat_utils.h b/interfaces/innerkits/code_sign_utils/include/stat_utils.h index 48e8149..361a51b 100644 --- a/interfaces/innerkits/code_sign_utils/include/stat_utils.h +++ b/interfaces/innerkits/code_sign_utils/include/stat_utils.h @@ -16,9 +16,9 @@ #ifndef CODE_SIGN_STAT_UTILS_H #define CODE_SIGN_STAT_UTILS_H +#include #include #include -#include #include namespace OHOS { diff --git a/interfaces/innerkits/code_sign_utils/src/code_sign_utils.cpp b/interfaces/innerkits/code_sign_utils/src/code_sign_utils.cpp index 044de39..f92ab2a 100644 --- a/interfaces/innerkits/code_sign_utils/src/code_sign_utils.cpp +++ b/interfaces/innerkits/code_sign_utils/src/code_sign_utils.cpp @@ -19,14 +19,14 @@ #include #include #include -#include #include +#include +#include +#include #include #include #include #include -#include -#include #include "cs_hisysevent.h" #include "cs_hitrace.h" diff --git a/interfaces/innerkits/local_code_sign/src/local_code_sign_client.cpp b/interfaces/innerkits/local_code_sign/src/local_code_sign_client.cpp index 85a0b91..235bf74 100644 --- a/interfaces/innerkits/local_code_sign/src/local_code_sign_client.cpp +++ b/interfaces/innerkits/local_code_sign/src/local_code_sign_client.cpp @@ -14,12 +14,7 @@ */ #include "local_code_sign_client.h" - -#include -#include #include -#include - #include "cs_hisysevent.h" #include "local_code_sign_proxy.h" #include "local_code_sign_load_callback.h" @@ -192,7 +187,6 @@ LocalCodeSignClient *GetLocalCodeSignClient() { return &LocalCodeSignClient::GetInstance(); } - } } } \ No newline at end of file diff --git a/services/key_enable/utils/include/key_utils.h b/services/key_enable/utils/include/key_utils.h index 019eb4a..c2dbf60 100644 --- a/services/key_enable/utils/include/key_utils.h +++ b/services/key_enable/utils/include/key_utils.h @@ -17,7 +17,6 @@ #define CODE_SIGN_KEY_UTILS_H #include -#include typedef int32_t KeySerial; diff --git a/test/unittest/code_sign_utils_test.cpp b/test/unittest/code_sign_utils_test.cpp index 7a0b045..1e805c4 100644 --- a/test/unittest/code_sign_utils_test.cpp +++ b/test/unittest/code_sign_utils_test.cpp @@ -20,80 +20,81 @@ namespace OHOS { namespace Security { namespace CodeSign { - using namespace testing::ext; using namespace std; -#define TMP_BASE_PATH "/data/service/el1/public/bms/bundle_manager_service/tmp" -#define APP_BASE_PATH "/data/app/el1/bundle/public/tmp" +static const std::string TMP_BASE_PATH = "/data/service/el1/public/bms/bundle_manager_service/tmp"; +static const std::string APP_BASE_PATH = "/data/app/el1/bundle/public/tmp"; + static const EntryMap g_hapWithoutLibRetSuc = { - {"Hap", APP_BASE_PATH"/demo_without_lib/demo_without_lib.hap"}, + {"Hap", APP_BASE_PATH + "/demo_without_lib/demo_without_lib.hap"}, }; static const std::string g_sigWithoutLibRetSucPath = - TMP_BASE_PATH"/demo_without_lib/demo_without_lib.sig"; + TMP_BASE_PATH + "/demo_without_lib/demo_without_lib.sig"; static EntryMap g_hapWithMultiLibRetSuc = { {"Hap", - APP_BASE_PATH"/demo_with_multi_lib/demo_with_multi_lib.hap"}, + APP_BASE_PATH + "/demo_with_multi_lib/demo_with_multi_lib.hap"}, {"libs/arm64-v8a/libc++_shared.so", - APP_BASE_PATH"/demo_with_multi_lib/libs/arm64-v8a/libc++_shared.so"}, + APP_BASE_PATH + "/demo_with_multi_lib/libs/arm64-v8a/libc++_shared.so"}, {"libs/arm64-v8a/libentry.so", - APP_BASE_PATH"/demo_with_multi_lib/libs/arm64-v8a/libentry.so"} + APP_BASE_PATH + "/demo_with_multi_lib/libs/arm64-v8a/libentry.so"} }; static const std::string g_sigWithMultiLibRetSucPath = - TMP_BASE_PATH"/demo_with_multi_lib/demo_with_multi_lib.sig"; + TMP_BASE_PATH + "/demo_with_multi_lib/demo_with_multi_lib.sig"; -//wrong hap and wrong lib +// wrong hap and wrong lib static EntryMap g_wrongHapWithMultiLibRetFail = { {"Hap", - APP_BASE_PATH"/demo_with_multi_lib_error/demo_with_multi_lib.hap"}, + APP_BASE_PATH + "/demo_with_multi_lib_error/demo_with_multi_lib.hap"}, {"libs/arm64-v8a/libc++_shared.so", - APP_BASE_PATH"/demo_with_multi_lib_error/libs/arm64-v8a/libc++_shared.so"}, + APP_BASE_PATH + "/demo_with_multi_lib_error/libs/arm64-v8a/libc++_shared.so"}, {"libs/arm64-v8a/libentry.so", - APP_BASE_PATH"/demo_with_multi_lib_error/libs/arm64-v8a/libentry.so"} -}; + APP_BASE_PATH + "/demo_with_multi_lib_error/libs/arm64-v8a/libentry.so"}}; -//examples of Enforce code signature for app +// examples of Enforce code signature for app static const std::vector g_HapWithoutLibSigPkcs7ErrorPath({ - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_001.sig", //Ilegal pkcs7 format - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_002.sig", //Disable to find cert chain - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_003.sig", //Don't support digest algorithm - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_004.sig", //Don't support signature algorithm - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_005.sig", //Wrong signature - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_006.sig", //Expired signature - TMP_BASE_PATH"/demo_without_lib/pkcs7_error/demo_without_lib_007.sig", //Cert chain validate fail + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_001.sig", // Ilegal pkcs7 format + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_002.sig", // Disable to find cert chain + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_003.sig", // Don't support digest algorithm + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_004.sig", // Don't support signature algorithm + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_005.sig", // Wrong signature + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_006.sig", // Expired signature + TMP_BASE_PATH + "/demo_without_lib/pkcs7_error/demo_without_lib_007.sig", // Cert chain validate fail }); static const std::vector g_HapWithMultiLibSigPkcs7ErrorPath({ - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_001.sig", //Ilegal pkcs7 format - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_002.sig", //Disable to find cert chain - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_003.sig", //Don't support digest algorithm - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_004.sig", //Don't support signature algorithm - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_005.sig", //Wrong signature - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_006.sig", //Expired signature - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_007.sig", //Cert chain validate fail + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_001.sig", // Ilegal pkcs7 format + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_002.sig", // Disable to find cert chain + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_003.sig", // Don't support digest algorithm + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_004.sig", // Don't support signature algorithm + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_005.sig", // Wrong signature + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_006.sig", // Expired signature + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/demo_with_multi_lib_007.sig", // Cert chain validate fail }); -//examples of Enforce code signature for file -static const std::string g_fileEnableSuc = APP_BASE_PATH"/demo_with_multi_lib/libs/arm64-v8a/libentry.so"; -static const std::string g_filesigEnablePath = TMP_BASE_PATH"/demo_with_multi_lib/libs/arm64-v8a/libentry.so.fsv-sig"; +// examples of Enforce code signature for file +static const std::string g_fileEnableSuc = APP_BASE_PATH + "/demo_with_multi_lib/libs/arm64-v8a/libentry.so"; +static const std::string g_filesigEnablePath = + TMP_BASE_PATH + "/demo_with_multi_lib/libs/arm64-v8a/libentry.so.fsv-sig"; -//wrong format file -static const std::string g_wrongFileEnableFail = APP_BASE_PATH"/demo_with_multi_lib_error/libs/arm64-v8a/libentry.so"; +// wrong format file +static const std::string g_wrongFileEnableFail = + APP_BASE_PATH + "/demo_with_multi_lib_error/libs/arm64-v8a/libentry.so"; static const std::vector g_fileSigEnableFailPath({ - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_01.so.fsv-sig", //ilegal pkcs7 format - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_02.so.fsv-sig", //Disable to find cert chain - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_03.so.fsv-sig", //Don't support digest algorithm - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_04.so.fsv-sig", //Don't support signature algorithm - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_05.so.fsv-sig", //Wrong signature - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_06.so.fsv-sig", //Expired signature - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_07.so.fsv-sig", //Cert chain validate fail + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_01.so.fsv-sig", // ilegal pkcs7 format + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_02.so.fsv-sig", // Disable to find cert chain + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_03.so.fsv-sig", // Don't support digest algorithm + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_04.so.fsv-sig", // Don't support signature algorithm + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_05.so.fsv-sig", // Wrong signature + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_06.so.fsv-sig", // Expired signature + TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_07.so.fsv-sig", // Cert chain validate fail }); -//examples of can't find the signature file +// examples of can't find the signature file static const EntryMap g_hapSigNotExist = { - {"sigNotExist", APP_BASE_PATH"/demo_without_lib/demo_without_lib.hap"}, + {"sigNotExist", APP_BASE_PATH + "/demo_without_lib/demo_without_lib.hap"}, }; class CodeSignUtilsTest : public testing::Test { @@ -128,8 +129,8 @@ static bool ReadSignatureFromFile(const std::string &path, ByteBuffer &data) return ret == fileSize; } -//excute the exceptional examples first, because of it's always successful -//once the same file signature verified successfully +// excute the exceptional examples first, because of it's always successful +// once the same file signature verified successfully /** * @tc.name: CodeSignUtilsTest_0001 @@ -139,8 +140,8 @@ static bool ReadSignatureFromFile(const std::string &path, ByteBuffer &data) */ HWTEST_F(CodeSignUtilsTest, CodeSignUtilsTest_0001, TestSize.Level0) { - int ret = CodeSignUtils::EnforceCodeSignForApp(g_hapWithoutLibRetSuc, - TMP_BASE_PATH"/demo_with_multi_lib/pkcs7_error/file/libentry_01.so.fsv-sig"); + std::string sigPath = TMP_BASE_PATH + "/demo_with_multi_lib/pkcs7_error/file/libentry_01.so.fsv-sig"; + int ret = CodeSignUtils::EnforceCodeSignForApp(g_hapWithoutLibRetSuc, sigPath); EXPECT_EQ(ret, CS_ERR_EXTRACT_FILES); } @@ -320,6 +321,6 @@ HWTEST_F(CodeSignUtilsTest, CodeSignUtilsTest_0012, TestSize.Level0) ret = CodeSignUtils::EnforceCodeSignForApp(g_hapWithMultiLibRetSuc, g_sigWithMultiLibRetSucPath); EXPECT_EQ(ret, CS_SUCCESS); } -} //namespace CodeSign -} //namespace Security -} //namespace OHOS \ No newline at end of file +} // namespace CodeSign +} // namespace Security +} // namespace OHOS \ No newline at end of file diff --git a/test/unittest/local_code_sign_test.cpp b/test/unittest/local_code_sign_test.cpp index 72cf94b..a6fa1a7 100644 --- a/test/unittest/local_code_sign_test.cpp +++ b/test/unittest/local_code_sign_test.cpp @@ -30,9 +30,8 @@ using namespace std; namespace OHOS { namespace Security { namespace CodeSign { - -#define AN_BASE_PATH "/data/local/ark-cache/tmp/" -static const std::string DEMO_AN_PATH = AN_BASE_PATH"demo.an"; +static const std::string AN_BASE_PATH = "/data/local/ark-cache/tmp/"; +static const std::string DEMO_AN_PATH = AN_BASE_PATH + "demo.an"; class LocalCodeSignTest : public testing::Test { public: @@ -116,6 +115,6 @@ HWTEST_F(LocalCodeSignTest, LocalCodeSignTest_0005, TestSize.Level0) NativeTokenReset(selfTokenId); EXPECT_EQ(ret, CS_ERR_FILE_PATH); } -} //namespace CodeSign -} //namespace Security -} //namespace OHOS +} // namespace CodeSign +} // namespace Security +} // namespace OHOS diff --git a/test/unittest/multi_thread_local_sign_test.cpp b/test/unittest/multi_thread_local_sign_test.cpp index 013d01b..673087a 100644 --- a/test/unittest/multi_thread_local_sign_test.cpp +++ b/test/unittest/multi_thread_local_sign_test.cpp @@ -116,6 +116,6 @@ HWMTEST_F(MultiThreadLocalSignTest, MultiThreadLocalSignTest_0001, TestSize.Leve { LocalCodeSignAndEnforce(); } -} //namespace CodeSign -} //namespace Security -} //namespace OHOS \ No newline at end of file +} // namespace CodeSign +} // namespace Security +} // namespace OHOS \ No newline at end of file diff --git a/test/unittest/sign_and_enforce_test.cpp b/test/unittest/sign_and_enforce_test.cpp index 926549f..aa28d78 100644 --- a/test/unittest/sign_and_enforce_test.cpp +++ b/test/unittest/sign_and_enforce_test.cpp @@ -33,17 +33,17 @@ using namespace std; namespace OHOS { namespace Security { namespace CodeSign { - -#define AN_BASE_PATH "/data/local/ark-cache/tmp/" - -static const std::string DEMO_AN_PATH = AN_BASE_PATH"demo.an"; -static const std::string DEMO_TAMPER_AN_PATH = AN_BASE_PATH"fake_demo.an"; +static const std::string AN_BASE_PATH = "/data/local/ark-cache/tmp/"; +static const std::string DEMO_AN_PATH = AN_BASE_PATH + "demo.an"; +static const std::string DEMO_TAMPER_AN_PATH = AN_BASE_PATH + "fake_demo.an"; static const char *g_validCaller = "installs"; static const std::string FAKE_SERIAL_NUMBER = "0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; static const std::string FAKE_CONTENT = "FAKE"; +static const int MAX_TEST_BUF_LEN = 1024; + static void ModifySignatureFormat(ByteBuffer &pkcs7Data) { uint8_t *data = pkcs7Data.GetBuffer(); @@ -54,12 +54,15 @@ static void ModifySignatureValue(PKCS7_SIGNER_INFO *p7info) { const uint8_t *data = ASN1_STRING_get0_data(p7info->enc_digest); int len = ASN1_STRING_length(p7info->enc_digest); + if (len <= 0 || len > MAX_TEST_BUF_LEN) { + return; + } uint8_t *fdata = static_cast(malloc(len)); if (fdata == nullptr) { return; } - (void) memcpy_s(fdata, len, data, len); - (void) memcpy_s(fdata, len, FAKE_CONTENT.c_str(), FAKE_CONTENT.length()); + (void)memcpy_s(fdata, len, data, len); + (void)memcpy_s(fdata, len, FAKE_CONTENT.c_str(), FAKE_CONTENT.length()); ASN1_STRING_set0(p7info->enc_digest, fdata, len); } @@ -237,6 +240,6 @@ HWTEST_F(SignAndEnforceTest, SignAndEnforceTest_0006, TestSize.Level0) int32_t ret = CodeSignUtils::EnforceCodeSignForFile(DEMO_AN_PATH, sig); EXPECT_EQ(ret, CS_SUCCESS); } -} //namespace CodeSign -} //namespace Security -} //namespace OHOS \ No newline at end of file +} // namespace CodeSign +} // namespace Security +} // namespace OHOS \ No newline at end of file diff --git a/utils/src/cert_utils.cpp b/utils/src/cert_utils.cpp index a40adae..b980e44 100644 --- a/utils/src/cert_utils.cpp +++ b/utils/src/cert_utils.cpp @@ -23,7 +23,6 @@ namespace OHOS { namespace Security { namespace CodeSign { - static const uint32_t CERT_DATA_SIZE = 8192; static const uint32_t CERT_COUNT = 4; diff --git a/utils/src/fsverity_utils_helper.cpp b/utils/src/fsverity_utils_helper.cpp index 1bdafff..108a59f 100644 --- a/utils/src/fsverity_utils_helper.cpp +++ b/utils/src/fsverity_utils_helper.cpp @@ -12,16 +12,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - +#include "fsverity_utils_helper.h" #include #include #include #include #include - #include "errcode.h" #include "file_helper.h" -#include "fsverity_utils_helper.h" #include "log.h" namespace OHOS { diff --git a/utils/src/signer_info.cpp b/utils/src/signer_info.cpp index a8d49ea..dc497b7 100644 --- a/utils/src/signer_info.cpp +++ b/utils/src/signer_info.cpp @@ -14,9 +14,6 @@ */ #include "signer_info.h" - -#include - #include "log.h" #include "openssl/asn1.h" #include "openssl/pem.h" @@ -142,6 +139,9 @@ bool SignerInfo::AddSignatureInSignerInfo(const ByteBuffer &signature) } uint32_t signatureSize = signature.GetSize(); // tmp will be free when freeing p7info_ + if (signatureSize == 0) { + return false; + } uint8_t *tmp = static_cast(malloc(signatureSize)); if (tmp == nullptr) { return false; -- Gitee