Update the values of PSA Crypto API macros to version 1.0.0 #3948

bensze01 · 2020-12-07T20:10:50Z

Description

This PR contains what I believe to be the minimum changes required for us to be compatible with the stable storage format going forward.

Closes #3276
Closes #3921

Status

READY

Requires Backporting

No, PSA only

Migrations

The storage format changes to be in line with the PSA Crypto API version 1.0.0

Todos

Storage format tests updated
Changelog entry

This algorithm replaces the pre-existing stream cipher algorithms. The underlying stream cipher is determined by the key type. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

This should be everything that's needed for the stable storage format. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

gilles-peskine-arm

I made a first pass and verified that this is a sensible change which affects the areas of the code that it should affect, no more no less, except:

• Missing changes of key storage tests in test_suite_psa_crypto_persistent_key.data.
• Missing changelog entry.

Other than some minor issues in crypto_compat.h, and provided that the new values are the ones we want (which I will validate after posting this review), this looks good to me.

include/psa/crypto_compat.h

include/psa/crypto_values.h

gilles-peskine-arm · 2020-12-07T21:39:46Z

I merged a294551 from this pull request with 4b11b25 from #3936 and ran programs/test/verify_psa_macros. It passes.

There are warnings, but none of them are problematic. Excluding size macros and get/test macros,

programs/test/verify_psa_macros 2>&1 | grep -Ev '_(LENGTH|SIZE)[( ]|_(GET|IS)_'

only lists identifers that Mbes TLS doesn't declare because they're unsupported, plus:

WARN:  PSA_ALG_AEAD_WITH_DEFAULT_LENGTH_TAG(aead_alg) is not defined
WARN:  PSA_ALG_AEAD_WITH_SHORTENED_TAG(aead_alg, tag_length) is not defined
WARN:  PSA_ALG_NONE is not defined
WARN:  PSA_KEY_ID_NULL is not defined
WARN:  PSA_KEY_LOCATION_PRIMARY_SECURE_ELEMENT is not defined

The AEAD_WITH_xxx_TAG macros have an older name. I verified manually that their new definitions are correct, as well as the corresponding macros for MAC which the current version of verify_psa_macros does not verify adequately.

I also made some spot checks.

I am satisfied that the new values are correct.

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Deprecate PSA_ALG_ARC4 and PSA_ALG_CHACHA20. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 · 2020-12-07T23:33:07Z

@gilles-peskine-arm I think I've addressed all of the feedback.

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

mpg

After reviewing the changes and using the test scripts, I'm happy with this PR.

I left an optional suggestion for improving internal documentation, but it could be left for later.

mpg · 2020-12-08T08:50:47Z

tests/scripts/test_psa_constant_names.py

-        self.mac_algorithms = set(['0x02ff00ff'])
-        self.ka_algorithms = set(['0x30fc0000'])
-        self.kdf_algorithms = set(['0x200000ff'])
+        self.hash_algorithms = set(['0x020000fe'])


I have no idea how these values were chosen. Can you add a comment about that?

That's more my failing originally. They have to have correct values for PSA_ALG_IS_xxx, and to be distinct from any current or likely values of a valid PSA_ALG_xxx. So for example hash algorithms have the pattern 0x020000nn, and the spec starts from low values of nn, so the numerical value for testing takes n==0xfe (why not n==0xff as before? It doesn't matter).

hash_algorithms ended with 0xfe before my patch as well, so I left it like that.

Oh, It's because of PSA_ALG_ANY_HASH - ~~which incidentally wasn't updated. Give me a moment.~~

Ah, good spot. I should have put a comment about this.

Sorry, false alarm. PSA_ALG_ANY_HASH was updated. I was just looking at the wrong branch.

mpg · 2020-12-08T08:54:26Z

tests/suites/test_suite_psa_crypto_persistent_key.data

@@ -1,14 +1,14 @@
 Format for storage: RSA private key
-format_storage_data_check:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"505341004b455900000000000100000001700004010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN
+format_storage_data_check:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"505341004b455900000000000100000001700004010000000000000700000006620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN


Note for future PRs (not asking for a history rewrite here): I think the commit message should give more details about what changes were made and how the new data was generated. (I guess from the slack history that you manually edited the hex data, but that should really be available from the git history.)

The mistake was introduced in Mbed-TLS#3948.

The mistake was introduced in Mbed-TLS#3948. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 added 2 commits December 7, 2020 21:03

Introduce PSA_ALG_STREAM_CIPHER

1de907d

This algorithm replaces the pre-existing stream cipher algorithms. The underlying stream cipher is determined by the key type. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Update PSA Crypto macro values to version 1.0.0

a294551

This should be everything that's needed for the stable storage format. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 added mbed TLS team labels Dec 7, 2020

bensze01 requested a review from gilles-peskine-arm December 7, 2020 20:10

bensze01 self-assigned this Dec 7, 2020

gilles-peskine-arm added component-crypto Crypto primitives and low-level interfaces needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Dec 7, 2020

gilles-peskine-arm requested changes Dec 7, 2020

View reviewed changes

include/psa/crypto_compat.h Show resolved Hide resolved

include/psa/crypto_compat.h Outdated Show resolved Hide resolved

include/psa/crypto_values.h Outdated Show resolved Hide resolved

bensze01 added 4 commits December 7, 2020 23:03

Update PSA Crypto persistent key storage tests

c6a26c9

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Substitute PSA_ALG_STREAM_CIPHER into all files

cbe3953

Deprecate PSA_ALG_ARC4 and PSA_ALG_CHACHA20. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Simplify description of PSA_ALG_STREAM_CIPHER

99ffb2b

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Add changelog

6879f42

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 added the needs-ci Needs to pass CI tests label Dec 7, 2020

bensze01 requested a review from gilles-peskine-arm December 7, 2020 23:35

Update test_psa_constant_names.py for new values

7e37bf9

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 removed the needs-ci Needs to pass CI tests label Dec 8, 2020

mpg self-requested a review December 8, 2020 08:21

gilles-peskine-arm approved these changes Dec 8, 2020

View reviewed changes

gilles-peskine-arm removed the needs-reviewer This PR needs someone to pick it up for review label Dec 8, 2020

mpg approved these changes Dec 8, 2020

View reviewed changes

bensze01 removed the needs-review Every commit must be reviewed by at least two team members, label Dec 8, 2020

gilles-peskine-arm merged commit 2b75962 into Mbed-TLS:development Dec 8, 2020

bensze01 mentioned this pull request Dec 8, 2020

Improve documentation in test_psa_constant_names.py #3949

Merged

bensze01 added a commit to bensze01/mbedtls that referenced this pull request Feb 8, 2021

Fix mistake in 2.25.0 Changelog

1877ed9

The mistake was introduced in Mbed-TLS#3948.

bensze01 mentioned this pull request Feb 8, 2021

Fix mistake in 2.25.0 Changelog #4120

Merged

bensze01 added a commit to bensze01/mbedtls that referenced this pull request Feb 8, 2021

Fix mistake in 2.25.0 Changelog

fe9a425

The mistake was introduced in Mbed-TLS#3948. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the values of PSA Crypto API macros to version 1.0.0 #3948

Update the values of PSA Crypto API macros to version 1.0.0 #3948

bensze01 commented Dec 7, 2020 •

edited

Loading

gilles-peskine-arm left a comment

gilles-peskine-arm commented Dec 7, 2020 •

edited

Loading

bensze01 commented Dec 7, 2020

mpg left a comment

mpg Dec 8, 2020

gilles-peskine-arm Dec 8, 2020

bensze01 Dec 8, 2020

bensze01 Dec 8, 2020 •

edited

Loading

gilles-peskine-arm Dec 8, 2020

bensze01 Dec 8, 2020

mpg Dec 8, 2020

Update the values of PSA Crypto API macros to version 1.0.0 #3948

Update the values of PSA Crypto API macros to version 1.0.0 #3948

Conversation

bensze01 commented Dec 7, 2020 • edited Loading

Description

Status

Requires Backporting

Migrations

Todos

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Dec 7, 2020 • edited Loading

bensze01 commented Dec 7, 2020

mpg left a comment

Choose a reason for hiding this comment

mpg Dec 8, 2020

Choose a reason for hiding this comment

gilles-peskine-arm Dec 8, 2020

Choose a reason for hiding this comment

bensze01 Dec 8, 2020

Choose a reason for hiding this comment

bensze01 Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

gilles-peskine-arm Dec 8, 2020

Choose a reason for hiding this comment

bensze01 Dec 8, 2020

Choose a reason for hiding this comment

mpg Dec 8, 2020

Choose a reason for hiding this comment

bensze01 commented Dec 7, 2020 •

edited

Loading

gilles-peskine-arm commented Dec 7, 2020 •

edited

Loading

bensze01 Dec 8, 2020 •

edited

Loading