Azure / azure-iot-sdk-c

A C99 SDK for connecting devices to Microsoft Azure IoT services
https://azure.github.io/azure-iot-sdk-c
Other
588 stars 740 forks source link

Support openssl engine #2400

Closed JonathanKnam closed 1 year ago

JonathanKnam commented 1 year ago
# Checklist - [x] I have read the [contribution guidelines] (https://github.com/Azure/azure-iot-sdk-c/blob/main/.github/CONTRIBUTING.md). - [x] I added or modified the existing tests to cover the change (we do not allow our test coverage to go down). - If this is a modification that impacts the behavior of a public API - [ ] I edited the corresponding document in the `devdoc` folder and added or modified requirements. - I submitted this PR against the correct branch: - [x] This pull-request is submitted against the `main` branch. - [ ] I have merged the latest `main` branch prior to submission and re-merged as needed after I took any feedback. - [ ] I have squashed my changes into one with a clear description of the change. # Reference/Link to the issue solved with this PR (if any) # Description of the problem Normal communication via mqtt works find when using an OpenSSL engine. However when it compes to uploading files to the blob storage it failed, because OpenSSL engines have not been supported when using curl-library. # Description of the solution This solution builds up on my previous pull request, that was already merged (https://github.com/Azure/azure-c-shared-utility/pull/602) in the c-utilities submodule, and will now fully enable the support to upload files to the blob storage when using openssl engines together with curl.
CIPop commented 1 year ago

/azp run

azure-pipelines[bot] commented 1 year ago
Azure Pipelines successfully started running 3 pipeline(s).
JonathanKnam commented 1 year ago

So I see the error in the tests:

iothub_client/src/iothub_client_ll_uploadtoblob.c Func:IoTHubClient_LL_UploadToBlob_SetOption Line:1052 trying to set an openssl engine while the authentication scheme is not x509

and

iothub_client/src/iothub_client_ll_uploadtoblob.c Func:IoTHubClient_LL_UploadToBlob_SetOption Line:1020 trying to set a x509 private key type while the authentication scheme is not x509

Basically I could adjust the logic, to also accept these arguments (engine and private key) in case it uses no certificate based authentication (x509). Can probably be achieved by just reverting my second commit 090ade74.

Or actually I should probably adjust the tests, to set the required property beforehand.

JonathanKnam commented 1 year ago

/azp run

azure-pipelines[bot] commented 1 year ago
Commenter does not have sufficient privileges for PR 2400 in repo Azure/azure-iot-sdk-c
ericwolz commented 1 year ago

/azp run

azure-pipelines[bot] commented 1 year ago
Azure Pipelines successfully started running 3 pipeline(s).
ericwolz commented 1 year ago

The following tests FAILED: 61 - iothub_client_ll_u2b_ut (Failed) 62 - iothub_client_ll_u2b_ut_valgrind (Failed) 63 - iothub_client_ll_u2b_ut_helgrind (Failed) 64 - iothub_client_ll_u2b_ut_drd (Failed)

CIPop commented 1 year ago

I can repro this on my system: likely the Unit test expectations on expected API calls are incorrect.

The 3 new UTs are failing:

Executing test IoTHubClient_LL_UploadToBlob_SetOption_openssl_engine_type_succeeds ...
  Assert failed in line 1429  Expected: , Actual: [mallocAndStrcpy_s(0x7fffffffd7c0,"pkcs11")]
Test IoTHubClient_LL_UploadToBlob_SetOption_openssl_engine_type_succeeds result = !!! FAILED !!!

Executing test IoTHubClient_LL_UploadToBlob_SetOption_openssl_private_key_type_succeeds ...
  Assert failed in line 1410  Expected: , Actual: [gballoc_malloc(4)]
Test IoTHubClient_LL_UploadToBlob_SetOption_openssl_private_key_type_succeeds result = !!! FAILED !!!

Executing test IoTHubClient_LL_UploadToBlob_Destroy_handle_x509_succeeds ...
  Assert failed in line 992  Expected: , Actual: [gballoc_free(0x555555983470)][gballoc_free(0x55555597e240)]
Test IoTHubClient_LL_UploadToBlob_Destroy_handle_x509_succeeds result = !!! FAILED !!!

To repro, configure the following CMake options (I find it easier to use ccmake or cmake-gui):

 CMAKE_BUILD_TYPE                 Debug
 run_unittests                    ON

Then run

./build/iothub_client/tests/iothubclient_ll_u2b_ut/iothub_client_ll_u2b_ut_exe
CIPop commented 1 year ago

/azp run

azure-pipelines[bot] commented 1 year ago
Azure Pipelines successfully started running 3 pipeline(s).
JonathanKnam commented 1 year ago

@CIPop Thanks for merging, and also for the explanations about the missing parts in the unittests.