Skip to content

Add *_SSLOptions - #1463

Open
djmott wants to merge 3 commits into
eclipse-paho:masterfrom
djmott:master
Open

Add *_SSLOptions#1463
djmott wants to merge 3 commits into
eclipse-paho:masterfrom
djmott:master

Conversation

@djmott

@djmott djmott commented Apr 1, 2024

Copy link
Copy Markdown

Add *_SSLOptions members for a callback and context to be called after SSL context creation and initialization, to allow further customization of the SSL context, if required.

djmott added 3 commits April 1, 2024 16:32
…r SSL context creation and initialization, to allow further customization of the SSL context, if required.
…case sensitive names, windows doesn't care. The library on ubuntu is librpcrt4.a
@djmott

djmott commented Apr 3, 2024

Copy link
Copy Markdown
Author

Can anyone provide insight into why the tests are failing?

@Pascal-Fremaux

Copy link
Copy Markdown

Hello, did you check the result of the unit tests ?
Your modification on the SSLOption structure make a lot of tests to fail.
No real idea why (we have a patch similar to yours, never pushed partially due to the potential impact on the unit tests that would need to be updated to the new structure capacity)

If I check the first test to fail
https://ci.appveyor.com/project/eclipsewebmaster/paho-mqtt-c/builds/49524234/job/olv19wvv6igh0f0f?fullLog=true
test3-1-ssl-conn-to-non-SSL-broker from https://github.com/eclipse/paho.mqtt.c/blob/6b1e202a701ffcdaa277b5644ed291287a70a7aa/test/test3.c#L625 (need to take the exact same version as for your code...)
Seems that your change, for unknown reason (need to dig into), make the connect() succeed where it was supposed to fail.

31: Assertion failed, file C:\projects\paho-mqtt-c\test\test3.c, line 626, description: Connect should fail
31: rc was -8 20240402 023115.539 failed: test test1. 4 tests run, 1 failures

I encourage you to push forward as it would be great to have this patch official as we have a similar need :)

@Pascal-Fremaux

Pascal-Fremaux commented Apr 17, 2024

Copy link
Copy Markdown

In fact you did not take into account the impact of the change of the struct version.

The client do check on that number and would reject your structure as the number is above the max known:
See the code in the clients:
Example:
https://github.com/eclipse/paho.mqtt.c/blob/6b1e202a701ffcdaa277b5644ed291287a70a7aa/src/MQTTClient.c#L1801

	if (strncmp(options->ssl->struct_id, "MQTS", 4) != 0 || options->ssl->struct_version < 0 || options->ssl->struct_version > 5)
	{
		rc.reasonCode = MQTTCLIENT_BAD_STRUCTURE;
		goto exit;
	}

(I still do not understand why your change make the above test succeed when it should fail... but the code part above explain that your modificaution cannot work like that: you change nothing related to the max version in your patch.

@Pascal-Fremaux

Pascal-Fremaux commented Apr 17, 2024

Copy link
Copy Markdown

Another question is the goal of the change: is it close to the needs reached by other ways with #1328 #1247 ?
Accessing to one engine and private key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants