Create a new provisioning example #25

Patater · 2019-11-13T17:55:37Z

Teach how to manufacture a PSA Crypto device with provisioned keys through a provisioning example that could ostensibly be used in a device factory.

gilles-peskine-arm

I've reviewed the documentation so far. More precisely I've reviewed everything except main.c. I'll review main.c later.

gilles-peskine-arm · 2019-11-14T10:01:35Z

provisioning/README.md

@@ -0,0 +1,109 @@
+# Mbed Crypto provisioning example application


The name and the short description of the example are misleading. There are three ways you can provision a device onto a trust network:

Generate a key off-device, import it onto the device, and enter it into the device database.

Generate a key on-device, export it from the device, and enter it into the device database.

Already have a key on the device, export it and enter it into the device database.

(Above, “key” can refer to either the private key or its public key in different parts of the same sentence.)

This application covers both on-device generation and an already-existing key, but not import. This is a useful example, but the description does a bad job of setting the context.

The name is misleading because “provisioning” could apply to a key or to a device. Provisioning a key is not covered at all: that would mean importing a key, which the example does not do. Instead the example uses a key that has already been provisioned. The example performs a part of the process of provisioning a device, so it's misleading to call it just “provisioning”. This is an example of simple (challengeless) on-boarding.

The example could do importing, but our ATECC608A driver doesn't yet implement this so I had to remove that bit.

Another name we use for this sort of application has been "Factory Configuration". I rejected that name because there are multiple factories involved and it wasn't clear which factory was being referenced. We could use with more clear and consistent terminology here.

provisioning/README.md

provisioning/mbedtls_user_config.h

AndrzejKurek · 2019-11-15T11:52:51Z

provisioning/main.c

+#include "mbed_assert.h"
+
+/* The slot number for the device private key stored in the secure element by
+ * the secure element factory. Note: If, for example, your SE has a


Similar comment as Gilles' in a different place - at this stage noone would assume that if there's a pre-provisioned key in slot 0, the slot number cannot be 0.

That depends on the reader's background. If they're familiar with the PSA crypto API but not with the secure element interface, they might assume that 0 = invalid/unset, because that's usually true in the API. So it would be worth stating something like “Note that unlike most numerical values in the PSA Crypto API, 0 is a valid value for a slot number. Like many other secure elements, the ATECC608A numbers its slots from 0 to N-1 (and specifically for the ATECC608A, N=16).”

Stating that slot numbers are not key ids belongs in a separate sentence. It's completely unrelated to whether 0 is a valid value.

AndrzejKurek · 2019-11-15T11:56:20Z

provisioning/main.c

+
+    /* Set device private key attributes. */
+    /* Note: It is not necessary to hard-code the physical slot number within
+     * the secure element. The secure element driver can automatically allocate


I understood this sentence a bit differently than intended: hard-coding within the secure element sounds like embedding some kind of a number inside the secure element. Maybe it should be
hard-code the number of the physical secure element slot?

AndrzejKurek · 2019-11-15T12:00:19Z

provisioning/main.c

+    mbedtls_pk_free(&pk);
+}
+
+/* The secure element factory put a device private key pair (not attestation


These comments look more like a draft of reminders for you. What is their purpose?

Removed the note about the TLS application. Moved the comment to a better place and added a new one.

AndrzejKurek · 2019-11-15T12:01:26Z

provisioning/main.c

+    free(output);
+}
+
+void generate_csr(psa_key_id_t key_id)


This function also prints csr's. Maybe generate_print_csr?

gilles-peskine-arm

Reviewed main.c, completing my previous review.

gilles-peskine-arm · 2019-11-15T13:18:18Z

provisioning/main.c

+#include "mbed_assert.h"
+
+/* The slot number for the device private key stored in the secure element by
+ * the secure element factory. Note: If, for example, your SE has a


That depends on the reader's background. If they're familiar with the PSA crypto API but not with the secure element interface, they might assume that 0 = invalid/unset, because that's usually true in the API. So it would be worth stating something like “Note that unlike most numerical values in the PSA Crypto API, 0 is a valid value for a slot number. Like many other secure elements, the ATECC608A numbers its slots from 0 to N-1 (and specifically for the ATECC608A, N=16).”

Stating that slot numbers are not key ids belongs in a separate sentence. It's completely unrelated to whether 0 is a valid value.

provisioning/main.c

Patater

Rebased to address review feedback

provisioning/main.c

Patater · 2019-11-19T14:09:36Z

provisioning/main.c

+    mbedtls_pk_free(&pk);
+}
+
+/* The secure element factory put a device private key pair (not attestation


Removed the note about the TLS application. Moved the comment to a better place and added a new one.

Patater · 2019-11-20T10:53:06Z

Updated to address more comments and add some further refinements.

dgreen-arm

If we want to have this example tested in CI, we'll need a tests/provisioning.log file for the CI to compare against. Otherwise we can't test this automatically.

Add a device provisioning example that works with two different types of secure element keys: - Keys already stored in the secure element (in physical slot 0 in our example), placed there by the secure element vendor or another secure facility before coming to the assembly house. - Keys generated within the secure element during assembly (into physical slot 2 in our example) The provisiong example fulfils the need post-device-assembly to notify Mbed Crypto about which keys are present where, so that applications can use those keys. Mbed Crypto will remember this information across reboots, so after provisioning, another application can be flashed onto the device which can use these keys.

Automatically create and print a certificate signing request. This makes it convenient to create device certificates using never-left-the-device, PSA-managed private keys. Include instructions on how to create and verify a device certificate in the README.

It is a common source of failure that the provisioning data is overwritten when programming a new flash image. In a new troubleshooting section in the README, add a recommendation to upgrade the DAPLink version to a version that supports partial image updates.

Add the expected example output so that we can automatically test that the example is working as intended.

gilles-peskine-arm · 2019-11-21T11:52:56Z

provisioning/README.md

+Crypto. How does Mbed Crypto know the key exists? We can tell Mbed Crypto by
+running a factory device provisionining application, which is typically
+separate from the primary application and may be run as a final device assembly
+step.


After reading this paragraph, I understand that this document is going to show me how to use a pre-provisioned key. So when I reach the section “Generate a new key within a secure element”, I understand it as a step on the path to using a pre-provisioned key, which is puzzling because I don't see why you'd need to generate a key.

Please rewrite this paragraph not to be misleading.

gilles-peskine-arm · 2019-11-21T12:15:10Z

provisioning/README.md

+For registering keys already present within a secure element, Mbed Crypto
+provides a function to inform Mbed Crypto about the existence of the key:
+`mbedtls_psa_register_se_key()`. This function adds the necessary metadata to
+persistent storage so that the secure element keys can be used by an


While this sentence is fine on its own, it's a bit weird to transition from “key” singular to “keys” plural. Also the sentence would be easier to read in the active voice.

gilles-peskine-arm · 2019-11-21T12:17:37Z

provisioning/README.md

+
+With `psa_set_key_slot_number()`, you can specify which physical secure element
+slot the key is in. This function operates on a key attributes structure. Fill
+in any other necessary attributes and then call `mbedtls_psa_register_se_key()`


“any other necessary attributes” makes it seem that just setting the slot number can be enough, but it never is. Lifetime and key id are always necessary, usage policy most of the time. Type and size may or may not be necessary depending on whether the hardware already has this information (e.g. in the slot configuration). Either change it to “other necessary attributes” or list the attributes.

gilles-peskine-arm · 2019-11-21T12:19:41Z

provisioning/README.md

+number.
+
+For generated keys, unlike pre-existing secure element keys, calling
+`psa_set_key_slot_number()` is not required. The driver will select a valid and


It's not required in general, but it's required with this particular driver since this particular driver does not implement slot number allocation.

gilles-peskine-arm · 2019-11-21T12:30:59Z

provisioning/main.c

+ * secure element by this example provisioning application. */
+#define EXAMPLE_GENERATED_KEY_ID 18
+
+/* Mbed TLS needs a CSPRNG to generate a CSR. Provide it a callback which uses


Suggested change

/* Mbed TLS needs a CSPRNG to generate a CSR. Provide it a callback which uses

/* Mbed TLS needs a random generator to generate a CSR. Provide it a callback which uses

The random generator is used for a random challenge and for blinding. A PRNG would not work for either use.

gilles-peskine-arm · 2019-11-21T12:32:06Z

provisioning/main.c

+
+    /* Set device private key attributes. */
+    /* Note that it is not necessary to specify the physical slot number within
+     * the secure element. The secure element driver can automatically allocate


A driver can do it, but the driver we have now doesn't do it. So it isn't necessary in general, but it's necessary for this particular (version of this particular) driver.

gilles-peskine-arm · 2019-11-21T12:39:19Z

provisioning/main.c

+    /* We also want to generate our own key entirely within the secure element
+     * during the factory device provisioning stage in device assembly. Ask
+     * Mbed Crypto to generate this key for us. */
+    printf("\tGenerating keys within the secure element... ");


Not a blocker, but it's a bit weird that the message says “generating keys” but the function is generate_key singular. And the registration above uses plural both in the message and in the function name, but actually only registers one key (ok, technically a key pair, but technically a key pair is a single key object).

tests/provisioning.log

Patater added the enhancement New feature or request label Nov 13, 2019

gilles-peskine-arm self-requested a review November 14, 2019 09:30

AndrzejKurek self-requested a review November 14, 2019 09:37

gilles-peskine-arm suggested changes Nov 14, 2019

View reviewed changes

AndrzejKurek reviewed Nov 15, 2019

View reviewed changes

gilles-peskine-arm suggested changes Nov 15, 2019

View reviewed changes

Patater force-pushed the new-provisioning-example branch from 7404a73 to f6f7a5c Compare November 19, 2019 14:17

Patater commented Nov 19, 2019

View reviewed changes

Patater force-pushed the new-provisioning-example branch from f6f7a5c to abf44d8 Compare November 20, 2019 10:52

dgreen-arm suggested changes Nov 20, 2019

View reviewed changes

Patater added 3 commits November 20, 2019 12:53

Patater force-pushed the new-provisioning-example branch from abf44d8 to 3397325 Compare November 20, 2019 13:04

provisioning: Add expected example output

480daf0

Add the expected example output so that we can automatically test that the example is working as intended.

Patater force-pushed the new-provisioning-example branch from 3397325 to 480daf0 Compare November 20, 2019 13:47

gilles-peskine-arm self-requested a review November 21, 2019 10:21

gilles-peskine-arm added the needs: review label Nov 21, 2019

Patater requested a review from AndrzejKurek November 21, 2019 11:26

gilles-peskine-arm suggested changes Nov 21, 2019

View reviewed changes

danh-arm added needs: work and removed needs: review labels May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a new provisioning example #25

Create a new provisioning example #25

Patater commented Nov 13, 2019

gilles-peskine-arm left a comment

gilles-peskine-arm Nov 14, 2019

Patater Nov 14, 2019

Patater Nov 14, 2019

AndrzejKurek Nov 15, 2019

gilles-peskine-arm Nov 15, 2019

AndrzejKurek Nov 15, 2019

AndrzejKurek Nov 15, 2019

Patater Nov 19, 2019

AndrzejKurek Nov 15, 2019 •

edited

Loading

gilles-peskine-arm left a comment

gilles-peskine-arm Nov 15, 2019

Patater left a comment

Patater Nov 19, 2019

Patater commented Nov 20, 2019

dgreen-arm left a comment

gilles-peskine-arm Nov 21, 2019

gilles-peskine-arm Nov 21, 2019

gilles-peskine-arm Nov 21, 2019

gilles-peskine-arm Nov 21, 2019

gilles-peskine-arm Nov 21, 2019

gilles-peskine-arm Nov 21, 2019

gilles-peskine-arm Nov 21, 2019

		@@ -0,0 +1,109 @@
		# Mbed Crypto provisioning example application

	/* Mbed TLS needs a CSPRNG to generate a CSR. Provide it a callback which uses
	/* Mbed TLS needs a random generator to generate a CSR. Provide it a callback which uses

Create a new provisioning example #25

Are you sure you want to change the base?

Create a new provisioning example #25

Conversation

Patater commented Nov 13, 2019

gilles-peskine-arm 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrzejKurek Nov 15, 2019 • edited Loading

Choose a reason for hiding this comment

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Patater left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Patater commented Nov 20, 2019

dgreen-arm 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

Choose a reason for hiding this comment

AndrzejKurek Nov 15, 2019 •

edited

Loading