libraries/SocketWrapper: Fix agrs and improve error handling.

- Fix socket timeout arg to use proper struct timeval
- Initialize addrinfo structs to prevent undefined behavior
- Add error checking for tls_credential_add() and setsockopt() calls
- Centralize socket cleanup in error path
- Change default return value to false for safer error handling
- Change cert args to const

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>

Author: iabdalkader

libraries/SocketWrapper/SocketWrapper.h

@@ -30,8 +30,8 @@ class ZephyrSocketWrapper {
 	bool connect(const char *host, uint16_t port) {
 
 		// Resolve address
-		struct addrinfo hints;
-		struct addrinfo *res;
+		struct addrinfo hints = {0};
+		struct addrinfo *res = nullptr;
 		bool rv = true;
 
 		hints.ai_family = AF_INET;
@@ -102,24 +102,27 @@ class ZephyrSocketWrapper {
 	}
 
 #if defined(CONFIG_NET_SOCKETS_SOCKOPT_TLS)
-	bool connectSSL(const char *host, uint16_t port, char *ca_certificate_pem = nullptr) {
+	bool connectSSL(const char *host, uint16_t port, const char *ca_certificate_pem = nullptr) {
 
 		// Resolve address
-		struct addrinfo hints;
-		struct addrinfo *res;
+		struct addrinfo hints = {0};
+		struct addrinfo *res = nullptr;
 
 		hints.ai_family = AF_INET;
 		hints.ai_socktype = SOCK_STREAM;
 
 		int resolve_attempts = 100;
 		int ret;
-		bool rv = true;
+		bool rv = false;
 
 		sec_tag_t sec_tag_opt[] = {
 			CA_CERTIFICATE_TAG,
 		};
 
-		uint32_t timeo_optval = 100;
+		struct timeval timeout_opt = {
+			.tv_sec = 0,
+			.tv_usec = 100000,
+		};
 
 		while (resolve_attempts--) {
 			ret = getaddrinfo(host, String(port).c_str(), &hints, &res);
@@ -132,33 +135,33 @@ class ZephyrSocketWrapper {
 		}
 
 		if (ret != 0) {
-			rv = false;
 			goto exit;
 		}
 
 		if (ca_certificate_pem != nullptr) {
 			ret = tls_credential_add(CA_CERTIFICATE_TAG, TLS_CREDENTIAL_CA_CERTIFICATE,
 									 ca_certificate_pem, strlen(ca_certificate_pem) + 1);
+			if (ret != 0) {
+				goto exit;
+			}
 		}
 
 		sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TLS_1_2);
 		if (sock_fd < 0) {
-			rv = false;
 			goto exit;
 		}
 
-		setsockopt(sock_fd, SOL_TLS, TLS_SEC_TAG_LIST, sec_tag_opt, sizeof(sec_tag_opt));
-
-		setsockopt(sock_fd, SOL_TLS, TLS_HOSTNAME, host, strlen(host));
-
-		setsockopt(sock_fd, SOL_SOCKET, SO_RCVTIMEO, &timeo_optval, sizeof(timeo_optval));
+		if (setsockopt(sock_fd, SOL_TLS, TLS_HOSTNAME, host, strlen(host)) ||
+			setsockopt(sock_fd, SOL_TLS, TLS_SEC_TAG_LIST, sec_tag_opt, sizeof(sec_tag_opt)) ||
+			setsockopt(sock_fd, SOL_SOCKET, SO_RCVTIMEO, &timeout_opt, sizeof(timeout_opt))) {
+			goto exit;
+		}
 
 		if (::connect(sock_fd, res->ai_addr, res->ai_addrlen) < 0) {
-			::close(sock_fd);
-			sock_fd = -1;
-			rv = false;
 			goto exit;
 		}
+
+		rv = true;
 		is_ssl = true;
 
 	exit:
@@ -167,6 +170,10 @@ class ZephyrSocketWrapper {
 			res = nullptr;
 		}
 
+		if (!rv && sock_fd >= 0) {
+			::close(sock_fd);
+			sock_fd = -1;
+		}
 		return rv;
 	}
 #endif

libraries/SocketWrapper/ZephyrClient.h

@@ -32,8 +32,8 @@ class ZephyrClient : public arduino::Client, ZephyrSocketWrapper {
 		return ret;
 	}
 #if defined(CONFIG_NET_SOCKETS_SOCKOPT_TLS)
-	int connectSSL(const char *host, uint16_t port, char *cert) {
-		auto ret = ZephyrSocketWrapper::connectSSL((char *)host, port, cert);
+	int connectSSL(const char *host, uint16_t port, const char *cert) {
+		auto ret = ZephyrSocketWrapper::connectSSL(host, port, cert);
 		if (ret) {
 			_connected = true;
 		}

libraries/SocketWrapper/ZephyrSSLClient.h

@@ -14,7 +14,7 @@ class ZephyrSSLClient : public ZephyrClient {
 		return connectSSL(host, port, nullptr);
 	}
 
-	int connect(const char *host, uint16_t port, char *cert) {
+	int connect(const char *host, uint16_t port, const char *cert) {
 		return connectSSL(host, port, cert);
 	}
 };