Update TLS porting guide #17

yanesca · 2016-10-18T15:48:43Z

A new interface has been introduced in the hal and it is the new
preferred way to enable mbed TLS using the TRNG on a new platform.

This PR updates the porting guide to reflect this change.

A new interface has been introduced in the hal and it is the new preferred way to enable mbed TLS using the TRNG on a new platform. This PR updates the porting guide to reflect this change.

yanesca · 2016-10-18T15:50:36Z

@sbutcher-arm , @0xc0170 could you please review the section about the new TRNG hal?

simonbutcher

Sorry Janos, but I think this may take a couple of iterations.

simonbutcher · 2016-10-21T08:27:16Z

docs/advanced/tls_porting.md


 The next two sections explain how to do this.

-## How to implement the hardware poll
+## How to implement the TRNG api


Please capitalise API.

simonbutcher · 2016-11-04T11:24:19Z

docs/advanced/tls_porting.md


-The `mbedtls_hardware_poll` function is declared in `entropy_poll.h`:
+The implementation of this interface needs to be in the HAL.


What do you mean by 'in the HAL'? Do you mean it has to be in the HAL layer? Or in a specific directory?

simonbutcher · 2016-11-04T11:25:23Z

docs/advanced/tls_porting.md

@@ -24,46 +24,56 @@ mbed TLS distinguishes between strong and weak entropy sources. Of the sources r

 The preferred way to provide a custom entropy source:

-1. Implement the `mbedtls_hardware_poll` function to let it access the device's entropy source.
-2. Set the appropriate macros for your target in `hal/targets.json`.
+1. Implement the `trng_api.h` to let mbed TLS access the device's entropy source.


I would prefer something like 'Provide an implementation of the functions defined in ting_api.h' which I think is clearer.

simonbutcher · 2016-11-04T11:25:55Z

docs/advanced/tls_porting.md

-1. Implement the `mbedtls_hardware_poll` function to let it access the device's entropy source.
-2. Set the appropriate macros for your target in `hal/targets.json`.
+1. Implement the `trng_api.h` to let mbed TLS access the device's entropy source.
+2. Indicate in `hal/targets.json` that your target has an entropy source.


How do you indicate this?

simonbutcher · 2016-11-04T11:27:43Z

docs/advanced/tls_porting.md

+
+### Initialisation and deinitialisation
+
+Provide means for initialising and deinitialising the peripherial with the following functions:


Isn't that a fragment? Don't you need to say something like 'You need to provide means....'.

simonbutcher · 2016-11-04T11:32:14Z

docs/advanced/tls_porting.md

-You can read more about how to add a macro for your target [here](../mbed_OS/Targets.md).
-
-<span class="notes">**Note:** You should define the `MBEDTLS_NO_PLATFORM_ENTROPY` macro on any platform that does not support standards like /dev/urandom or Windows CryptoAPI.</span>
-
 ## 4. How to implement the Non-Volatile seed entropy source

 If a hardware platform does not have a hardware entropy source to leverage into the entropy pool, alternatives have to be considered. As said before, a strong entropy source is crucial for security of cryptographic and TLS operations. For platforms that support non-volatile memory, an option is to use the NV seed entropy source that is provided with mbed TLS.


Rather than 'As said before', better to say, 'As stated above' or something similar.

Can we make a statement here about the seed needing to be unique per device at manufacture? There's lots we can say about the seed, but that should be a minimum.

simonbutcher · 2016-11-04T11:33:42Z

docs/advanced/tls_porting.md

@@ -24,46 +24,56 @@ mbed TLS distinguishes between strong and weak entropy sources. Of the sources r



I think we need to define and characterise a TRNG. Say what kind of peripheral can use this API and what a TRNG is and some characteristics of it.

andresag01 · 2016-12-09T14:11:50Z

docs/advanced/tls_porting.md

@@ -1,6 +1,6 @@
 # mbed TLS Porting Guide

-This document explains how to port [mbed TLS](https://github.com/ARMmbed/mbedtls) to a new mbed target board. It includes an [example implementation](https://github.com/ARMmbed/mbed-tls-lib) for K64F boards.
+This document explains how to port [mbed TLS](https://github.com/ARMmbed/mbedtls) to a new mbed target board.


development board maybe?

andresag01 · 2016-12-09T14:17:13Z

docs/advanced/tls_porting.md


 The preferred way to provide a custom entropy source:

-1. Implement the `mbedtls_hardware_poll` function to let it access the device's entropy source.
-2. Set the appropriate macros for your target in `hal/targets.json`.
+1. Provide an implementation of the functions defined in `trng_api.h` to let mbed TLS access the device's entropy source.


Its a large project, please provide a more specific path for trng_api.h starting from the mbed-os root directory.

Style: Perhaps is better Implement the functions declared in the header file XXX.h ... instead of Provide an implementation of the functions defined in .... Also, in my mind, a function definition is the signature AND the implementation while declarations are just the signatures.

andresag01 · 2016-12-09T14:18:59Z

docs/advanced/tls_porting.md

-1. Implement the `mbedtls_hardware_poll` function to let it access the device's entropy source.
-2. Set the appropriate macros for your target in `hal/targets.json`.
+1. Provide an implementation of the functions defined in `trng_api.h` to let mbed TLS access the device's entropy source.
+2. Indicate that your target has an entropy source in `hal/targets.json`, by adding `TRNG` to your device's `device_has` section.


This information is not correct, the file hal/targets.json has been moved to targets/target.json

andresag01 · 2016-12-09T14:28:50Z

docs/advanced/tls_porting.md

-mbed TLS distinguishes between strong and weak entropy sources. Of the sources registered by default, two are strong: /dev/urandom and Windows CryptoAPI. However, these resources are not available on many embedded platforms, and the default behavior of mbed TLS is to refuse to work if there are no strong sources present. To get around this, mbed TLS assumes that any hardware entropy source you register (as explained in this guide) is strong. It is very important that you add a strong source if you add a hardware entropy source yourself. For example, an integrated circuit extracting statistically random data from two oscillators of unknown frequencies and independent phases is strong, while anything derived from a real time clock is weak.
+mbed TLS distinguishes between strong and weak entropy sources. Of the sources registered by default, two are strong: /dev/urandom and Windows CryptoAPI. However, these resources are not available on many embedded platforms, and the default behaviour of mbed TLS is to refuse to work if there are no strong sources present. To get around this, mbed TLS assumes that the hardware entropy source you register (as explained in this section) is a TRNG and thus treats it as strong.
+
+It is very important that you only add a TRNG the way described in this section. For the purposes of this document a device is considered a TRNG only if it is dedicated to generating entropy to be used in cryptographic applications, and careful consideration has been given to how much the data generated is subject to adversarial manipulation and to how much entropy it can actually provide.


Style: It is very important that you only add a TRNG the way described in this section -> It is very important that you only add a TRNG as described in this section

Style: This sentence is a little bit too long and is easy to get lost in the middle. This information is very important, could you please break this up into smaller, more manageable bits?

andresag01 · 2016-12-09T14:32:33Z

docs/advanced/tls_porting.md

+
+It is very important that you only add a TRNG the way described in this section. For the purposes of this document a device is considered a TRNG only if it is dedicated to generating entropy to be used in cryptographic applications, and careful consideration has been given to how much the data generated is subject to adversarial manipulation and to how much entropy it can actually provide.
+
+For example, an integrated circuit extracting statistically random data from two oscillators of unknown frequencies and independent phases is considered as a TRNG, while anything derived from a real time clock is NOT.


I think this and the previous paragraph are partly described what a TRNG is and what is expected of it, which partly satisfies @sbutcher-arm's request in one of the review comments. Since this information is quite important, I would suggest we expand it and make it more visible, perhaps by moving it closer to the top of the file, giving it a section title or otherwise...

andresag01 · 2016-12-09T14:49:04Z

docs/advanced/tls_porting.md


-### Parameters
+The `trng_get_bytes()` function is declared as follows:


Perhaps a short explanation of what this function does would help. Something like: The entropy collector function, trng_get_bytes(), retrieves a specified number of random bytes from the TRNG. Or something along those lines.

andresag01 · 2016-12-09T14:49:49Z

docs/advanced/tls_porting.md


- ``unsigned char *output``: A pointer to the output buffer. The function should write the entropy it collected to the buffer; mbed TLS then uses this data as entropy. Please consult your board's manual and write only the strongest entropy possible in this buffer. 
+- ``trng_t *obj``: `trng_t` is an alias to `trng_s` and it is the callers responsibility to initialise it before passing it to this function and deinitialise it (with the help of `trng_init()` and `trng_free()` respectively) when it is not required anymore.


Style: Please use American English i.e. initialise -> initialize

andresag01 · 2016-12-09T14:53:46Z

docs/advanced/tls_porting.md


-You can read more about how to add a macro for your target [here](../mbed_OS/Targets.md).
+To indicate that the target has an entropy source, you have to add `TRNG` to the capabilities of the target in `targets.json`:


Please specify a path to the targets.json starting from the root of the mbed OS directory.Notice that the previous version of this document contained a link, perhaps that would be useful too.

I deleted the link because the document it was pointing to does not exist anymore.

andresag01 · 2016-12-09T14:55:34Z

docs/advanced/tls_porting.md

-If a hardware platform does not have a hardware entropy source to leverage into the entropy pool, alternatives have to be considered. As said before, a strong entropy source is crucial for security of cryptographic and TLS operations. For platforms that support non-volatile memory, an option is to use the NV seed entropy source that is provided with mbed TLS.
+If a hardware platform does not have a hardware entropy source to leverage into the entropy pool, alternatives have to be considered. As stated above, a strong entropy source is crucial for security of cryptographic and TLS operations. For platforms that support non-volatile memory, an option is to use the NV seed entropy source that is provided with mbed TLS.
+
+This makes mbed TLS use a fixed amount of entropy as a seed and update this seed each time it runs.


I do not think entropy as a seed and update this seed each time it runs is clear enough. Does this mean whenever the device is reset, the entropy source is polled, etc.

andresag01 · 2016-12-09T14:56:19Z

docs/advanced/tls_porting.md


-This makes mbed TLS use a fixed amount of entropy as a seed and update this seed each time it runs. To make this option a relatively strong compromise, the seed should be initialized separately for each device with true random data.
+<span class="notes">**Note:** To make this option a relatively strong compromise, the seed should be initialised separately for each device with true random data at manufacture time. It has to be true random data, something dependant on for example the serial number is NOT secure. </span>


Style: Please use American English e.g. initialised -> initialized

simonbutcher · 2016-12-09T15:08:05Z

LGTM.

andresag01

I am happy with the current state of the document, just a couple of broken links and another small issue.

andresag01 · 2016-12-09T16:37:06Z

docs/advanced/tls_porting.md


- ``unsigned char *output``: A pointer to the output buffer. The function should write the entropy it collected to the buffer; mbed TLS then uses this data as entropy. Please consult your board's manual and write only the strongest entropy possible in this buffer. 
+- ``trng_t *obj``: `trng_t` is an alias to `trng_s` and it is the callers responsibility to initialise it before passing it to this function and deinitialise it (with the help of `trng_init()` and `trng_free()` respectively) when it is not required anymore.


Please use international English i.e. initialise -> initialize. Also deinitialise -> deinitialize, but I am not sure that word exists.

andresag01 · 2016-12-09T16:40:37Z

docs/advanced/tls_porting.md

@@ -78,7 +100,7 @@ To enable the NV seed entropy source, you have to add `MBEDTLS_ENTROPY_NV_SEED`
 "macros": ["MBEDTLS_ENTROPY_NV_SEED", etc.],
 ```

-This makes sure the entropy pool knows it can use the NV seed entropy source. 
+This makes sure the entropy pool knows it can use the NV seed entropy source.

 You can read more about how to add a macro for your target [here](../mbed_OS/Targets.md).


Could you please modify this link? that document no longer exists but it seems the contents were moved to https://github.com/ARMmbed/Handbook/blob/master/docs/advanced/mbed_targets.md

andresag01 · 2016-12-09T16:43:10Z

docs/advanced/tls_porting.md

@@ -111,7 +133,7 @@ Both of the above options are secure if done properly, and depending on the plat

 This option is very dangerous, because compiling with it results in a build that is not secure! You have to let mbed TLS know that you are using it deliberately and you are aware of the consequences. That is why you have to turn off any entropy sources explicitly first.

-Since it is a very dangerous option and no one should use it in production, we recommend to limit its scope as much as possible; you should apply these settings to the application specific config file, instead of the target related configuration as we did it above. You can read more about how to add a macro for your application [here](../mbed_OS/Config_sys.md).
+Since it is a very dangerous option and no one should use it in production, we recommend to limit its scope as much as possible; you should apply these settings to the application specific configuration file, instead of the target related configuration as we did it above. You can read more about how to add a macro for your application [here](../mbed_OS/Config_sys.md).


I think the document ../mbed_OS/Config_sys.md no longer exists. Could you please modify the link to point to https://github.com/ARMmbed/Handbook/blob/master/docs/advanced/config_system.md?

andresag01

I am happy with the current state of the document.

iriark01 · 2016-12-09T17:02:53Z

docs/advanced/tls_porting.md

+### 3.1 What kind of a source can be added
+
+It is very important that you only add a TRNG as described in this section. For the purposes of this document a device is considered a TRNG only if
+    - it is dedicated to generating entropy to be used in cryptographic applications


For every bullet, please start with a capital letter and end with a period.

iriark01 · 2016-12-09T17:03:10Z

docs/advanced/tls_porting.md

+    - careful consideration has been given to how much the data generated is subject to adversarial manipulation
+    - a thorough engineering study has been made to determine how much entropy it can actually provide
+
+For example, an integrated circuit extracting statistically random data from two oscillators of unknown frequencies and independent phases is considered as a TRNG, while anything derived from a real time clock is NOT.


"considered a TRNG". "as a TRNG" means something else

iriark01 · 2016-12-09T17:07:07Z

docs/advanced/tls_porting.md


- ``unsigned char *output``: A pointer to the output buffer. The function should write the entropy it collected to the buffer; mbed TLS then uses this data as entropy. Please consult your board's manual and write only the strongest entropy possible in this buffer. 
+- ``trng_t *obj``: `trng_t` is an alias to `trng_s` and it is the callers responsibility to initialize it before passing it to this function and release it (with the help of `trng_init()` and `trng_free()` respectively) when it is not required anymore.


iriark01 · 2016-12-09T17:10:18Z

docs/advanced/tls_porting.md


-This makes mbed TLS use a fixed amount of entropy as a seed and update this seed each time it runs. To make this option a relatively strong compromise, the seed should be initialized separately for each device with true random data.
+<span class="notes">**Note:** To make this option a relatively strong compromize, the seed should be initialized separately for each device with true random data at manufacture time. It has to be true random data, something dependant on for example the serial number is NOT secure. </span>


manufacturing time
... something dependent on, for example, the serial...

Please lower-case NOT and put it in bold not

iriark01 · 2016-12-09T17:11:38Z

docs/advanced/tls_porting.md

@@ -78,9 +100,9 @@ To enable the NV seed entropy source, you have to add `MBEDTLS_ENTROPY_NV_SEED`
 "macros": ["MBEDTLS_ENTROPY_NV_SEED", etc.],
 ```

-This makes sure the entropy pool knows it can use the NV seed entropy source. 
+This makes sure the entropy pool knows it can use the NV seed entropy source.


makes sure == ensures

* Update TLS porting guide A new interface has been introduced in the hal and it is the new preferred way to enable mbed TLS using the TRNG on a new platform. This PR updates the porting guide to reflect this change. * Apply feedback to mbed TLS Porting Guide update * Apply review feedback to TLS Porting guide * Fix broken links in TLS guide * Apply review feedback to TLS porting guide

Update TLS porting guide

f267133

A new interface has been introduced in the hal and it is the new preferred way to enable mbed TLS using the TRNG on a new platform. This PR updates the porting guide to reflect this change.

simonbutcher suggested changes Nov 4, 2016

View reviewed changes

Apply feedback to mbed TLS Porting Guide update

98774c7

andresag01 suggested changes Dec 9, 2016

View reviewed changes

Apply review feedback to TLS Porting guide

fa33414

andresag01 suggested changes Dec 9, 2016

View reviewed changes

Fix broken links in TLS guide

73cd832

andresag01 approved these changes Dec 9, 2016

View reviewed changes

yanesca changed the title ~~Update TLS porting guide - DRAFT, don't merge~~ Update TLS porting guide Dec 9, 2016

iriark01 reviewed Dec 9, 2016

View reviewed changes

Apply review feedback to TLS porting guide

be997b3

AnotherButler merged commit 2decf95 into ARMmbed:master Dec 13, 2016


		The `mbedtls_hardware_poll` function is declared in `entropy_poll.h`:
		The implementation of this interface needs to be in the HAL.


		### Initialisation and deinitialisation

		Provide means for initialising and deinitialising the peripherial with the following functions:

		@@ -24,46 +24,56 @@ mbed TLS distinguishes between strong and weak entropy sources. Of the sources r


		It is very important that you only add a TRNG the way described in this section. For the purposes of this document a device is considered a TRNG only if it is dedicated to generating entropy to be used in cryptographic applications, and careful consideration has been given to how much the data generated is subject to adversarial manipulation and to how much entropy it can actually provide.

		For example, an integrated circuit extracting statistically random data from two oscillators of unknown frequencies and independent phases is considered as a TRNG, while anything derived from a real time clock is NOT.


		### Parameters
		The `trng_get_bytes()` function is declared as follows:


		- ``unsigned char *output``: A pointer to the output buffer. The function should write the entropy it collected to the buffer; mbed TLS then uses this data as entropy. Please consult your board's manual and write only the strongest entropy possible in this buffer.
		- ``trng_t *obj``: `trng_t` is an alias to `trng_s` and it is the callers responsibility to initialise it before passing it to this function and deinitialise it (with the help of `trng_init()` and `trng_free()` respectively) when it is not required anymore.


		You can read more about how to add a macro for your target [here](../mbed_OS/Targets.md).
		To indicate that the target has an entropy source, you have to add `TRNG` to the capabilities of the target in `targets.json`:


		This makes mbed TLS use a fixed amount of entropy as a seed and update this seed each time it runs. To make this option a relatively strong compromise, the seed should be initialized separately for each device with true random data.
		<span class="notes">Note: To make this option a relatively strong compromise, the seed should be initialised separately for each device with true random data at manufacture time. It has to be true random data, something dependant on for example the serial number is NOT secure. </span>

Update TLS porting guide #17

Update TLS porting guide #17

Uh oh!

Conversation

yanesca commented Oct 18, 2016

Uh oh!

yanesca commented Oct 18, 2016

Uh oh!

simonbutcher 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!

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!

simonbutcher commented Dec 9, 2016

Uh oh!

andresag01 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!

andresag01 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!

Uh oh!