Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash #325

gilles-peskine-arm · 2019-11-26T17:28:08Z

Rename psa_asymmetric_sign to psa_sign_hash to make it clearer that this function takes a hash as its input (or something that serves as a hash). Likewise for psa_asymmetric_verify to psa_verify_hash. Rename some macros accordingly. A follow-up will add a new pair of functions psa_sign_message and psa_verify_message.

Keep deprecated aliases. Improve the testing of deprecated features: do test some deprecated features when MBEDTLS_DEPRECATED_REMOVED and MBEDTLS_DEPRECATED_WARNING are both disabled, and add a test component that builds and runs with those tests and MBEDTLS_DEPRECATED_WARNING enabled and with -Werror=deprecated-declarations disabled.

This enacts a change in the PSA specification (private link: https://github.com/ARMmbed/psa-crypto/pull/301). Internal ref: IOTCRYPT-933

Move backward compatibility aliases to a separate header. Reserve crypto_extra.h for implementation-specific extensions that we intend to keep supporting. This is better documentation for users. New users should simply ignore backward compatibility aliases, and old users can look at crypto_compat.h to see what is deprecated without bothering about new features appearing in crypto_extra.h. This facilitates maintenance because scripts such as generate_psa_constants that want to ignore backward compability aliases can simply exclude crypto_compat.h from their parsing.

Generalize MBEDTLS_DEPRECATED_NUMERIC_CONSTANT into macros that can accommodate types other than int.

Rename some macros and functions related to signature which are changing as part of the addition of psa_sign_message and psa_verify_message. perl -i -pe '%t = ( PSA_KEY_USAGE_SIGN => PSA_KEY_USAGE_SIGN_HASH, PSA_KEY_USAGE_VERIFY => PSA_KEY_USAGE_VERIFY_HASH, PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE => PSA_SIGNATURE_MAX_SIZE, PSA_ASYMMETRIC_SIGN_OUTPUT_SIZE => PSA_SIGN_OUTPUT_SIZE, psa_asymmetric_sign => psa_sign_hash, psa_asymmetric_verify => psa_verify_hash, ); s/\b(@{[join("|", keys %t)]})\b/$t{$1}/ge' $(git ls-files . ':!:**/crypto_compat.h')

Define deprecated aliases for identifiers that are being renamed.

When MBEDTLS_TEST_DEPRECATED is defined, run some additional tests to validate deprecated PSA macros. We don't need to test deprecated features extensively, but we should at least ensure that they don't break the build. Add some code to component_build_deprecated in all.sh to run these tests with MBEDTLS_DEPRECATED_WARNING enabled. The tests are also executed when MBEDTLS_DEPRECATED_WARNING and MBEDTLS_DEPRECATED_REMOVED are both disabled.

Test psa_asymmetric_sign and psa_asymmetric_verify.

We're going to create some edge cases where the attributes of a key are not bitwise identical to the attributes passed during creation. Have a test function ready for that.

gilles-peskine-arm · 2019-11-28T10:01:59Z

CI passed except for expected failures: an ABI change (two external symbols have been renamed) and generated files in TLS (due to the new header).

AndrzejKurek · 2019-11-29T09:23:43Z

include/psa/crypto_compat.h

+/**
+ * \file psa/crypto_compat.h
+ *
+ * \brief PSA cryptography module: Backward compatibility aliases


I would add a sentence about this file also having deprecation information.

What do you mean? Something like “This header declares alternative names for macros and functions that may be removed in a future version of Mbed Crypto”?

Or just Backward compatibility aliases and deprecated entities

AndrzejKurek

PSA_ASYMMETRIC_SIGN_OUTPUT_SIZE is still used in test_suite_psa_crypto.function.

gilles-peskine-arm · 2019-11-29T09:44:38Z

tests/suites/test_suite_psa_crypto.function

    TEST_EQUAL( actual_size, (size_t) expected_size_arg );
+#if defined(MBEDTLS_TEST_DEPRECATED)
+    TEST_EQUAL( actual_size,
+                PSA_ASYMMETRIC_SIGN_OUTPUT_SIZE( type, bits, alg ) );


This is conditioned by MBEDTLS_TEST_DEPRECATED. Using a deprecated macro outside of a MBEDTLS_TEST_DEPRECATED guard should result in a CI failure in the job with MBEDTLS_DEPRECATED_WARNING set.

AndrzejKurek · 2019-11-29T10:59:34Z

tests/suites/test_suite_psa_crypto.function

@@ -3769,6 +3769,15 @@ void sign_deterministic( int key_type_arg, data_t *key_data,
    ASSERT_COMPARE( output_data->x, output_data->len,
                    signature, signature_length );

+#if defined(MBEDTLS_TEST_DEPRECATED)
+    PSA_ASSERT( psa_asymmetric_sign( handle, alg,


I was thinking about a potential gap in testing deprecated stuff here:
If psa_asymmetric_sign somehow happens to return PSA_SUCCESS on error, but doesn't modify signature in the process, the next ASSERT_COMPARE will succeed anyway, since there's old data in the buffer.
Not sure if it's worth to add buffer zeroization here, since psa_asymmetric_sign should not behave badly.

I don't think it's useful to test deprecated identifiers as black boxes. My intent was a gray-box test, where we assume that the deprecated identifiers are defined as aliases but they may be defined as the wrong alias, or pass the wrong parameter, or have syntax errors in a macro expansion. Zeroing signature here is cheap and useful so I'll do it.

Catch more potential plumbing errors such as not returning the right value or not writing to an output parameter.

Note that the identifiers declared in this header are deprecated. Indicate what API version identifiers were from.

AndrzejKurek

I've verified that the renamed entities are not used across the library (apart from crypto_compat and deprecation tests). The changes look fine.

gilles-peskine-arm · 2019-12-02T12:18:50Z

Again CI (head, merge) passed except for expected failures: an ABI change (two external symbols have been renamed) and generated files in TLS (due to the new header).

* ARMmbed#321: Replace config.pl by config.py * ARMmbed#322: Update Mbed Crypto with latest Mbed TLS changes as of 2019-11-15 * ARMmbed#308: Small performance improvement of mbedtls_mpi_div_mpi() * ARMmbed#324: test_psa_constant_names: support key agreement, better code structure * ARMmbed#320: Link to the PSA crypto portal page from README.md * ARMmbed#293: Always gather MBEDTLS_ENTROPY_BLOCK_SIZE bytes of entropy * ARMmbed#310: Clarify test descriptions in test_suite_memory_buffer_alloc * ARMmbed#307: Add ASN.1 ENUMERATED tag support * ARMmbed#328: Remove dependency of crypto_values.h on crypto_extra.h * ARMmbed#325: Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash Missed listing in the previous submodule update: * ARMmbed#304: Make sure Asan failures are detected in 'make test'

gilles-peskine-arm added 3 commits November 26, 2019 18:20

Simplify support for deprecated constants of various types

7b0ab6d

Generalize MBEDTLS_DEPRECATED_NUMERIC_CONSTANT into macros that can accommodate types other than int.

gilles-peskine-arm added needs: review The pull request is ready for review. This generally means that it has no known issues. needs: ci Needs a passing full CI run labels Nov 26, 2019

gilles-peskine-arm force-pushed the psa-sign_hash branch from 17061a4 to dd0bf39 Compare November 26, 2019 18:01

gilles-peskine-arm added 5 commits November 26, 2019 19:08

Add backward compatibility aliases for signature-related identifiers

4151094

Define deprecated aliases for identifiers that are being renamed.

Make the key_policy test function more flexible

1a96049

Add tests of deprecated PSA functions

0627f98

Test psa_asymmetric_sign and psa_asymmetric_verify.

Add test function for effective key attributes

06c2889

We're going to create some edge cases where the attributes of a key are not bitwise identical to the attributes passed during creation. Have a test function ready for that.

gilles-peskine-arm force-pushed the psa-sign_hash branch from dd0bf39 to 06c2889 Compare November 26, 2019 18:14

dgreen-arm requested review from AndrzejKurek and Patater November 28, 2019 09:36

gilles-peskine-arm removed the needs: ci Needs a passing full CI run label Nov 28, 2019

gilles-peskine-arm mentioned this pull request Nov 28, 2019

Implement psa_hash_compute and psa_hash_compare #327

Merged

AndrzejKurek reviewed Nov 29, 2019

View reviewed changes

AndrzejKurek suggested changes Nov 29, 2019

View reviewed changes

gilles-peskine-arm commented Nov 29, 2019

View reviewed changes

AndrzejKurek reviewed Nov 29, 2019

View reviewed changes

gilles-peskine-arm added 3 commits November 29, 2019 12:15

Add negative test cases for deprecated aliases

895242b

Catch more potential plumbing errors such as not returning the right value or not writing to an output parameter.

Document MBEDTLS_TEST_DEPRECATED

2ff02c3

Better documentation in crypto_compat.h

0168f2f

Note that the identifiers declared in this header are deprecated. Indicate what API version identifiers were from.

AndrzejKurek approved these changes Nov 29, 2019

View reviewed changes

AndrzejKurek reviewed Nov 29, 2019

View reviewed changes

Patater removed their request for review December 4, 2019 07:17

dgreen-arm self-requested a review December 4, 2019 09:16

gilles-peskine-arm mentioned this pull request Dec 4, 2019

Streamline PSA key type and curve encodings #330

Merged

dgreen-arm approved these changes Dec 5, 2019

View reviewed changes

gilles-peskine-arm removed the needs: review The pull request is ready for review. This generally means that it has no known issues. label Dec 6, 2019

gilles-peskine-arm merged commit 81f7909 into ARMmbed:development Dec 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash #325

Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash #325

Uh oh!

gilles-peskine-arm commented Nov 26, 2019 •

edited

Loading

Uh oh!

gilles-peskine-arm commented Nov 28, 2019

Uh oh!

AndrzejKurek Nov 29, 2019

Uh oh!

gilles-peskine-arm Nov 29, 2019

Uh oh!

AndrzejKurek Nov 29, 2019

Uh oh!

AndrzejKurek left a comment

Uh oh!

gilles-peskine-arm Nov 29, 2019

Uh oh!

AndrzejKurek Nov 29, 2019

Uh oh!

gilles-peskine-arm Nov 29, 2019

Uh oh!

AndrzejKurek left a comment

Uh oh!

gilles-peskine-arm commented Dec 2, 2019 •

edited

Loading

Uh oh!

Uh oh!

Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash #325

Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash #325

Uh oh!

Conversation

gilles-peskine-arm commented Nov 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gilles-peskine-arm commented Nov 28, 2019

Uh oh!

AndrzejKurek Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek left a comment

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek left a comment

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

gilles-peskine-arm commented Nov 26, 2019 •

edited

Loading

gilles-peskine-arm commented Dec 2, 2019 •

edited

Loading