Add a reference example for PSA_ALG_SP800_108_COUNTER_HMAC/CMAC

[mswarowsky]

Adds a reference implementation for a counter mode KDF using the construction recommended by NIST SP 800-108, introduced in #123.

The example is using the PSA API's with mbedtls implementation.

[athoelke]

For source code we are using Apache License 2.0 in this project.

We are using Creative Commons (CC-BY-SA-4.0) for the specification text/graphics, and also for the readme content. However, we are happy to accept this example readme under either license - you can use Apache 2.0 for all of the contribution if that is easier.

We are also trying to consistently use SPDX identifier for both copyright and license tags in every source file.

[athoelke]

A few minor items in the example code:

• Slightly more correct usage flags for the key attributes.
• Extra comment.
• Incorrect error output text in some cases.
• Missing clean up of the operations on error. Mbedtls might permit this behavior (?), and this example will exit on error - but strictly API-conformant code should abort the operation to ensure resource clean up.

[athoelke]

If we are going to be concerned with ensuring that the error clean-up code is correct in this example, we should probably also ensure that the example destroys the temporary keys that are created on each call to sp800_108_counter_hmac_kdf() and sp800_108_counter_cmac_kdf() - on both the success and error paths?

[athoelke]

I am satisfied that the example implements the KDF that we agreed in #123.

I have asked @gilles-peskine-arm to take a look to review this follows best practice as an mbedtls example as well.

[mswarowsky]

If we are going to be concerned with ensuring that the error clean-up code is correct in this example, we should probably also ensure that the example destroys the temporary keys that are created on each call to sp800_108_counter_hmac_kdf() and sp800_108_counter_cmac_kdf() - on both the success and error paths?

Good point, I added calls for psa_destroy_key(...) to clean up correctly

[mswarowsky]

Rebased again, is there anything more I can do to get this moving forward?

[athoelke]

Rebased again, is there anything more I can do to get this moving forward?

Hi @mswarowsky - as the example is based on using the mbedtls implementation, I was hoping to have one of the mbedtls team review the example as well.

Let me check if that is likely to happen soon - if not, then I won't let that block merging the PR.

[gilles-peskine-arm]

This is still on my list, I'll try to review this week.

[gilles-peskine-arm]

I've reviewed the code and the documentation. I won't review the build script because I don't know CMake.

I reviewed to the standards of Mbed TLS except for the code style. We may not have the same goals in the PSA example repository, so I don't necessarily expect all my comments to be addressed. I would prefer to at least resolve the comments marked [MAJOR], but Andrew has the final say.

[mswarowsky]

Addressed the feedback from @gilles-peskine-arm

[mswarowsky]

I changed it to bytes_produced += min(output_size, output_length - bytes_produced); this way bytes_produced will not get bigger than output_length that should fix the problem

[athoelke]

A couple of minor issues. The rework looks correct, Gilles might like to review the update.

As this PR introduces new files, squashing the PR makes incremental review more challenging. Please can you avoid squashing until the review is complete. I will rebase and squash the PR immediately prior to merging, as we don't need/want the edit history in these files..

[gilles-peskine-arm]

Thanks for addressing my comments. Looks good to me apart from some very minor readability issues.

[mswarowsky]

Addressed the new comments, with a fixup commit

[gilles-peskine-arm]

LGTM