Skip to content

Conversation

@iabdalkader
Copy link

  • 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

@iabdalkader iabdalkader requested a review from pennam October 30, 2025 08:57
@per1234 per1234 added bug Something isn't working enhancement New feature or request labels Oct 30, 2025

Serial.println("Adding CA certificate...");
// Add the ISRG Root X1 CA certificate
int ret = tls_credential_add(CA_CERTIFICATE_TAG, TLS_CREDENTIAL_CA_CERTIFICATE,
Copy link

@pennam pennam Nov 5, 2025

Choose a reason for hiding this comment

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

I would add setCACert(const char *rootCA) to ZephyrSSLClient instead of using directly tls_credential_add

Copy link
Author

Choose a reason for hiding this comment

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

Actually, connectSSL(const char *host, uint16_t port, char *ca_certificate_pem = nullptr) already accepts a cert blob, I should just use that instead.

}

#if defined(CONFIG_NET_SOCKETS_SOCKOPT_TLS)
bool connectSSL(const char *host, uint16_t port, char *ca_certificate_pem = nullptr) {
Copy link

Choose a reason for hiding this comment

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

This should be const char *ca_certificate_pem together with

int connectSSL(const char *host, uint16_t port, const char *cert) in ZephyrClient.h and
int connect(const char *host, uint16_t port, const char *cert) in ZephyrSSLClient.h

or we need to cast the certificate blob

Copy link
Author

Choose a reason for hiding this comment

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

Fixed. Can you test it again?

- 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>
Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
@pennam
Copy link

pennam commented Nov 5, 2025

works on

PR coming to enable MBEDTLS on GIGA R1

@pennam pennam merged commit 6847a9c into arduino:main Nov 5, 2025
24 checks passed
@iabdalkader iabdalkader deleted the fix_sockets branch November 5, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants