From 48a71ed753f7b8041f2aa27462518aaea8a8c166 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 8 Jun 2015 16:05:47 +0200 Subject: [PATCH 1/1] Update parameter list API [Problem] Param name range check is needed. Support for param overwriting is needed. Getters in CAPI are needed. IV param has to be added manually. [Solution] Add predefined range for possible ParamName values. Add ParamName value check. Support param overwriting. Add CAPI param getters. IV param is not generated in ckmc_generate_params. [Verification] Run ckm-tests --group=CKM_ALGO_PARAMS and ckm-tests-internal -t SERIALIZATION_TEST All should pass. Change-Id: I72a2c603d7a8f60bab5cb0c18fdc3866a28c7a82 --- src/include/ckm/ckm-type.h | 17 ++-- src/include/ckmc/ckmc-type.h | 93 ++++++++++++++++++---- .../client-async/client-manager-async-impl.cpp | 12 +-- src/manager/client-capi/ckmc-type.cpp | 70 ++++++++-------- src/manager/client/client-manager-impl.cpp | 12 +-- src/manager/common/algo-param.cpp | 7 +- src/manager/common/protocols.cpp | 4 +- src/manager/service/ckm-logic.cpp | 10 +-- tests/test_serialization.cpp | 20 ++--- 9 files changed, 161 insertions(+), 84 deletions(-) diff --git a/src/include/ckm/ckm-type.h b/src/include/ckm/ckm-type.h index a152e1d..281f351 100644 --- a/src/include/ckm/ckm-type.h +++ b/src/include/ckm/ckm-type.h @@ -129,6 +129,10 @@ enum class ParamName : int { // sign & verify SV_HASH_ALGO = 301, // hash algorithm (HashAlgorithm) SV_RSA_PADDING, // RSA padding (RSAPaddingAlgorithm) + + // special values marking valid values range + FIRST = ALGO_TYPE, + LAST = SV_RSA_PADDING }; // algorithm types (ALGO_TYPE param) @@ -152,9 +156,9 @@ public: template bool getParam(ParamName name, T& value) const; - // returns false if param 'name' already exists + // returns false if param 'name' is invalid template - bool addParam(ParamName name, const T& value); + bool setParam(ParamName name, const T& value); protected: class BaseParam { @@ -211,12 +215,15 @@ template <> bool CryptoAlgorithm::getParam(ParamName name, RawBuffer& value) const; template -bool CryptoAlgorithm::addParam(ParamName name, const T& value) { - return m_params.emplace(name, IntParam::create(static_cast(value))).second; +bool CryptoAlgorithm::setParam(ParamName name, const T& value) { + if (name < ParamName::FIRST || name > ParamName::LAST) + return false; + m_params[name] = IntParam::create(static_cast(value)); + return true; } template <> -bool CryptoAlgorithm::addParam(ParamName name, const RawBuffer& value); +bool CryptoAlgorithm::setParam(ParamName name, const RawBuffer& value); } // namespace CKM diff --git a/src/include/ckmc/ckmc-type.h b/src/include/ckmc/ckmc-type.h index 9a9669f..19504de 100644 --- a/src/include/ckmc/ckmc-type.h +++ b/src/include/ckmc/ckmc-type.h @@ -744,7 +744,7 @@ void ckmc_cert_list_all_free(ckmc_cert_list_s *first); * @remarks Caller is responsible for freeing it with ckmc_param_list_free * * @param[in] ppparam_list Double pointer to the list variable to which the newly created list will - * be assigned. Last element of the list has param = NULL; + * be assigned. * * @return @c 0 on success, otherwise a negative error value * @@ -766,11 +766,10 @@ int ckmc_param_list_new(ckmc_param_list_s **ppparams); * @since_tizen 3.0 * * @remarks Caller is responsible for ckmc_param_list_s creation. - * @remarks Last element of the list has param = NULL; * - * @param[in] previous Any element of the param list. - * @param[in] name Name of parameter to add. Each parameter name has an associated value type: - * integer or buffer. Passing a buffer parameter name will result in an error + * @param[in] params List of params created with ckcm_param_list_new. + * @param[in] name Name of parameter to add. Existing parameter will be overwritten. Passing + * invalid parameter name will result in an error. * @param[in] value Value of the parameter in form of a integer. * * @return @c 0 on success, otherwise a negative error value @@ -780,6 +779,8 @@ int ckmc_param_list_new(ckmc_param_list_s **ppparams); * * @see ckmc_param_list_new * @see ckmc_param_list_add_buffer + * @see ckmc_param_list_get_integer + * @see ckmc_param_list_get_buffer * @see ckmc_param_list_free * @see ckmc_generate_params * @see #ckmc_param_list_s @@ -795,11 +796,10 @@ int ckmc_param_list_add_integer(ckmc_param_list_s *params, * @since_tizen 3.0 * * @remarks Caller is responsible for ckmc_param_list_s creation. - * @remarks Last element of the list has param = NULL; * - * @param[in] previous Any element of the param list. - * @param[in] name Name of parameter to add. Each parameter name has an associated value type: - * integer or buffer. Passing an integer parameter name will result in an error + * @param[in] params List of params created with ckcm_param_list_new. + * @param[in] name Name of parameter to add. Existing parameter will be overwritten. Passing + * invalid parameter name will result in an error * @param[in] buffer Value of the parameter in form of a buffer. Caller is responsible for * creating and freeing the buffer. * @@ -810,6 +810,8 @@ int ckmc_param_list_add_integer(ckmc_param_list_s *params, * * @see ckmc_param_list_new * @see ckmc_param_list_add_integer + * @see ckmc_param_list_get_integer + * @see ckmc_param_list_get_buffer * @see ckmc_param_list_free * @see ckmc_generate_params * @see #ckmc_param_list_s @@ -820,6 +822,66 @@ int ckmc_param_list_add_buffer(ckmc_param_list_s *params, const ckmc_raw_buffer_s *buffer); /** + * @brief Gets integer parameter from the list. + * + * @since_tizen 3.0 + * + * @remarks Caller is responsible for ckmc_param_list_s creation. + * + * @param[in] params List of params created with ckcm_param_list_new. + * @param[in] name Name of parameter to get. + * @param[out] value Value of the parameter in form of a integer. + * + * @return @c 0 on success, otherwise a negative error value + * + * @retval #CKMC_ERROR_NONE Successful + * @retval #CKMC_ERROR_INVALID_PARAMETER Input parameter is invalid + * + * @see ckmc_param_list_new + * @see ckmc_param_list_add_integer + * @see ckmc_param_list_add_buffer + * @see ckmc_param_list_get_buffer + * @see ckmc_param_list_free + * @see ckmc_generate_params + * @see #ckmc_param_list_s + * @see #ckmc_param_name_e + */ + +int ckmc_param_list_get_integer(const ckmc_param_list_s *params, + ckmc_param_name_e name, + uint64_t* value); + +/** + * @brief Gets buffer parameter from the list. + * + * @since_tizen 3.0 + * + * @remarks Caller is responsible for ckmc_param_list_s creation. + * + * @param[in] params List of params created with ckcm_param_list_new. + * @param[in] name Name of parameter to get. + * @param[out] buffer Value of the parameter in form of a buffer. Caller is responsible for + * creating and freeing the buffer. + * + * @return @c 0 on success, otherwise a negative error value + * + * @retval #CKMC_ERROR_NONE Successful + * @retval #CKMC_ERROR_INVALID_PARAMETER Input parameter is invalid + * + * @see ckmc_param_list_new + * @see ckmc_param_list_add_integer + * @see ckmc_param_list_add_buffer + * @see ckmc_param_list_get_integer + * @see ckmc_param_list_free + * @see ckmc_generate_params + * @see #ckmc_param_list_s + * @see #ckmc_param_name_e + */ +int ckmc_param_list_get_buffer(const ckmc_param_list_s *params, + ckmc_param_name_e name, + ckmc_raw_buffer_s **buffer); + +/** * @brief Frees previously allocated list of algorithm params * * @since_tizen 3.0 @@ -829,10 +891,13 @@ int ckmc_param_list_add_buffer(ckmc_param_list_s *params, * @see ckmc_param_list_new * @see ckmc_param_list_add_integer * @see ckmc_param_list_add_buffer + * @see ckmc_param_list_get_integer + * @see ckmc_param_list_get_buffer * @see ckmc_generate_params * @see #ckmc_param_list_s * @see #ckmc_param_name_e */ + void ckmc_param_list_free(ckmc_param_list_s *params); /** @@ -841,12 +906,10 @@ void ckmc_param_list_free(ckmc_param_list_s *params); * @since_tizen 3.0 * * @remarks Caller is responsible for ckmc_param_list_s creation and destruction. - * @remarks Algorithm parameters used for encryption could be then used for decryption but this - * function should not be used for generating decryption parameters only. * @remarks Algorithm parameters are set to default values. Optional fields are left empty. - * Initialization vectors are randomly generated. Param list passed as ckmc_param_list_s - * will be extended with new params. Caller is responsible for freeing the list - * with ckmc_param_list_free. + * Initialization vectors are left empty (they have to be added manually). Existing params + * will be overwritten with default values. Caller is responsible for freeing the list with + * ckmc_param_list_free. * @remarks If the function returns error provided param list may contain some of default parameters * * @param[in] type Type of the algorithm @@ -861,6 +924,8 @@ void ckmc_param_list_free(ckmc_param_list_s *params); * @see ckmc_param_list_new * @see ckmc_param_list_add_integer * @see ckmc_param_list_add_buffer + * @see ckmc_param_list_get_integer + * @see ckmc_param_list_get_buffer * @see ckmc_param_list_free * @see #ckmc_param_list_s * @see #ckmc_param_name_e diff --git a/src/manager/client-async/client-manager-async-impl.cpp b/src/manager/client-async/client-manager-async-impl.cpp index 036fb8c..a04645e 100644 --- a/src/manager/client-async/client-manager-async-impl.cpp +++ b/src/manager/client-async/client-manager-async-impl.cpp @@ -314,20 +314,20 @@ void ManagerAsync::Impl::createKeyPair(const ManagerAsync::ObserverPtr& observer { case KeyType::KEY_RSA_PUBLIC: case KeyType::KEY_RSA_PRIVATE: - keyGenAlgorithm.addParam(ParamName::ALGO_TYPE, AlgoType::RSA_GEN); - keyGenAlgorithm.addParam(ParamName::GEN_KEY_LEN, additional_param); + keyGenAlgorithm.setParam(ParamName::ALGO_TYPE, AlgoType::RSA_GEN); + keyGenAlgorithm.setParam(ParamName::GEN_KEY_LEN, additional_param); break; case KeyType::KEY_DSA_PUBLIC: case KeyType::KEY_DSA_PRIVATE: - keyGenAlgorithm.addParam(ParamName::ALGO_TYPE, AlgoType::DSA_GEN); - keyGenAlgorithm.addParam(ParamName::GEN_KEY_LEN, additional_param); + keyGenAlgorithm.setParam(ParamName::ALGO_TYPE, AlgoType::DSA_GEN); + keyGenAlgorithm.setParam(ParamName::GEN_KEY_LEN, additional_param); break; case KeyType::KEY_ECDSA_PUBLIC: case KeyType::KEY_ECDSA_PRIVATE: - keyGenAlgorithm.addParam(ParamName::ALGO_TYPE, AlgoType::ECDSA_GEN); - keyGenAlgorithm.addParam(ParamName::GEN_EC, additional_param); + keyGenAlgorithm.setParam(ParamName::ALGO_TYPE, AlgoType::ECDSA_GEN); + keyGenAlgorithm.setParam(ParamName::GEN_EC, additional_param); break; default: diff --git a/src/manager/client-capi/ckmc-type.cpp b/src/manager/client-capi/ckmc-type.cpp index d665dc0..94fad5e 100644 --- a/src/manager/client-capi/ckmc-type.cpp +++ b/src/manager/client-capi/ckmc-type.cpp @@ -41,30 +41,6 @@ const size_t DEFAULT_IV_LEN = 16; const size_t DEFAULT_IV_LEN_BITS = 8*DEFAULT_IV_LEN; const size_t DEFAULT_KEY_LEN_BITS = 4096; -int _ckmc_random_buffer(ckmc_raw_buffer_s **buffer, size_t len) -{ - if(!buffer) - return CKMC_ERROR_INVALID_PARAMETER; - - char* data = static_cast(malloc(len*sizeof(char))); - if(!data) - return CKMC_ERROR_OUT_OF_MEMORY; - - std::ifstream is("/dev/urandom", std::ifstream::binary); - if(!is) { - free(data); - return CKMC_ERROR_FILE_SYSTEM; - } - - is.read(data, len); - if (static_cast(len) != is.gcount()) { - free(data); - return CKMC_ERROR_FILE_SYSTEM; - } - - return ckmc_buffer_new(reinterpret_cast(data), len, buffer); -} - int _ckmc_load_cert_from_x509(X509 *xCert, ckmc_cert_s **cert) { if(xCert == NULL) { @@ -613,7 +589,7 @@ int ckmc_param_list_add_integer(ckmc_param_list_s *params, return CKMC_ERROR_INVALID_PARAMETER; CKM::CryptoAlgorithm* algo = reinterpret_cast(params); - bool ret = algo->addParam(static_cast(name), value); + bool ret = algo->setParam(static_cast(name), value); return (ret ? CKMC_ERROR_NONE : CKMC_ERROR_INVALID_PARAMETER); } @@ -627,11 +603,42 @@ int ckmc_param_list_add_buffer(ckmc_param_list_s *params, CKM::CryptoAlgorithm* algo = reinterpret_cast(params); CKM::RawBuffer b(buffer->data, buffer->data + buffer->size); - bool ret = algo->addParam(static_cast(name), b); + bool ret = algo->setParam(static_cast(name), b); return (ret ? CKMC_ERROR_NONE : CKMC_ERROR_INVALID_PARAMETER); } KEY_MANAGER_CAPI +int ckmc_param_list_get_integer(const ckmc_param_list_s *params, + ckmc_param_name_e name, + uint64_t* value) +{ + if (!params || !value) + return CKMC_ERROR_INVALID_PARAMETER; + + const CKM::CryptoAlgorithm* algo = reinterpret_cast(params); + if (!algo->getParam(static_cast(name),*value)) + return CKMC_ERROR_INVALID_PARAMETER; + + return CKMC_ERROR_NONE; +} + +KEY_MANAGER_CAPI +int ckmc_param_list_get_buffer(const ckmc_param_list_s *params, + ckmc_param_name_e name, + ckmc_raw_buffer_s **buffer) +{ + if (!params || !buffer) + return CKMC_ERROR_INVALID_PARAMETER; + + const CKM::CryptoAlgorithm* algo = reinterpret_cast(params); + CKM::RawBuffer value; + if (!algo->getParam(static_cast(name),value)) + return CKMC_ERROR_INVALID_PARAMETER; + + return ckmc_buffer_new(value.data(), value.size(), buffer); +} + +KEY_MANAGER_CAPI void ckmc_param_list_free(ckmc_param_list_s *params) { CKM::CryptoAlgorithm* algo = reinterpret_cast(params); @@ -645,24 +652,17 @@ int ckmc_generate_params(ckmc_algo_type_e type, ckmc_param_list_s *params) if(params == NULL) return CKMC_ERROR_INVALID_PARAMETER; - ckmc_raw_buffer_s* buffer = NULL; int ret = CKMC_ERROR_NONE; switch(type) { case CKMC_ALGO_AES_CTR: ret = ckmc_param_list_add_integer(params, CKMC_PARAM_ED_CTR_LEN, DEFAULT_IV_LEN_BITS); - // no break on purpose + break; case CKMC_ALGO_AES_CBC: case CKMC_ALGO_AES_GCM: case CKMC_ALGO_AES_CFB: - if (ret == CKMC_ERROR_NONE) - ret = _ckmc_random_buffer(&buffer, DEFAULT_IV_LEN); - if (ret == CKMC_ERROR_NONE) - ret = ckmc_param_list_add_buffer(params, CKMC_PARAM_ED_IV, buffer); - else - ckmc_buffer_free(buffer); - break; case CKMC_ALGO_RSA_OAEP: + // no iv by default break; case CKMC_ALGO_RSA_SV: case CKMC_ALGO_DSA_SV: diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index e6a7f4b..76a5e57 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -510,20 +510,20 @@ int ManagerImpl::createKeyPair( { case KeyType::KEY_RSA_PUBLIC: case KeyType::KEY_RSA_PRIVATE: - keyGenAlgorithm.addParam(ParamName::ALGO_TYPE, AlgoType::RSA_GEN); - keyGenAlgorithm.addParam(ParamName::GEN_KEY_LEN, additional_param); + keyGenAlgorithm.setParam(ParamName::ALGO_TYPE, AlgoType::RSA_GEN); + keyGenAlgorithm.setParam(ParamName::GEN_KEY_LEN, additional_param); break; case KeyType::KEY_DSA_PUBLIC: case KeyType::KEY_DSA_PRIVATE: - keyGenAlgorithm.addParam(ParamName::ALGO_TYPE, AlgoType::DSA_GEN); - keyGenAlgorithm.addParam(ParamName::GEN_KEY_LEN, additional_param); + keyGenAlgorithm.setParam(ParamName::ALGO_TYPE, AlgoType::DSA_GEN); + keyGenAlgorithm.setParam(ParamName::GEN_KEY_LEN, additional_param); break; case KeyType::KEY_ECDSA_PUBLIC: case KeyType::KEY_ECDSA_PRIVATE: - keyGenAlgorithm.addParam(ParamName::ALGO_TYPE, AlgoType::ECDSA_GEN); - keyGenAlgorithm.addParam(ParamName::GEN_EC, additional_param); + keyGenAlgorithm.setParam(ParamName::ALGO_TYPE, AlgoType::ECDSA_GEN); + keyGenAlgorithm.setParam(ParamName::GEN_EC, additional_param); break; default: diff --git a/src/manager/common/algo-param.cpp b/src/manager/common/algo-param.cpp index 7846ce5..54799e9 100644 --- a/src/manager/common/algo-param.cpp +++ b/src/manager/common/algo-param.cpp @@ -59,9 +59,12 @@ bool CryptoAlgorithm::getParam(ParamName name, RawBuffer& value) const } template <> -bool CryptoAlgorithm::addParam(ParamName name, const RawBuffer& value) +bool CryptoAlgorithm::setParam(ParamName name, const RawBuffer& value) { - return m_params.emplace(name, BufferParam::create(value)).second; + if (name < ParamName::FIRST || name > ParamName::LAST) + return false; + m_params[name] = BufferParam::create(value); + return true; } } // namespace CKM diff --git a/src/manager/common/protocols.cpp b/src/manager/common/protocols.cpp index 2a5e6dd..91a3497 100644 --- a/src/manager/common/protocols.cpp +++ b/src/manager/common/protocols.cpp @@ -133,7 +133,7 @@ CryptoAlgorithmSerializable::CryptoAlgorithmSerializable(IStream &stream) case ParamName::ED_AAD: case ParamName::ED_LABEL: Deserializer::Deserialize(stream, buffer); - addParam(name, buffer); + setParam(name, buffer); break; case ParamName::ALGO_TYPE: @@ -144,7 +144,7 @@ CryptoAlgorithmSerializable::CryptoAlgorithmSerializable(IStream &stream) case ParamName::SV_HASH_ALGO: case ParamName::SV_RSA_PADDING: Deserializer::Deserialize(stream, integer); - addParam(name, integer); + setParam(name, integer); break; default: diff --git a/src/manager/service/ckm-logic.cpp b/src/manager/service/ckm-logic.cpp index 0154933..d37bc5b 100644 --- a/src/manager/service/ckm-logic.cpp +++ b/src/manager/service/ckm-logic.cpp @@ -1175,7 +1175,7 @@ int CKMLogic::createKeyAESHelper( const PolicySerializable &policy) { CryptoAlgorithm keyGenAlgorithm; - keyGenAlgorithm.addParam(ParamName::GEN_KEY_LEN, size); + keyGenAlgorithm.setParam(ParamName::GEN_KEY_LEN, size); Token key = m_decider.getStore(DataType::KEY_AES, policy.extractable).generateSKey(keyGenAlgorithm); return saveDataHelper(cred, @@ -1526,8 +1526,8 @@ RawBuffer CKMLogic::createSignature( DB::Row row; RawBuffer signature; CryptoAlgorithm cryptoAlg; - cryptoAlg.addParam(ParamName::SV_HASH_ALGO, hash); - cryptoAlg.addParam(ParamName::SV_RSA_PADDING, padding); + cryptoAlg.setParam(ParamName::SV_HASH_ALGO, hash); + cryptoAlg.setParam(ParamName::SV_RSA_PADDING, padding); int retCode = CKM_API_SUCCESS; @@ -1586,8 +1586,8 @@ RawBuffer CKMLogic::verifySignature( DB::Row row; CryptoAlgorithm params; - params.addParam(ParamName::SV_HASH_ALGO, hash); - params.addParam(ParamName::SV_RSA_PADDING, padding); + params.setParam(ParamName::SV_HASH_ALGO, hash); + params.setParam(ParamName::SV_RSA_PADDING, padding); // try certificate first - looking for a public key. // in case of PKCS, pub key from certificate will be found first diff --git a/tests/test_serialization.cpp b/tests/test_serialization.cpp index 882dc4d..653e5fd 100644 --- a/tests/test_serialization.cpp +++ b/tests/test_serialization.cpp @@ -72,9 +72,9 @@ void checkBufferParamNegative(const CryptoAlgorithm& algo, ParamName name) } template -void addParam(CryptoAlgorithm& algo, ParamName name, const T& value, bool success) +void setParam(CryptoAlgorithm& algo, ParamName name, const T& value, bool success) { - BOOST_REQUIRE_MESSAGE(success == algo.addParam(name, value), + BOOST_REQUIRE_MESSAGE(success == algo.setParam(name, value), "Adding param " << static_cast(name) << " should " << (success ? "succeed":"fail")); } @@ -85,11 +85,11 @@ BOOST_AUTO_TEST_SUITE(SERIALIZATION_TEST) BOOST_AUTO_TEST_CASE(Serialization_CryptoAlgorithm) { CryptoAlgorithm ca; - addParam(ca,ParamName::ALGO_TYPE, static_cast(AlgoType::AES_GCM), true); - addParam(ca,ParamName::ED_IV, IV, true); - addParam(ca,ParamName::ED_IV, AAD, false); // try to overwrite - addParam(ca,ParamName::ED_TAG_LEN, 128, true); - addParam(ca,ParamName::ED_AAD, AAD, true); + setParam(ca,ParamName::ALGO_TYPE, static_cast(AlgoType::AES_GCM), true); + setParam(ca,ParamName::ED_IV, AAD, true); + setParam(ca,ParamName::ED_IV, IV, true); // try to overwrite + setParam(ca,ParamName::ED_TAG_LEN, 128, true); + setParam(ca,ParamName::ED_AAD, AAD, true); CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; @@ -121,8 +121,10 @@ BOOST_AUTO_TEST_CASE(Serialization_CryptoAlgorithm) { BOOST_AUTO_TEST_CASE(Serialization_CryptoAlgorithm_wrong_name) { CryptoAlgorithm ca; - // unuspported param name - addParam(ca, static_cast(666), 666, true); + // param name out of range + setParam(ca, static_cast(666), 666, false); + // param name not supported by serializer + setParam(ca, static_cast(10), 666, true); CryptoAlgorithmSerializable input(ca); CryptoAlgorithmSerializable output; -- 2.7.4