drivers: wifi: simplelink: Implement setsockopt() for TLS offload #11522

GAnthony · 2018-11-20T01:34:44Z

Implements setsockopt() for the socket offload driver
to process the TLS tags sent in via the Zephyr setsockopt() API,
when CONFIG_NET_SOCKETS_SOCKOPT_TLS is chosen.
For each tag, the credential filenames are retrieved and
set via SimpleLink's sl_SetSockOpt() API.

Also, creates a new KConfig option for TLS_CREDENTIAL_FILENAMES.
This new option is used by apps/protocols to add TLS credentials
via filenames referring to the actual content stored on a secure file
system or flash.

Handles the IPPROTO_TLS_* socket protocol families in the
socket() offloaded API.

This was validated on the cc3220sf_launchxl with the http_get sockets
sample, with the globalsign_r2.der file loaded to secure flash via
the TI Uniflash tool, and using the TI Catalog of known good
root CA's.

codecov-io · 2018-11-20T02:16:46Z

Codecov Report

Merging #11522 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #11522     +/-   ##
=========================================
- Coverage   48.42%   48.33%   -0.1%     
=========================================
  Files         270      270             
  Lines       42131    42118     -13     
  Branches    10141    10140      -1     
=========================================
- Hits        20402    20357     -45     
- Misses      17647    17675     +28     
- Partials     4082     4086      +4

Impacted Files	Coverage Δ
subsys/logging/log_output.c	`63.59% <0%> (-8.74%)`	⬇️
subsys/logging/log_core.c	`70.85% <0%> (-4.69%)`	⬇️
misc/printk.c	`85.45% <0%> (-3.1%)`	⬇️
include/logging/log_msg.h	`89.09% <0%> (-1.82%)`	⬇️
include/logging/log_core.h	`100% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0916659...24e6017. Read the comment docs.

rlubos

Thank you for submitting this. Looks good overall, left some remarks to discuss on how to integrate the change into the sample.

rlubos · 2018-11-20T10:33:38Z

drivers/wifi/simplelink/simplelink_sockets.c

 			}
+			break;


/* if user wishes to have TCP_NODELAY = FALSE, * we return EINVAL and fail in the cases below.

Just from above, this will not happen if we break here, right?

Yes, look like the break is not wanted here. Thanks for catching.

rlubos · 2018-11-20T10:42:31Z

samples/net/sockets/http_get/src/http_get.c

@@ -25,8 +25,14 @@

 #if defined(CONFIG_NET_SOCKETS_SOCKOPT_TLS)
 #include <net/tls_credentials.h>
+#if defined(CONFIG_TLS_CREDENTIAL_FILENAMES)


IMO we could do all this discrimination in ca_certificate.h and just include it as it was. The sample code would not need to be modifed at all, as the only difference is on what we keep in the ca_certificate array.

True, that would appear less intrusive.

rlubos · 2018-11-20T10:43:18Z

samples/net/sockets/http_get/src/http_get.c

 #include "ca_certificate.h"
 #endif
+#define CA_CERTIFICATE_TAG 1
+static const unsigned int ca_cert_len = sizeof(ca_certificate);


Do we need to introduce this variable at all? In both cases we store there sizeof(ca_certificate).

Yup, not needed. Can restore to what it was.

dbkinder

Thanks for the doc update

dbkinder · 2018-11-20T21:32:01Z

boards/arm/cc3220sf_launchxl/doc/cc3220sf_launchxl.rst

+SimpleLink WiFi driver, by setting :option:`CONFIG_NET_SOCKETS_OFFLOAD`
+to ``y``.
+
+Secure socket (TLS) communication is handled as part of the socket APIs, and


Since this is a long sentence with multiple items to do, it would read better as a bullet list:

Secure socket (TLS) communication is handled as part of the socket APIs, and enabled by: - setting both :option:`CONFIG_NET_SOCKETS_SOCKOPT_TLS` and :option:`CONFIG_TLS_CREDENTIAL_FILENAMES` to ``y``, - using the TI Uniflash tool to program the required certificates and keys to the secure flash filesystem, and enabling the TI Trusted Root-Certificate Catalog.

OK, updated per request. Thanks.

GAnthony · 2018-11-21T01:52:33Z

recheck

rlubos

Thanks!

dbkinder · 2018-11-21T22:22:38Z

boards/arm/cc3220sf_launchxl/doc/cc3220sf_launchxl.rst

+
+Secure socket (TLS) communication is handled as part of the socket APIs,
+and enabled by:
+- setting both :option:`CONFIG_NET_SOCKETS_SOCKOPT_TLS`


needs to be a blank line before the first item of a bullet list.

Ok, so that's why shippable was failing. Thanks for pointing that out!
Rebased, and re-pushed.

Implements setsockopt() for the socket offload driver to process the TLS tags sent in via the Zephyr setsockopt() API, when CONFIG_NET_SOCKETS_SOCKOPT_TLS is chosen. For each tag, the credential filenames are retrieved and set via SimpleLink's sl_SetSockOpt() API. Also, creates a new KConfig option for TLS_CREDENTIAL_FILENAMES. This new option is used by apps/protocols to add TLS credentials via filenames referring to the actual content stored on a secure file system or flash. Handles the IPPROTO_TLS_* socket protocol families in the socket() offloaded API. This was validated on the cc3220sf_launchxl with the http_get sockets sample, with the globalsign_r2.der file loaded to secure flash via the TI Uniflash tool, and using the TI Catalog of known good root CA's. Signed-off-by: Gil Pitney <gil.pitney@linaro.org>

Update the http_get sockets sample to enable association of secure tags with certificate filenames. This allows certificates and keys to be provisioned to the file system on secure flash, without having to include the actual certificates in the application. Validated on the cc3220sf_launchxl board. Signed-off-by: Gil Pitney <gil.pitney@linaro.org>

Reference working sockets example http_get, and add reference to SimpleLink Certificate Handling. Signed-off-by: Gil Pitney <gil.pitney@linaro.org>

addressed

GAnthony requested review from dbkinder, jukkar, pfalcon and tbursztyka as code owners November 20, 2018 01:34

GAnthony requested a review from rlubos November 20, 2018 01:36

rlubos reviewed Nov 20, 2018

View reviewed changes

GAnthony force-pushed the secure_socket_offload branch from c6192ac to 90d394a Compare November 20, 2018 19:44

dbkinder suggested changes Nov 20, 2018

View reviewed changes

GAnthony force-pushed the secure_socket_offload branch 2 times, most recently from 6eba743 to 8d04bcc Compare November 20, 2018 23:06

rlubos approved these changes Nov 21, 2018

View reviewed changes

pfalcon mentioned this pull request Nov 21, 2018

[RFC] Add PF_CAN protocol for socket CAN #11454

Closed

dbkinder previously requested changes Nov 21, 2018

View reviewed changes

Gil Pitney added 3 commits November 21, 2018 15:35

doc: boards: cc3220sf_launchxl: Add notes on secure socket offload

24e6017

Reference working sockets example http_get, and add reference to SimpleLink Certificate Handling. Signed-off-by: Gil Pitney <gil.pitney@linaro.org>

GAnthony force-pushed the secure_socket_offload branch from 8d04bcc to 24e6017 Compare November 21, 2018 23:35

nashif merged commit 0d22202 into zephyrproject-rtos:master Nov 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: wifi: simplelink: Implement setsockopt() for TLS offload #11522

drivers: wifi: simplelink: Implement setsockopt() for TLS offload #11522

GAnthony commented Nov 20, 2018

codecov-io commented Nov 20, 2018 •

edited

Loading

rlubos left a comment

rlubos Nov 20, 2018

GAnthony Nov 20, 2018

rlubos Nov 20, 2018

GAnthony Nov 20, 2018

rlubos Nov 20, 2018

GAnthony Nov 20, 2018

dbkinder left a comment

dbkinder Nov 20, 2018

GAnthony Nov 20, 2018

GAnthony commented Nov 21, 2018

rlubos left a comment

dbkinder Nov 21, 2018

GAnthony Nov 21, 2018

drivers: wifi: simplelink: Implement setsockopt() for TLS offload #11522

drivers: wifi: simplelink: Implement setsockopt() for TLS offload #11522

Conversation

GAnthony commented Nov 20, 2018

codecov-io commented Nov 20, 2018 • edited Loading

Codecov Report

rlubos left a 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

Choose a reason for hiding this comment

dbkinder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GAnthony commented Nov 21, 2018

rlubos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Nov 20, 2018 •

edited

Loading