Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Driver dispatch for PSA asymmetric encryption + RSA tests #5292

Merged
merged 26 commits into from
Mar 10, 2022

Conversation

mprse
Copy link
Contributor

@mprse mprse commented Dec 9, 2021

Description

Resolves #5250. Resolves #5249.

Driver dispatch for PSA asymmetric encryption.

Status

READY

Requires Backporting

Yes 2.x?

Migrations

NO

Todos

  • Impl for psa_asymmetric_decrypt
  • Add tests for asymmetric encrypt/decrypt
  • Add driver dispatch tests for RSA signatures

@mprse mprse linked an issue Dec 9, 2021 that may be closed by this pull request
@mprse mprse force-pushed the asym_encrypt branch 4 times, most recently from ce99bd8 to 089f9f3 Compare December 13, 2021 09:01
@mprse
Copy link
Contributor Author

mprse commented Dec 13, 2021

In this PR I also see the problem with windows build (but different than #5139 (comment)):

42>ClCompile:
         crypto_examples.c
    89>C:\Program Files (x86)\MSBuild\Microsoft\VisualStudio\v12.0\CodeAnalysis\Microsoft.CodeAnalysis.targets(214,5): error MSB4175: The task factory "CodeTaskFactory" could not be loaded from the assembly "C:\Program Files (x86)\MSBuild\12.0\bin\amd64\Microsoft.Build.Tasks.v12.0.dll". Could not find file 'C:\Users\Administrator\AppData\Local\Temp\j5mjvtfy.dll'. [C:\builds\workspace\mbed-tls-pr-merge_PR-5292-merge\worktrees\tmpjxoaly25\visualc\VS2010\rsa_verify_pss.vcxproj]
    89>Done Building Project "C:\builds\workspace\mbed-tls-pr-merge_PR-5292-merge\worktrees\tmpjxoaly25\visualc\VS2010\rsa_verify_pss.vcxproj" (Rebuild target(s)) -- FAILED.
    32>Done Building Project "C:\builds\workspace\mbed-tls-pr-merge_PR-5292-merge\worktrees\tmpjxoaly25\visualc\VS2010\rsa_verify_pss.vcxproj.metaproj" (Rebuild target(s)) -- FAILED.

@mprse mprse changed the title Driver dispatch for PSA asymmetric encryption Driver dispatch for PSA asymmetric encryption + RSA tests Dec 20, 2021
@mprse mprse linked an issue Dec 22, 2021 that may be closed by this pull request
@mprse
Copy link
Contributor Author

mprse commented Dec 22, 2021

I couldn't find test vectors for all functions/algorithms:

hash sign hash verify message sign message verify
PSA_ALG_RSA_PKCS1V15_SIGN_RAW OK OK
PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256) OK OK OK
PSA_ALG_RSA_PSS(PSA_ALG_SHA_256) OK OK
PSA_ALG_RSA_PSS_ANY_SALT(PSA_ALG_SHA_256) OK OK
asym encrypt/decrypt
PSA_ALG_RSA_PKCS1V15_CRYPT OK
PSA_ALG_RSA_OAEP(PSA_ALG_SHA_256) OK

@mprse mprse added 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 22, 2021
@mprse mprse added component-psa PSA keystore/dispatch layer (storage, drivers, …) and removed needs-work labels Jan 3, 2022
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
…encrypt

Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
…decrypt

Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Tested key types:
PSA_ALG_RSA_PKCS1V15_SIGN_RAW
PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256)

Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Tested algs:
PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256)

Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
@gilles-peskine-arm gilles-peskine-arm removed the approved Design and code approved - may be waiting for CI or backports label Mar 7, 2022
* Remove expected_output_data: since asymmetric encryption is randomized,
  it can't be useful.
* The decryption check needs the private exponent, not the public exponent.
* Use PSA macro for the expected ciphertext buffer size.
* Move RSA sanity checks to their own function for clarity.
* For RSAES-PKCS1-v1_5, check that the result of the private key operation
  has the form 0x00 0x02 ... 0x00 M where M is the plaintext.
* For OAEP, check that the result of the private key operation starts with
  0x00. The rest is the result of masking which it would be possible to
  check here, but not worth the trouble of implementing.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests needs-review Every commit must be reviewed by at least two team members, and removed needs-work labels Mar 7, 2022
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
@mprse mprse removed the needs-ci Needs to pass CI tests label Mar 8, 2022
@mprse
Copy link
Contributor Author

mprse commented Mar 8, 2022

pr-merge passes. CI can be considered as passed.

psa_status_t forced_status_encrypt = forced_status_encrypt_arg;
psa_status_t expected_status_encrypt = expected_status_encrypt_arg;
psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
(void)expected_output_data;
Copy link
Contributor

Choose a reason for hiding this comment

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

space missing

mbedtls_mpi_init( &X );
#endif /* MBEDTLS_BIGNUM_C */

int ok = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe ret instead of ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

We use the convention ret for (0 = success, MBEDTLS_ERR_xxx = error, sometimes >0 = output size). For boolean return values (1 = success, 0 = failure), we use a different variable name. In the test suites it's typically ok.

Copy link
Contributor

@AndrzejKurek AndrzejKurek left a comment

Choose a reason for hiding this comment

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

Really minor changes suggested, but not required.

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Thanks for doing those last changes. Looks good to me!

Please backport the improvements related to testing with signature to 2.28. I'm going to merge this pull request without waiting for a backport to avoid bitrot.

@gilles-peskine-arm gilles-peskine-arm merged commit afb4828 into Mbed-TLS:development Mar 10, 2022
@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-psa PSA keystore/dispatch layer (storage, drivers, …)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Driver dispatch for PSA asymmetric encryption Driver dispatch tests for RSA
3 participants