Add curl easy socket backend. #380

philljj · 2023-12-06T18:04:30Z

Description

Added libcurl easy socket backend support for testing purposes.

Built with:

./configure --enable-curl

Prereqs

wolfssl has been built with --enable-curl and installed to /usr/local.
libcurl has been built with --with-wolfssl and installed to /usr/local.

Testing

Added new curl easy socket example to examples/mqttnet.c, gated behind ENABLE_MQTT_CURL. If using TLS a CAfile can be passed in with -A:

$./examples/mqttclient/mqttclient -p 8883 -A mosquitto.org.crt -t
MQTT Client: QoS 0, Use TLS 1
MQTT Net Init: Success (0)
MQTT Init: Success (0)
*   Trying [2001:41d0:1:925e::1]:8883...
*   Trying 91.121.93.94:8883...
* Connected to test.mosquitto.org (2001:41d0:1:925e::1) port 8883
* successfully set certificate verify locations:
*  CAfile: mosquitto.org.crt
*  CApath: /etc/ssl/certs
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* using HTTP/1.x
* Connection #0 to host test.mosquitto.org left intact
MQTT Socket Connect: Success (0)
MQTT Connect: Proto (v3.1.1), Success (0)
MQTT Connect Ack: Return Code 0, Session Present 0
MQTT Subscribe: Success (0)
  Topic wolfMQTT/example/testTopic, Qos 0, Return Code 0
MQTT Publish: Topic wolfMQTT/example/testTopic, ID 2, Success (0)
MQTT Waiting for message...
MQTT Message: Topic wolfMQTT/example/testTopic, Qos 0, Len 4
Payload (0 - 4) printing 4 bytes:
test
MQTT Message: Done

$./examples/mqttclient/mqttclient -p 1883
MQTT Client: QoS 0, Use TLS 0
MQTT Net Init: Success (0)
MQTT Init: Success (0)
*   Trying [2001:41d0:1:925e::1]:1883...
* Connected to test.mosquitto.org (2001:41d0:1:925e::1) port 1883
* Connection #0 to host test.mosquitto.org left intact
MQTT Socket Connect: Success (0)
MQTT Connect: Proto (v3.1.1), Success (0)
MQTT Connect Ack: Return Code 0, Session Present 0
MQTT Subscribe: Success (0)
  Topic wolfMQTT/example/testTopic, Qos 0, Return Code 0
MQTT Publish: Topic wolfMQTT/example/testTopic, ID 2, Success (0)
MQTT Waiting for message...
MQTT Message: Topic wolfMQTT/example/testTopic, Qos 0, Len 4
Payload (0 - 4) printing 4 bytes:
test
MQTT Message: Done

Also test with:

./configure --enable-curl && make check

./configure --enable-mt --enable-nonblock CFLAGS="-DWOLFMQTT_TEST_NONBLOCK" --enable-curl  && make check

These tests are enabled and work with --enable-curl:

scripts/firmware.test
scripts/client.test

These tests work as well if they are enabled:

scripts/multithread.test
scripts/nbclient.test

dgarske · 2023-12-06T18:13:16Z

configure.ac

+
+if test "x$ENABLED_CURL" = "xyes"; then
+    if test "x$ENABLED_TLS" = "xyes"; then
+        AC_MSG_ERROR([--enable-tls and --enable-curl are incompatible])


Do these really need to be incompatible? Just seems odd to forcefully have to disable TLS here to get the CURL easy sockets to work.

In the last update --enable-curl and --enable-tls are allowed now. It disables some examples that aren't expected to work with curl. And when curl is enabled some of the lower level TLS code in src/mqtt_socket.c is gated out.

src/mqtt_curl.c

dgarske · 2023-12-06T18:15:32Z

src/mqtt_curl.c

+    #include <errno.h>
+#endif
+
+#include "wolfmqtt/mqtt_client.h"


We must support building sources directly. This while file needs wrapped with ENABLE_MQTT_CURL

Good catch!

examples/mqttcurl.c

dgarske · 2023-12-06T18:18:39Z

examples/mqttcurl.c

+
+    if (ctx->curl == NULL) {
+        PRINTF("error: curl_easy_init returned NULL");
+        return MQTT_CODE_ERROR_MEMORY;


Please refactor to single return point. Can't you leak handles returning early?

Done! Refactored the setopt and curl_easy_perform to a separate function, and the handle is cleaned up if that function fails.

examples/mqttexample.c

src/mqtt_curl.c

embhorn · 2023-12-06T20:55:23Z

src/mqtt_curl.c

+    }
+
+    return rc;
+}


Some compilers complain when there is no blank line at the eof.

src/mqtt_curl.c

configure.ac

embhorn

Could you add a section to the readme about using the libcurl option?

philljj · 2023-12-08T16:21:57Z

The client and firmware tests work with --enable-curl now, and the curl support includes TLS and mTLS.

Will add a github action now.

dgarske

Great work. Looking forward to the GitHub CI action being in place.

dgarske · 2023-12-08T16:35:02Z

examples/mqttnet.c

+
+    FD_SET(sockfd, &errfd);
+
+    if(for_recv) {


NIT: Please use if ( ... add space.

dgarske · 2023-12-08T16:35:27Z

examples/mqttnet.c

+        return MQTT_CODE_ERROR_BAD_ARG;
+    }
+
+    /* Toggle with option, or put behind debug define? */


Please wrap with DEBUG_WOLFMQTT

philljj · 2023-12-08T17:33:15Z

Added the curl ubuntu CI test in a separate workflow
.github/workflows/ubuntu-check-curl.yml
just to avoid potential side-effects between the with and without curl CI tests.

(The --enable-curl in wolfssl pulls in opensslextra and other things).

The new workflow tests these combinations:

--enable-curl
--enable-curl --disable-tls

dgarske · 2023-12-08T18:44:42Z

src/mqtt_socket.c

@@ -534,6 +544,10 @@ int MqttSocket_Disconnect(MqttClient *client)
            rc = client->net->disconnect(client->net->context);
        }
        MqttClient_Flags(client, MQTT_CLIENT_FLAG_IS_CONNECTED, 0);
+
+    #ifdef ENABLE_MQTT_CURL
+        curl_global_cleanup();


Should this now be in mqttnet.c or maybe MqttClient_DeInit?

I think mqttnet.c is too low level.

Would want it somewhere higher level where it's among the first/last functions called on init/exit.

mqtt_socket just seemed natural because that's where we call wolfSSL_Init/cleanup.

dgarske · 2023-12-08T18:45:08Z

src/mqtt_socket.c

@@ -34,6 +34,10 @@
    #include <errno.h>
 #endif

+#ifdef ENABLE_MQTT_CURL


Can likely be removed if curl_global_cleanup is moved

dgarske · 2023-12-08T19:11:49Z

examples/mqttnet.c

+mqttcurl_connect(SocketContext * sock, const char* host, word16 port,
+    int timeout_ms)
+{
+    CURLcode res = 0;


examples/mqttnet.c:435:14: error: cannot initialize a variable of type 'CURLcode' with an rvalue of type 'int' CURLcode res = 0; ^ ~ examples/mqttnet.c:647:21: error: cannot initialize a variable of type 'CURLcode' with an rvalue of type 'int' CURLcode res = 0; ^ ~ examples/mqttnet.c:714:21: error: cannot initialize a variable of type 'CURLcode' with an rvalue of type 'int' CURLcode res = 0; ^ ~

embhorn

./configure --enable-all --enable-curl produces build errors:

examples/mqttnet.c: In function ‘SN_ClientNet_Init’:
examples/mqttnet.c:1442:24: error: ‘SN_NetConnect’ undeclared (first use in this function); did you mean ‘NetConnect’?
 1442 |         net->connect = SN_NetConnect;
      |                        ^~~~~~~~~~~~~
      |                        NetConnect
examples/mqttnet.c:1442:24: note: each undeclared identifier is reported only once for each function it appears in
examples/mqttnet.c:1445:21: error: ‘NetPeek’ undeclared (first use in this function)
 1445 |         net->peek = NetPeek;
      |                     ^~~~~~~

philljj · 2023-12-08T21:24:30Z

I think several of these combinations need to be rejected in configure.ac. Looking at this.

embhorn

In examples/mqttclient/mqttclient, the client gets into the "Read loop" and fails to send messages typed. Also the client never sends a ping, just exits on timeout:

MQTT Waiting for message...
MQTT Message: Topic wolfMQTT/example/testTopic, Qos 0, Len 4
Payload (0 - 4) printing 4 bytes:
test
MQTT Message: Done
test
error: recvd 0 bytes, expected 2
Network Error Callback: Error (libcurl) (error -16)
MQTT Message Wait: Error (libcurl) (-16)
MQTT Disconnect: Success (0)
MQTT Socket Disconnect: Success (0)

philljj · 2023-12-08T22:44:04Z

Also, I'll document more clearly which --enable- options are compatible with --enable-curl.

… TIMEOUT, fix tests.

dgarske

@embhorn when you are happy merge please.

embhorn

Fantastic!

Add curl easy socket backend.

2b267f6

philljj self-assigned this Dec 6, 2023

dgarske requested changes Dec 6, 2023

View reviewed changes

dgarske requested a review from embhorn December 6, 2023 18:20

embhorn requested changes Dec 6, 2023

View reviewed changes

philljj added 3 commits December 6, 2023 22:59

Add curl easy socket backend: consolidate mqttcurl into mqttnet.

e25d2a6

Add curl easy socket backend: consolidate mqtt_curl into mqtt_socket.

da94327

Add curl easy socket backend: allow enable-curl with enable-tls.

23b50a0

philljj requested review from embhorn and dgarske December 7, 2023 20:19

embhorn reviewed Dec 8, 2023

View reviewed changes

configure.ac Show resolved Hide resolved

embhorn reviewed Dec 8, 2023

View reviewed changes

Add curl easy socket backend: fix tests, add curl readme.

6cdc376

dgarske requested changes Dec 8, 2023

View reviewed changes

Add curl easy socket backend: cleanup, add curl CI test.

d98607e

philljj requested review from embhorn and dgarske December 8, 2023 17:37

dgarske requested changes Dec 8, 2023

View reviewed changes

Add curl easy socket backend: fix warning, fix multithread test.

c329ef7

embhorn requested changes Dec 8, 2023

View reviewed changes

philljj added 2 commits December 10, 2023 14:41

Add curl easy socket backend: configure checks, handle STDIN_WAKE and…

ce679c9

… TIMEOUT, fix tests.

Add curl easy socket backend: update readme.

2945e1e

philljj requested review from dgarske and embhorn December 11, 2023 16:35

dgarske approved these changes Dec 11, 2023

View reviewed changes

embhorn approved these changes Dec 11, 2023

View reviewed changes

embhorn merged commit 15ba2f2 into wolfSSL:master Dec 11, 2023
10 checks passed

philljj deleted the add_curl_easy_sockets branch December 12, 2023 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add curl easy socket backend. #380

Add curl easy socket backend. #380

philljj commented Dec 6, 2023 •

edited

dgarske Dec 6, 2023

philljj Dec 7, 2023

dgarske Dec 6, 2023

philljj Dec 6, 2023

dgarske Dec 6, 2023

philljj Dec 7, 2023

embhorn Dec 6, 2023

embhorn left a comment

philljj commented Dec 8, 2023

dgarske left a comment

dgarske Dec 8, 2023

dgarske Dec 8, 2023

philljj commented Dec 8, 2023 •

edited

dgarske Dec 8, 2023

philljj Dec 8, 2023

philljj Dec 8, 2023

dgarske Dec 8, 2023

dgarske Dec 8, 2023

embhorn left a comment

philljj commented Dec 8, 2023

embhorn left a comment

philljj commented Dec 8, 2023

dgarske left a comment

embhorn left a comment

Add curl easy socket backend. #380

Add curl easy socket backend. #380

Conversation

philljj commented Dec 6, 2023 • edited

Description

Prereqs

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

embhorn left a comment

Choose a reason for hiding this comment

philljj commented Dec 8, 2023

dgarske left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philljj commented Dec 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

embhorn left a comment

Choose a reason for hiding this comment

philljj commented Dec 8, 2023

embhorn left a comment

Choose a reason for hiding this comment

philljj commented Dec 8, 2023

dgarske left a comment

Choose a reason for hiding this comment

embhorn left a comment

Choose a reason for hiding this comment

philljj commented Dec 6, 2023 •

edited

philljj commented Dec 8, 2023 •

edited