Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for mbedTLS 3.6.2 #651

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/cmake-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ jobs:
- args: "-DOC_DEBUG_ENABLED=ON -DOC_CLOUD_ENABLED=ON -DOC_COLLECTIONS_IF_CREATE_ENABLED=ON"
uses: ./.github/workflows/unit-test-with-cfg.yml
with:
build_args: -DOC_LOG_MAXIMUM_LOG_LEVEL=INFO -DOC_WKCORE_ENABLED=ON -DOC_SOFTWARE_UPDATE_ENABLED=ON -DOC_MNT_ENABLED=ON -DOC_DISCOVERY_RESOURCE_OBSERVABLE_ENABLED=ON -DOC_PUSH_ENABLED=ON -DPLGD_DEV_TIME_ENABLED=ON -DOC_ETAG_ENABLED=ON -DBUILD_MBEDTLS_FORCE_3_5_0=ON ${{ matrix.args }}
build_args: -DOC_LOG_MAXIMUM_LOG_LEVEL=INFO -DOC_WKCORE_ENABLED=ON -DOC_SOFTWARE_UPDATE_ENABLED=ON -DOC_MNT_ENABLED=ON -DOC_DISCOVERY_RESOURCE_OBSERVABLE_ENABLED=ON -DOC_PUSH_ENABLED=ON -DPLGD_DEV_TIME_ENABLED=ON -DOC_ETAG_ENABLED=ON -DBUILD_MBEDTLS_FORCE_3_5_0=ON -DBUILD_MBEDTLS_FORCE_3_6_2=OFF ${{ matrix.args }}
build_type: ${{ (github.event_name == 'workflow_dispatch' && inputs.build_type) || 'Debug' }}
clang: ${{ github.event_name == 'workflow_dispatch' && inputs.clang }}
coverage: false
Expand Down
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ON)
set(BUILD_EXAMPLE_APPLICATIONS ON CACHE BOOL "Build example applications.")
set(BUILD_MBEDTLS ON CACHE BOOL "Build Mbed TLS library. When set to OFF, the Mbed TLS library with the OCF patches has to be provided.")
set(BUILD_MBEDTLS_FORCE_3_5_0 OFF CACHE BOOL "Force v3.5.0 of the MbedTLS library to be used (by default v3.1.0 is used by master)")
set(BUILD_MBEDTLS_FORCE_3_6_2 ON CACHE BOOL "Force v3.6.2 of the MbedTLS library to be used (by default v3.5.0 is used by master)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ON ?

set(OC_INSTALL_MBEDTLS ON CACHE BOOL "Include Mbed TLS in installation")
Danielius1922 marked this conversation as resolved.
Show resolved Hide resolved
set(BUILD_TINYCBOR ON CACHE BOOL "Build TinyCBOR library. When set to OFF, the TinyCBOR library has to be provided.")
set(OC_INSTALL_TINYCBOR ON CACHE BOOL "Include TinyCBOR in installation")
Expand All @@ -51,6 +52,10 @@ if(NOT BUILD_MBEDTLS_FORCE_3_5_0)
message(WARNING "MbedTLS v3.1.0 is deprecated and support will be removed in a future release")
endif()

if(NOT BUILD_MBEDTLS_FORCE_3_6_2)
message(WARNING "MbedTLS v3.5.0 is deprecated and support will be removed in a future release")
endif()

set(OC_DYNAMIC_ALLOCATION_ENABLED ON CACHE BOOL "Enable dynamic memory allocation within the OCF stack and Mbed TLS.")
set(OC_SECURITY_ENABLED ON CACHE BOOL "Enable security.")
if (OC_SECURITY_ENABLED)
Expand Down
20 changes: 20 additions & 0 deletions api/unittest/tcptest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,23 @@
#include "api/oc_tcp_internal.h"
#include "messaging/coap/coap_internal.h"
#include "port/oc_allocator_internal.h"
#include "port/oc_random.h"
#include "tests/gtest/Endpoint.h"
#include "util/oc_features.h"

#ifdef OC_OSCORE
#include "messaging/coap/oscore_internal.h"
#endif /* OC_OSCORE */

#ifdef OC_SECURITY
#include "security/oc_entropy_internal.h"
#endif /* OC_SECURITY */

#include "gtest/gtest.h"

#ifdef OC_SECURITY
#include "mbedtls/build_info.h"
#include "mbedtls/ctr_drbg.h"
#include "mbedtls/ssl.h"
#endif /* OC_SECURITY */

Expand All @@ -56,10 +63,12 @@ class TCPMessage : public testing::Test {
#ifdef OC_HAS_FEATURE_ALLOCATOR_MUTEX
oc_allocator_mutex_init();
#endif /* OC_HAS_FEATURE_ALLOCATOR_MUTEX */
oc_random_init();
}

static void TearDownTestCase()
{
oc_random_destroy();
#ifdef OC_HAS_FEATURE_ALLOCATOR_MUTEX
oc_allocator_mutex_destroy();
#endif /* OC_HAS_FEATURE_ALLOCATOR_MUTEX */
Expand Down Expand Up @@ -267,6 +276,15 @@ TEST_F(TCPMessage, GetTotalLength)
mbedtls_ssl_conf_min_version(&conf, MBEDTLS_SSL_MAJOR_VERSION_3,
MBEDTLS_SSL_MINOR_VERSION_3);
#endif /* MBEDTLS_VERSION_NUMBER <= 0x03010000 */
mbedtls_ctr_drbg_context ctr_drbg;
mbedtls_ctr_drbg_init(&ctr_drbg);
mbedtls_entropy_context entropy_ctx;
mbedtls_entropy_init(&entropy_ctx);
oc_entropy_add_source(&entropy_ctx);
std::vector<unsigned char> pers = { 't', 'e', 's', 't', '\0' };
ASSERT_EQ(0, mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func,
&entropy_ctx, pers.data(), pers.size()));
mbedtls_ssl_conf_rng(&conf, mbedtls_ctr_drbg_random, &ctr_drbg);
Comment on lines +279 to +287
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for entropy seeding.

The entropy seeding operation could fail, but the error code is not checked.

Apply this diff to add error handling:

  mbedtls_ctr_drbg_init(&ctr_drbg);
  mbedtls_entropy_context entropy_ctx;
  mbedtls_entropy_init(&entropy_ctx);
  oc_entropy_add_source(&entropy_ctx);
  std::vector<unsigned char> pers = { 't', 'e', 's', 't', '\0' };
- ASSERT_EQ(0, mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func,
-                                     &entropy_ctx, pers.data(), pers.size()));
+ int ret = mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func,
+                                 &entropy_ctx, pers.data(), pers.size());
+ ASSERT_EQ(0, ret) << "CTR-DRBG seeding failed with error: " << ret;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mbedtls_ctr_drbg_context ctr_drbg;
mbedtls_ctr_drbg_init(&ctr_drbg);
mbedtls_entropy_context entropy_ctx;
mbedtls_entropy_init(&entropy_ctx);
oc_entropy_add_source(&entropy_ctx);
std::vector<unsigned char> pers = { 't', 'e', 's', 't', '\0' };
ASSERT_EQ(0, mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func,
&entropy_ctx, pers.data(), pers.size()));
mbedtls_ssl_conf_rng(&conf, mbedtls_ctr_drbg_random, &ctr_drbg);
mbedtls_ctr_drbg_context ctr_drbg;
mbedtls_ctr_drbg_init(&ctr_drbg);
mbedtls_entropy_context entropy_ctx;
mbedtls_entropy_init(&entropy_ctx);
oc_entropy_add_source(&entropy_ctx);
std::vector<unsigned char> pers = { 't', 'e', 's', 't', '\0' };
int ret = mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func,
&entropy_ctx, pers.data(), pers.size());
ASSERT_EQ(0, ret) << "CTR-DRBG seeding failed with error: " << ret;
mbedtls_ssl_conf_rng(&conf, mbedtls_ctr_drbg_random, &ctr_drbg);

mbedtls_ssl_context ssl;
mbedtls_ssl_init(&ssl);
ASSERT_EQ(0, mbedtls_ssl_setup(&ssl, &conf));
Expand Down Expand Up @@ -296,6 +314,8 @@ TEST_F(TCPMessage, GetTotalLength)

oc_message_unref(message);
mbedtls_ssl_free(&ssl);
mbedtls_entropy_free(&entropy_ctx);
mbedtls_ctr_drbg_free(&ctr_drbg);
mbedtls_ssl_config_free(&conf);

#define SSL_MAJOR_VERSION_3 (3)
Expand Down
46 changes: 41 additions & 5 deletions deps/mbedtls-patch.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,24 @@ if(EXISTS "${MBEDTLS_SRC_DIR}/.git")
message("mbedtls cleaned")
endif()

if(BUILD_MBEDTLS_FORCE_3_6_2)
execute_process(
COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} rev-parse --is-shallow-repository
RESULT_VARIABLE IS_SHALLOW
OUTPUT_QUIET
)
if(IS_SHALLOW EQUAL 0)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} fetch --unshallow --tags)
else()
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} fetch --tags)
endif()
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} checkout v3.6.2)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} submodule update --init)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${IOTIVITY_SRC_DIR} add -u deps/mbedtls)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${IOTIVITY_SRC_DIR} submodule update --init)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${IOTIVITY_SRC_DIR} reset HEAD deps/mbedtls)
else()

if(BUILD_MBEDTLS_FORCE_3_5_0)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} fetch --unshallow)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} checkout v3.5.0)
Expand All @@ -33,14 +51,27 @@ else()
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${IOTIVITY_SRC_DIR} submodule update --init)
endif()

endif()

message("submodules initialised")

if(BUILD_MBEDTLS_FORCE_3_5_0)
file(GLOB PATCHES_COMMON "${IOTIVITY_PATCH_DIR}/mbedtls/3.5/*.patch")
file(GLOB PATCHES_CMAKE "${IOTIVITY_PATCH_DIR}/mbedtls/3.5/cmake/*.patch")
if(BUILD_MBEDTLS_FORCE_3_6_2)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} branch -D feature/iotivity-lite/v3.6.2 ERROR_QUIET)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} checkout -b feature/iotivity-lite/v3.6.2)
file(GLOB PATCHES_COMMON "${IOTIVITY_PATCH_DIR}/mbedtls/3.6/*.patch")
file(GLOB PATCHES_CMAKE "${IOTIVITY_PATCH_DIR}/mbedtls/3.6/cmake/*.patch")
else()
file(GLOB PATCHES_COMMON "${IOTIVITY_PATCH_DIR}/mbedtls/3.1/*.patch")
file(GLOB PATCHES_CMAKE "${IOTIVITY_PATCH_DIR}/mbedtls/3.1/cmake/*.patch")
if(BUILD_MBEDTLS_FORCE_3_5_0)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} branch -D feature/iotivity-lite/v3.5.0 ERROR_QUIET)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} checkout -b feature/iotivity-lite/v3.5.0)
file(GLOB PATCHES_COMMON "${IOTIVITY_PATCH_DIR}/mbedtls/3.5/*.patch")
file(GLOB PATCHES_CMAKE "${IOTIVITY_PATCH_DIR}/mbedtls/3.5/cmake/*.patch")
else()
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} branch -D feature/iotivity-lite/v3.1.0 ERROR_QUIET)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} checkout -b feature/iotivity-lite/v3.1.0)
file(GLOB PATCHES_COMMON "${IOTIVITY_PATCH_DIR}/mbedtls/3.1/*.patch")
file(GLOB PATCHES_CMAKE "${IOTIVITY_PATCH_DIR}/mbedtls/3.1/cmake/*.patch")
endif()
endif()

foreach(PATCH IN LISTS PATCHES_COMMON PATCHES_CMAKE)
Expand All @@ -51,6 +82,11 @@ foreach(PATCH IN LISTS PATCHES_COMMON PATCHES_CMAKE)
)
endforeach()

execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} add -u)
if(BUILD_MBEDTLS_FORCE_3_6_2 OR BUILD_MBEDTLS_FORCE_3_5_0)
execute_process(COMMAND ${GIT_EXECUTABLE} -C ${MBEDTLS_SRC_DIR} add include/mbedtls/mbedtls_oc_platform-standalone.h.in include/mbedtls/mbedtls_oc_platform.h.in)
endif()

set(MBEDTLS_INCLUDE_DIR "${IOTIVITY_SRC_DIR}/deps/mbedtls/include/mbedtls")

if(ENABLE_TESTING OR ENABLE_PROGRAMS)
Expand Down
2 changes: 1 addition & 1 deletion deps/mbedtls.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ foreach(target ${mbedtls_targets})
target_compile_definitions(${target} PRIVATE ${MBEDTLS_COMPILE_DEFINITIONS})

if(OC_COMPILER_IS_GCC OR OC_COMPILER_IS_CLANG)
if(NOT BUILD_MBEDTLS_FORCE_3_5_0)
if((NOT BUILD_MBEDTLS_FORCE_3_5_0) AND (NOT BUILD_MBEDTLS_FORCE_3_6_2))
target_compile_options(${target} PRIVATE -Wno-error=unused)
endif()
Danielius1922 marked this conversation as resolved.
Show resolved Hide resolved
endif()
Expand Down
35 changes: 24 additions & 11 deletions patches/mbedtls/3.5/01-ocf-anon-psk.patch
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h
index c7aae0ff8..a044543af 100644
index c7aae0ff87..a044543af6 100644
--- a/include/mbedtls/asn1.h
+++ b/include/mbedtls/asn1.h
@@ -644,10 +644,10 @@ void mbedtls_asn1_free_named_data_list_shallow(mbedtls_asn1_named_data *name);
Expand All @@ -16,7 +16,7 @@ index c7aae0ff8..a044543af 100644
-
#endif /* asn1.h */
diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h
index e18e9a5fc..6e3b06adf 100644
index e18e9a5fc6..6e3b06adff 100644
--- a/include/mbedtls/check_config.h
+++ b/include/mbedtls/check_config.h
@@ -430,6 +430,11 @@
Expand All @@ -32,7 +32,7 @@ index e18e9a5fc..6e3b06adf 100644
( !defined(MBEDTLS_CAN_ECDH) || \
!defined(MBEDTLS_PK_CAN_ECDSA_SIGN) || \
diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index debb1cc2c..d97d4e277 100644
index debb1cc2c1..d97d4e2770 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -645,7 +645,8 @@ union mbedtls_ssl_premaster_secret {
Expand Down Expand Up @@ -133,7 +133,7 @@ index debb1cc2c..d97d4e277 100644

#if defined(MBEDTLS_SSL_HANDSHAKE_WITH_PSK_ENABLED)
diff --git a/include/mbedtls/ssl_ciphersuites.h b/include/mbedtls/ssl_ciphersuites.h
index 07f2facef..64b74717f 100644
index 07f2facef5..64b74717f1 100644
--- a/include/mbedtls/ssl_ciphersuites.h
+++ b/include/mbedtls/ssl_ciphersuites.h
@@ -137,6 +137,8 @@ extern "C" {
Expand Down Expand Up @@ -188,8 +188,21 @@ index 07f2facef..64b74717f 100644
return 1;

default:
diff --git a/library/common.h b/library/common.h
index 3c472c685d..3690a063b1 100644
--- a/library/common.h
+++ b/library/common.h
@@ -177,7 +177,7 @@ static inline const unsigned char *mbedtls_buffer_offset_const(
* \param b Pointer to input (buffer of at least \p n bytes)
* \param n Number of bytes to process.
*/
-inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned char *b, size_t n)
+static inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned char *b, size_t n)
{
size_t i = 0;
#if defined(MBEDTLS_EFFICIENT_UNALIGNED_ACCESS)
diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c
index fdd753d1c..ec9e9e94d 100644
index fdd753d1cd..ec9e9e94db 100644
--- a/library/ctr_drbg.c
+++ b/library/ctr_drbg.c
@@ -160,7 +160,7 @@ static int block_cipher_df(unsigned char *output,
Expand All @@ -202,7 +215,7 @@ index fdd753d1c..ec9e9e94d 100644

if ((ret = mbedtls_aes_setkey_enc(&aes_ctx, key,
diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c
index 736b1423b..e69dea0e3 100644
index 736b1423be..e69dea0e30 100644
--- a/library/ssl_ciphersuites.c
+++ b/library/ssl_ciphersuites.c
@@ -111,6 +111,7 @@ static const int ciphersuite_preference[] =
Expand Down Expand Up @@ -242,7 +255,7 @@ index 736b1423b..e69dea0e3 100644

default:
diff --git a/library/ssl_misc.h b/library/ssl_misc.h
index a99bb3343..648041d9d 100644
index a99bb33439..648041d9d9 100644
--- a/library/ssl_misc.h
+++ b/library/ssl_misc.h
@@ -1666,6 +1666,8 @@ MBEDTLS_CHECK_RETURN_CRITICAL
Expand All @@ -255,7 +268,7 @@ index a99bb3343..648041d9d 100644
#endif /* MBEDTLS_X509_CRT_PARSE_C */

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index fc3fb85d7..cc0ff9005 100644
index fc3fb85d75..cc0ff9005a 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1862,6 +1862,39 @@ int mbedtls_ssl_conf_own_cert(mbedtls_ssl_config *conf,
Expand Down Expand Up @@ -357,7 +370,7 @@ index fc3fb85d7..cc0ff9005 100644
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)"));
if (ret == 0) {
diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c
index 27bbafa06..b84444124 100644
index 27bbafa06e..b844441241 100644
--- a/library/ssl_tls12_client.c
+++ b/library/ssl_tls12_client.c
@@ -1797,7 +1797,8 @@ static int ssl_parse_server_ecdh_params(mbedtls_ssl_context *ssl,
Expand Down Expand Up @@ -603,7 +616,7 @@ index 27bbafa06..b84444124 100644
defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED)
if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) {
diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c
index 6ebd5064f..c09485f68 100644
index 6ebd5064f6..c09485f680 100644
--- a/library/ssl_tls12_server.c
+++ b/library/ssl_tls12_server.c
@@ -768,7 +768,12 @@ static int ssl_pick_cert(mbedtls_ssl_context *ssl,
Expand Down Expand Up @@ -664,7 +677,7 @@ index 6ebd5064f..c09485f68 100644
if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK) {
if ((ret = ssl_parse_client_psk_identity(ssl, &p, end)) != 0) {
diff --git a/library/version_features.c b/library/version_features.c
index a89cef997..46a216097 100644
index a89cef997e..46a2160979 100644
--- a/library/version_features.c
+++ b/library/version_features.c
@@ -366,6 +366,9 @@ static const char * const features[] = {
Expand Down
Loading
Loading