Add example program for PSA hash

[tom-daubney-arm]

Description

This PR contains the PSA Hash program, originally authored by @hannestschofenig .

This PR is the second part of a series of PRs that are based on #5064 , which has been split in order to make reviewing easier.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

• changelog done
• backport not required
• tests not required

[gilles-peskine-arm]

There's a directory programs/psa that contains all programs that are related to the PSA API: examples of simple usage like psa_hash and crypto_examples and aead_demo and hmac_demo, examples of more complex usage like key_ladder_demo, and utility programs for developers (just one: psa_constant_names). This made sense originally when PSA was an experimental feature, but now that it's the preferred API for cryptography, maybe we shouldn't single out psa anymore?

[tom-daubney-arm]

Happy to do this as a follow-up PR but in this PR I want to focus on getting the PSA hash program merged.

[gilles-peskine-arm]

Sure, let's reorganize later.

[gilles-peskine-arm]

We should use mbedtls_printf. We normally don't use stdio directly in sample programs for crypto.

[gilles-peskine-arm]

It's unfortunate that we don't run sample programs on the CI. I would like that at least for new programs we arrange to run them on the CI. #2698 sets up some infrastructure to make that easier. Can we rework this as a part of the epic to get the PSA demo programs in?

[tom-daubney-arm]

At the moment I want to focus on bringing in these PSA programs as a priority, especially since they have been around a while already. I think the suggestion here introduces too much scope creep to the original plan. I am happy to discuss it and potentially work on it after the PSA programs are done, but I don't think we should make #2698 part of this bit of work.

[gilles-peskine-arm]

As discussed today, I'm going to split #2698 into two parts: one to add run_demos.py to CI runs, and one for cert_write_demo.hs. We'll push the run_demos.py part quickly and we'll add a demo script for the new sample programs.

[gilles-peskine-arm]

#7982 is available for review. I would like to merge it and the hash demo independently, but to make a follow-up to run the hash demo on the CI before we merge the other demos.

[gilles-peskine-arm]

LGTM

[tom-cosgrove-arm]

We don't normally have two consecutive blank lines in our code

[gilles-peskine-arm]

ag -c '\n\n\n' library/*.c programs/*/*.c

Yes we do. And I can't think of a good reason why that would be a problem.

[tom-cosgrove-arm]

Hmm, yes, so we do, but used so inconsistently that they are clearly unintentional (i.e. not clearly separating sections of a file, etc)

[tom-cosgrove-arm]

I think the sample program would read better if this sample message and its length appeared before the expected hash value. And note "expected" - again, it would read better with EXPECTED_HASH_DATA, expected_hash, and expected_hash_len.

Also, these variables are outside the macro guards, which means if it's compiled without MBEDTLS_PSA_CRYPTO_C or PSA_WANT_ALG_SHA_256, some compilers will give warnings about unused variables - at least, they should, but now I see that they're not static, which they probably should be - I don't see a reason to have them global. So they should be static and within the compilation guard.

Finally, I wonder if it's worth having the "interesting" version of main() - i.e. the one with all the good stuff in it - before the un-interesting "these aren't defined" version. That would mean the guards become #if defined(MBEDTLS_PSA_CRYPTO_C) && defined(PSA_WANT_ALG_SHA_256) and therefore match the #endif at the bottom of the file, which isn't currently right.

However, that is the opposite of the way the example programs are currently done, so I'd be keen to see what other reviewers think.

That is, how does the following layout seem?

:
#include "mbedtls/build_info.h"
#include "mbedtls/platform.h"

#if defined(MBEDTLS_PSA_CRYPTO_C) && defined(PSA_WANT_ALG_SHA_256)

/* Information about hashing with the PSA API can be found here:
 * https://arm-software.github.io/psa-api/crypto/1.1/api/ops/hashes.html
 :
 * the hash output in the EXPECTED_HASH_DATA macro below. */

#define HASH_ALG PSA_ALG_SHA_256

static const uint8_t sample_message[] = "Hello World!";
/* sample_message is terminated with a null byte which is not part of
 * the message itself so we make sure to subtract it in order to get
 * the message length. */
static const size_t sample_message_length = sizeof(sample_message) - 1;

#define EXPECTED_HASH_DATA {                                                \
    0x7f, 0x83, 0xb1, 0x65, 0x7f, 0xf1, 0xfc, 0x53, 0xb9, 0x2d, 0xc1, 0x81, \
    0x48, 0xa1, 0xd6, 0x5d, 0xfc, 0x2d, 0x4b, 0x1f, 0xa3, 0xd6, 0x77, 0x28, \
    0x4a, 0xdd, 0xd2, 0x00, 0x12, 0x6d, 0x90, 0x69 \
}

static const uint8_t expected_hash[] = EXPECTED_HASH_DATA;
static const size_t expected_hash_len = sizeof(expected_hash);

int main(void)
{
    psa_status_t status;
    uint8_t hash[PSA_HASH_LENGTH(HASH_ALG)];
    :
    :
    return EXIT_FAILURE;
}

#else   /* MBEDTLS_PSA_CRYPTO_C && PSA_WANT_ALG_SHA_256 */

int main(void)
{
    mbedtls_printf("MBEDTLS_PSA_CRYPTO_C and PSA_WANT_ALG_SHA_256"
                   "not defined.\r\n");
    return EXIT_SUCCESS;
}

#endif  /* MBEDTLS_PSA_CRYPTO_C && PSA_WANT_ALG_SHA_256 */

[gilles-peskine-arm]

I think the sample program would read better if this sample message and its length appeared before the expected hash value. And note "expected" - again, it would read better with EXPECTED_HASH_DATA, expected_hash, and expected_hash_len.

Agreed, though I don't consider this a blocker.

Also, these variables are outside the macro guards, which means if it's compiled without MBEDTLS_PSA_CRYPTO_C or PSA_WANT_ALG_SHA_256, some compilers will give warnings about unused variables - at least, they should, but now I see that they're not static, which they probably should be - I don't see a reason to have them global. So they should be static and within the compilation guard.

I don't care about any of this if the CI passes. Not even by the standards of a sample program. This is the part that will be replaced if someone takes this and turns it into actual code.

Finally, I wonder if it's worth having the "interesting" version of main() - i.e. the one with all the good stuff in it - before the un-interesting "these aren't defined" version. That would mean the guards become #if defined(MBEDTLS_PSA_CRYPTO_C) && defined(PSA_WANT_ALG_SHA_256) and therefore match the #endif at the bottom of the file, which isn't currently right.

However, that is the opposite of the way the example programs are currently done, so I'd be keen to see what other reviewers think.

I agree, but I'd rather be consistent between the sample programs than have one program read very slightly better.

[tom-daubney-arm]

I think I have implemented a compromise. PTAL when you have the bandwidth

[gilles-peskine-arm]

LGTM

[tom-cosgrove-arm]

LGTM

[tom-cosgrove-arm]

Merge queue failures appear to be transient errors, re-submitting

[gilles-peskine-arm]

The merge queue failure is real: check_code_style is failing (and then other jobs are marked as unstable because they were aborted).

The failure isn't caused by this PR though, so it looks like development is broken.