Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add example program for PSA hash #7942

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f8b9ebf
Add example program for PSA hash
Jul 18, 2023
209c9c9
Bring code-style up-to-date
tom-daubney-arm Jul 18, 2023
8907815
Added ChangeLog entry
tom-daubney-arm Jul 18, 2023
f7348ae
Improve program from first round review comments
tom-daubney-arm Jul 24, 2023
1db78fa
Demonstrate algorithm agility
tom-daubney-arm Jul 24, 2023
9520df7
Fix code style
tom-daubney-arm Jul 25, 2023
1fd916a
Address review comments
tom-daubney-arm Jul 25, 2023
2c87234
Fix erroneous macro guards
tom-daubney-arm Jul 28, 2023
6fc4ca2
Replace hash_size with hash_length
tom-daubney-arm Jul 28, 2023
a79f806
Remove superfluous calls to psa_hash_abort
tom-daubney-arm Jul 28, 2023
c050037
Remove mbedtls_ and psa_ prefix from var names
tom-daubney-arm Jul 28, 2023
3071c85
Clarify comments
tom-daubney-arm Jul 28, 2023
c07fa29
Change wording in error message
tom-daubney-arm Jul 28, 2023
9730cb1
Change hash output formatting
tom-daubney-arm Jul 28, 2023
a2b7519
Use memcmp instead of reinventing it
tom-daubney-arm Jul 28, 2023
1f98736
Add clarifying comment to new program section
tom-daubney-arm Jul 28, 2023
606110f
Restructure start of program
tom-daubney-arm Jul 28, 2023
ce14124
Check result of multipart operation
tom-daubney-arm Jul 28, 2023
fbe742b
Add extra check to one-shot operation results
tom-daubney-arm Jul 28, 2023
c918c32
Stop hashing the null byte
tom-daubney-arm Jul 28, 2023
1ba9744
Correct code style
tom-daubney-arm Jul 28, 2023
21fbe4c
Remove further superfluous call to psa_hash_abort
tom-daubney-arm Aug 3, 2023
5c2dcbd
Implement cleanup label
tom-daubney-arm Aug 3, 2023
102033c
Add new line at end of file to satisfy code style
tom-daubney-arm Aug 3, 2023
a68ef95
Check length before calling memcmp
tom-daubney-arm Aug 7, 2023
86f9795
Update documentation
tom-daubney-arm Oct 10, 2023
7605388
Inform user when unknown hash algorithm supplied
tom-daubney-arm Oct 10, 2023
3450087
Remove trailing white space in documentation
tom-daubney-arm Oct 11, 2023
1c2378b
Add variable for message length
tom-daubney-arm Oct 11, 2023
cd79f77
Add missing newline
tom-daubney-arm Oct 11, 2023
d8453bb
Remove superfluous comment
tom-daubney-arm Oct 11, 2023
a21c972
Remove extra blank line
tom-daubney-arm Oct 11, 2023
2e67781
Alter program layout for better clarity
tom-daubney-arm Oct 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ChangeLog.d/add-psa-example-program-hash.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Features
* Added an example program showing how to hash with the PSA API.
5 changes: 5 additions & 0 deletions programs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ APPS = \
psa/hmac_demo \
psa/key_ladder_demo \
psa/psa_constant_names \
psa/psa_hash \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's reorganize later.

random/gen_entropy \
random/gen_random_ctr_drbg \
ssl/dtls_client \
Expand Down Expand Up @@ -316,6 +317,10 @@ psa/psa_constant_names$(EXEXT): psa/psa_constant_names.c psa/psa_constant_names_
echo " CC psa/psa_constant_names.c"
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) psa/psa_constant_names.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@

psa/psa_hash$(EXEXT): psa/psa_hash.c $(DEP)
echo " CC psa/psa_hash.c"
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) psa/psa_hash.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@

random/gen_entropy$(EXEXT): random/gen_entropy.c $(DEP)
echo " CC random/gen_entropy.c"
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) random/gen_entropy.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@
Expand Down
1 change: 1 addition & 0 deletions programs/psa/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ set(executables
hmac_demo
key_ladder_demo
psa_constant_names
psa_hash
)

if(GEN_FILES)
Expand Down
147 changes: 147 additions & 0 deletions programs/psa/psa_hash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@tom-daubney-arm tom-daubney-arm Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#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.

* Example computing a SHA-256 hash using the PSA Crypto API
*
* The example computes the SHA-256 hash of a test string using the
* one-shot API call psa_hash_compute() and the using multi-part
* operation, which requires psa_hash_setup(), psa_hash_update() and
* psa_hash_finish(). The multi-part operation is popular on embedded
* devices where a rolling hash needs to be computed.
*
*
* Copyright The Mbed TLS Contributors
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#include "psa/crypto.h"
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

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

#define HASH_ALG PSA_ALG_SHA_256

#define TEST_SHA256_HASH { \
0x5a, 0x09, 0xe8, 0xfa, 0x9c, 0x77, 0x80, 0x7b, 0x24, 0xe9, 0x9c, 0x9c, \
0xf9, 0x99, 0xde, 0xbf, 0xad, 0x84, 0x41, 0xe2, 0x69, 0xeb, 0x96, 0x0e, \
0x20, 0x1f, 0x61, 0xfc, 0x3d, 0xe2, 0x0d, 0x5a \
}

const uint8_t mbedtls_test_sha256_hash[] = TEST_SHA256_HASH;

const size_t mbedtls_test_sha256_hash_len =
sizeof(mbedtls_test_sha256_hash);

#if !defined(MBEDTLS_PSA_CRYPTO_C) || !defined(PSA_WANT_ALG_SHA_256)
int main(void)
{
mbedtls_printf("MBEDTLS_PSA_CRYPTO_C and MBEDTLS_SHA256_C"
"not defined.\r\n");
return EXIT_SUCCESS;
}
#else

int main(void)
{
uint8_t buf[] = "Hello World!";
psa_status_t status;
uint8_t hash[PSA_HASH_LENGTH(HASH_ALG)];
size_t hash_size;
psa_hash_operation_t sha256_psa = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t cloned_sha256 = PSA_HASH_OPERATION_INIT;

mbedtls_printf("PSA Crypto API: SHA-256 example\n\n");

status = psa_crypto_init();
if (status != PSA_SUCCESS) {
mbedtls_printf("psa_crypto_init failed\n");
return EXIT_FAILURE;
}


/* Compute hash using multi-part operation */

status = psa_hash_setup(&sha256_psa, HASH_ALG);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since technically this can be of any size maybe it's better not to name it sha256_psa?

if (status != PSA_SUCCESS) {
mbedtls_printf("psa_hash_setup failed\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like Gilles pointed out psa_hash_abort is not needed if psa_hash_setup was not successful in the first place, also I think it's better if we have cleanup labels at the ends that we can jump to in case of failure so we don't dublicate cleanup code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh whoops yeah good catch. With the clean up labels you want, do you mean so that the calls to hash_abort get put there, along with the return EXIT_FAILURE?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes so at the end you have like:
cleanup: psa_hash_abort(&hash_operation); psa_hash_abort(&cloned_hash_operation);
and in case of any failure you do goto cleanup;
in this particular case you don't need to do that because you don't need to call psa_hash_abort but in other cases it' will be cleaner I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, yeah, agree that is better. Will implement now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@waleed-elmelegy-arm I have made the changes. PTAL.

return EXIT_FAILURE;
}

status = psa_hash_update(&sha256_psa, buf, sizeof(buf));
if (status != PSA_SUCCESS) {
mbedtls_printf("psa_hash_update failed\n");
return EXIT_FAILURE;
}

status = psa_hash_clone(&sha256_psa, &cloned_sha256);
if (status != PSA_SUCCESS) {
mbedtls_printf("PSA hash clone failed");
return EXIT_FAILURE;
}

status = psa_hash_finish(&sha256_psa, hash, sizeof(hash), &hash_size);
if (status != PSA_SUCCESS) {
mbedtls_printf("psa_hash_finish failed\n");
return EXIT_FAILURE;
}

status =
psa_hash_verify(&cloned_sha256, mbedtls_test_sha256_hash, mbedtls_test_sha256_hash_len);
if (status != PSA_SUCCESS) {
mbedtls_printf("psa_hash_verify failed\n");
return EXIT_FAILURE;
} else {
mbedtls_printf("Multi-part hash operation successful!\n");
}

/* Compute hash using one-shot function call */
memset(hash, 0, sizeof(hash));
hash_size = 0;

status = psa_hash_compute(HASH_ALG,
buf, sizeof(buf),
hash, sizeof(hash),
&hash_size);
if (status != PSA_SUCCESS) {
mbedtls_printf("psa_hash_compute failed\n");
return EXIT_FAILURE;
}

for (size_t j = 0; j < mbedtls_test_sha256_hash_len; j++) {
if (hash[j] != mbedtls_test_sha256_hash[j]) {
mbedtls_printf("One-shot hash operation failed!\n\n");
return EXIT_FAILURE;
}
}

mbedtls_printf("One-shot hash operation successful!\n\n");

mbedtls_printf("The SHA-256( '%s' ) is:\n", buf);

for (size_t j = 0; j < mbedtls_test_sha256_hash_len; j++) {
if (j % 8 == 0) {
mbedtls_printf("\n ");
}
mbedtls_printf("%02x ", hash[j]);
}

mbedtls_printf("\n");

mbedtls_psa_crypto_free();
return EXIT_SUCCESS;
}
#endif /* MBEDTLS_PSA_CRYPTO_C && MBEDTLS_SHA256_C */