Example: Support certificates #40

trond-snekvik · 2025-07-21T15:03:42Z

Adds support for certificates in the example. Certificates can both be loaded by mcumgr or be embedded in the firmware.

sam-golioth

We recently removed support for hardcoded credentials in the Firmware SDK, and I don't think we should be adding them here. The logic behind removing them is:

It models improper design for users, who more often than we'd expect end up using code in production that we intend to only be available for samples, and then ask us for support
It creates additional code that we need to maintain and test, but isn't (shouldn't be) going into customer applications.
The delta between hardcoding credentials and loading them at runtime isn't that large - it's more of a mental barrier.
- It's certainly more complex to use certificates than PSKs, but I'd argue that most of the complexity is involved in generating the keys - uploading the certificate to the device is a process that can be clearly described in just a few easy to follow steps.

I believe that most of the pushback from devrel has been around installing go and compiling mcumgr, rather than actually running through the steps to load the cert. I mentioned this in another comment, but we can use smpmgr installed via pip instead.

sam-golioth · 2025-07-22T01:09:17Z

examples/ble_gatt/src/credentials.c

+    mbedtls_pk_context pk;
+    mbedtls_pk_init(&pk);
+    int err = mbedtls_pk_parse_key(&pk, private_key, size, NULL, 0, psa_rng_for_mbedtls, NULL);
+    if (err)


Suggested change

if (err)

if (err != 0)

Small, bikeshed-shaped hill to die on, but I actually don't want to do this change, and I'd rather type out a response than leave it unaddressed.

The overwhelming majority of error checks in this repo and the firmware SDK uses if (err), like Zephyr and Linux does. A similar ratio appears to be practiced for pointer checks, where we're using if (!ptr) more often than if (ptr == NULL), particularly after allocations.

I recognize that this is a rather enraging response to a trivial suggestion in a PR, but unless it is in the style guide, it's a personal preference, IMO. I prefer if (err) because it reads more like English and follows the same convention as Zephyr (and the Linux Kernel, which is what our style guide defers to). I'm aware of the CERT recommendation, but for the error and NULL checks, I think readability and sticking with a familiar convention is more important. The scenarios listed as potential regressions in the recommendation are arguably rather contrived and extremely unlikely to manifest when we have a sane error return value convention. I absolutely agree with explicit equality checks for non-standard values like enums though, and would definitely consider it problematic if we used an implicit comparison to check for the new GOLIOTH_SETTING_VALUE_TYPE_UNKNOWN, for example.

Fair enough - it's not a hill I want to die on 🙂

sam-golioth · 2025-07-22T01:09:40Z

examples/ble_gatt/src/credentials.c

+
+    psa_key_id_t key_id;
+    err = mbedtls_pk_import_into_psa(&pk, &attrs, &key_id);
+    if (err)


Suggested change

if (err)

if (err != 0)

sam-golioth · 2025-07-22T01:10:12Z

examples/ble_gatt/src/credentials.h

@@ -0,0 +1,19 @@
+#pragma once


Missing copyright/license header

sam-golioth · 2025-07-22T01:10:22Z

examples/ble_gatt/src/main.c

+
+    struct pouch_config config;
+    err = load_certificate(&config.certificate);
+    if (err)


Suggested change

if (err)

if (err != 0)

sam-golioth · 2025-07-22T01:10:42Z

examples/ble_gatt/src/credentials.c

@@ -0,0 +1,194 @@
+/*
+ * Copyright (c) 2025 Golioth, Inc.


Let's add the SPDX identifier as well

sam-golioth · 2025-07-22T01:31:46Z

examples/ble_gatt/src/main.c

Let's add a README for this application with instructions for loading credentials using SMP. I know there's been pushback against mcumgr because of the go dependency - we can recommend smpmgr from pip instead.

I tested smpmgr, but this issue prevents the file uploads from working, unfortunately. The certificate file is too large to be transferred in time for the implicit timeout, even when I set the timeout flag. Happy to add it in once that is resolved, as the installation and UX is really slick.

I was able to get it to work last night, but I did have to set the mtu ( ——mtu ) to something smaller than the default (I used 128 but I didn't test to see if something larger would work). I didn't root cause to see if it's the same issue (I just guessed there was some buffer getting overflowed) so I'm not 100% this works around the timeout issue, but worth a shot?

I keep getting errors on my 303 byte cert with all mtu sizes too, looks like :-/

Looks like that was a PEBCAK, it works for me now. I'll add a section on using smpmgr 👍

For future, we could try to wrap smpmgr into west command, choose proper MTU (if that needs to be set explicitly) and probably also integrate with our Python REST API to simplify provisioning. Just dropping the idea here.

the mtu ( ——mtu ) to something smaller than the default

@sam-golioth FYI that smpclient should print a warning if the server is not enabling "mcumgr parameters". That endpoint negotiates the MTU at initialization time and should be used wherever possible to avoid these headaches!

https://docs.zephyrproject.org/latest/kconfig.html#CONFIG_MCUMGR_GRP_OS_MCUMGR_PARAMS

https://jphutchins.github.io/smp/latest/os_management/#smp.os_management.MCUMgrParametersReadResponse

https://github.com/intercreate/smpclient/blob/dcace1aae61910ff25d8a674fcb8351a538f95b3/smpclient/__init__.py#L493-L507

MTU defaults may be a problem if the MTU is not negotiated: https://github.com/intercreate/smpclient/tree/main/smpclient/transport

@JPHutchins When I enable the mcumgr parameters (CONFIG_MCUMGR_GRP_OS_MCUMGR_PARAMS), the warning you mention goes away, but the file transfer still fails if I don't manually set the MTU. This is on smpmgr v0.13.1.

Without specifying the mtu:

(golioth) samfriedman ~/golioth/bluetooth-gateway/bluetooth-gateway [main] $ smpmgr --port /dev/cu.usbmodem0010502274171 file upload ../../certs/playground-2f69a2-0000000001.key.der /lfs1/credentials/key.der ⠋ Connecting to /dev/cu.usbmodem0010502274171... OK [11:43:58] ERROR Timeout (2.5s) waiting for request header=Header(op=<OP.WRITE: 2>, version=<Version.V2: 1>, __init__.py:170 flags=<Flag.UNUSED: 0>, length=172, group_id=<GroupId.FILE_MANAGEMENT: 8>, sequence=1, command_id=<FileManagement.FILE_DOWNLOAD_UPLOAD: 0>) version=<Version.V2: 1> sequence=1 smp_data=b"\n\x00\x00\xac\x00\x08\x01\x00\xa4clen\x18ycoff\x00ddataXy0w\x02\x01\x01\x04 D\xa0!\xe1\x12\\QM\n\x8c\xffO\x80\xf2D\xb9\xbb]\xc1\xa6\xc9\xeb\xc2\x8c\xb0\xd3Axb\xe2\xca> \xa0\n\x06\x08*\x86H\xce=\x03\x01\x07\xa1D\x03B\x00\x04\xfa\x9a/\x15,\xf5\x1e;\x88@\xee\xa2 \xf3\x91\x07'\x91\xa1b\xafJ{:\xee\xfc\x8aZ\x00\x1b\x98\xa0}2t\xbc\x92\xf3^J\xa1\xfe\x8e\xd8 i.j:\xc2~-+\x1cO@|\xdc\x08\x97\x86\x00;5\x97\xecdnamex\x19/lfs1/credentials/key.der" off=0 data=b"0w\x02\x01\x01\x04 D\xa0!\xe1\x12\\QM\n\x8c\xffO\x80\xf2D\xb9\xbb]\xc1\xa6\xc9\xeb\xc2\x8c\xb0\xd3Axb\xe2\xca> \xa0\n\x06\x08*\x86H\xce=\x03\x01\x07\xa1D\x03B\x00\x04\xfa\x9a/\x15,\xf5\x1e;\x88@\xee\xa2 \xf3\x91\x07'\x91\xa1b\xafJ{:\xee\xfc\x8aZ\x00\x1b\x98\xa0}2t\xbc\x92\xf3^J\xa1\xfe\x8e\xd8 i.j:\xc2~-+\x1cO@|\xdc\x08\x97\x86\x00;5\x97\xec" name='/lfs1/credentials/key.der' len=121 - __init__:170 ERROR Connection to device lost: TimeoutError - Timeout (2.5s) waiting for request file_management.py:133 header=Header(op=<OP.WRITE: 2>, version=<Version.V2: 1>, flags=<Flag.UNUSED: 0>, length=172, group_id=<GroupId.FILE_MANAGEMENT: 8>, sequence=1, command_id=<FileManagement.FILE_DOWNLOAD_UPLOAD: 0>) version=<Version.V2: 1> sequence=1 smp_data=b"\n\x00\x00\xac\x00\x08\x01\x00\xa4clen\x18ycoff\x00ddataXy0w\x02\x01\x01\ x04 D\xa0!\xe1\x12\\QM\n\x8c\xffO\x80\xf2D\xb9\xbb]\xc1\xa6\xc9\xeb\xc2\x8c\xb0\xd3Axb\x e2\xca>\xa0\n\x06\x08*\x86H\xce=\x03\x01\x07\xa1D\x03B\x00\x04\xfa\x9a/\x15,\xf5\x1e ;\x88@\xee\xa2\xf3\x91\x07'\x91\xa1b\xafJ{:\xee\xfc\x8aZ\x00\x1b\x98\xa0}2t\xbc\x92\ xf3^J\xa1\xfe\x8e\xd8i.j:\xc2~-+\x1cO@|\xdc\x08\x97\x86\x00;5\x97\xecdnamex\x19/lfs1 /credentials/key.der" off=0 data=b"0w\x02\x01\x01\x04 D\xa0!\xe1\x12\\QM\n\x8c\xffO\x80\xf2D\xb9\xbb]\xc1\xa6\xc9\xeb\xc2\x8c\xb0\xd3Axb\x e2\xca>\xa0\n\x06\x08*\x86H\xce=\x03\x01\x07\xa1D\x03B\x00\x04\xfa\x9a/\x15,\xf5\x1e ;\x88@\xee\xa2\xf3\x91\x07'\x91\xa1b\xafJ{:\xee\xfc\x8aZ\x00\x1b\x98\xa0}2t\xbc\x92\ xf3^J\xa1\xfe\x8e\xd8i.j:\xc2~-+\x1cO@|\xdc\x08\x97\x86\x00;5\x97\xec" name='/lfs1/credentials/key.der' len=121 - file_management:133 ../../certs/playground-2f69a2-0000000001.key.der ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0.0% • 0/121 bytes • ? • -:--:--

With a manually specified mtu:

(golioth) samfriedman ~/golioth/bluetooth-gateway/bluetooth-gateway [main] $ smpmgr --port /dev/cu.usbmodem0010502274171 --mtu 128 file upload ../../certs/playground-2f69a2-0000000001.crt.der /lfs1/credentials/crt.der ⠋ Connecting to /dev/cu.usbmodem0010502274171... OK ../../certs/playground-2f69a2-0000000001.crt.der ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100.0% • 352/352 bytes • 1.0 kB/s • 0:00:00

Years ago, it was possible to compile with incompatible configs. There was some investigation here, and in smpclient's integration tests: https://github.com/intercreate/smpclient/tree/main/dutfirmware

If you're using USB and can spare the RAM, I recommend a single buffer of 1024B for a nice balance of speed and size, about 30KBps. Like this config: https://github.com/intercreate/smpclient/blob/main/dutfirmware/usb_smp_dut_1024_1_1024.conf

At 115200 UART I think you only need a 128 or 256 bytes buffer to saturate the 9KBps transport.

trond-snekvik · 2025-07-22T10:34:28Z

I would like to keep the hardcoded credentials as an alternative here, but I think all my arguments for it have already been presented, so I don't think I'll be able to come up with some rebuttal that could change your mind 😄

I think my viewpoint here is that we only have "don't do this at home"-solutions to this problem at the moment, so we might as well give users the foot gun with the fewest amounts of steps required to get started.

trond-snekvik · 2025-07-22T12:43:14Z

Removed the hardcoded credentials and added a readme 👍

mniestroj · 2025-07-22T13:19:53Z

examples/ble_gatt/src/credentials.c

+{
+    free((void *) cert->der);
+}


Do we want to update size here and/or assign der to NULL? This usually helps with maintaining information whether data should be valid or not (either in runtime or during debugging).

mniestroj · 2025-07-22T13:25:07Z

examples/ble_gatt/src/main.c

For future, we could try to wrap smpmgr into west command, choose proper MTU (if that needs to be set explicitly) and probably also integrate with our Python REST API to simplify provisioning. Just dropping the idea here.

Adds support for certificates in the example. Certificates can be loaded by mcumgr, as in the certificate provisioning example in the SDK. Signed-off-by: Trond Snekvik <trond@golioth.io>

trond-snekvik · 2025-07-22T16:27:49Z

Letting @hasheddan merge this when ready

JPHutchins · 2025-08-09T01:17:31Z

@mniestroj

For future, we could try to wrap smpmgr into west command, choose proper MTU (if that needs to be set explicitly) and probably also integrate with our Python REST API to simplify provisioning. Just dropping the idea here.

Generally, MTU is negotiated at initialization time, but I think that it may only work correctly for the BLE transport since the serial (uart/usb/can/shell) transport layer does not have the same concept of "MTU" as BLE. The interim solution is this PR that defers to Zephyr's defaults. The defaults tend to be OK for a transport like UART or CAN, but they slow down USB quite a bit.

trond-snekvik requested review from hasheddan, mniestroj and sam-golioth July 21, 2025 15:03

sam-golioth requested changes Jul 22, 2025

View reviewed changes

trond-snekvik force-pushed the cert_examples branch from b2bfb04 to b26e8cb Compare July 22, 2025 11:42

trond-snekvik requested a review from sam-golioth July 22, 2025 11:42

mniestroj approved these changes Jul 22, 2025

View reviewed changes

Example: Support certificates

5f76851

Adds support for certificates in the example. Certificates can be loaded by mcumgr, as in the certificate provisioning example in the SDK. Signed-off-by: Trond Snekvik <trond@golioth.io>

trond-snekvik force-pushed the cert_examples branch from b26e8cb to 5f76851 Compare July 22, 2025 14:36

sam-golioth approved these changes Jul 22, 2025

View reviewed changes

hasheddan approved these changes Jul 22, 2025

View reviewed changes

hasheddan merged commit d4fd756 into main Jul 22, 2025
4 checks passed

hasheddan deleted the cert_examples branch July 22, 2025 17:34

JPHutchins mentioned this pull request Aug 9, 2025

fix: use smpclient upstream mtu defaults intercreate/smpmgr#56

Merged

Example: Support certificates #40

Example: Support certificates #40

Conversation

trond-snekvik commented Jul 21, 2025

Uh oh!

sam-golioth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JPHutchins Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JPHutchins Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trond-snekvik commented Jul 22, 2025

Uh oh!

trond-snekvik commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trond-snekvik commented Jul 22, 2025

Uh oh!

Uh oh!

JPHutchins commented Aug 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JPHutchins Aug 3, 2025 •

edited

Loading

JPHutchins Aug 7, 2025 •

edited

Loading