Skip to content

mbedtls: call psa_crypto_init() in the AES-GCM and HMAC init paths - #826

Open
vikramdattu wants to merge 1 commit into
cisco:mainfrom
vikramdattu:fix/mbedtls-psa-crypto-init
Open

vikramdattu wants to merge 1 commit into
cisco:mainfrom
vikramdattu:fix/mbedtls-psa-crypto-init

Conversation

@vikramdattu

Copy link
Copy Markdown
Contributor

In the mbedTLS (PSA) backend, only srtp_aes_icm_mbedtls_context_init() calls psa_crypto_init(). srtp_aes_gcm_mbedtls_context_init() and srtp_hmac_mbedtls_init() rely on it having run already.

Through the public API that always holds: srtp_init() self-tests AES-ICM before AES-GCM and HMAC, and that self-test initializes PSA. Using either type without srtp_init() (for example from a test harness or a custom crypto kernel setup) fails, because the key import runs before PSA is initialized:

  • AES-GCM context init returns srtp_err_status_init_fail.
  • HMAC init returns srtp_err_status_auth_fail.

This calls psa_crypto_init() in both, at the same point and with the same error handling as AES-ICM. It is idempotent, so the srtp_init() path is unchanged. This brings main in line with 2_x_dev, whose mbedTLS 4 / PSA path (#813) already calls it in all three backends.

Test: mbedtls_init_driver (mbedTLS builds only, CMake and meson) initializes both types without srtp_init(). It is a separate executable because PSA initialization is process-wide: inside test_srtp, an earlier test would already have initialized it.

It fails without the fix and passes with it. Tested on mbedTLS 4.1.0: all 14 ctest tests pass with ENABLE_WARNINGS_AS_ERRORS=ON, and all 14 meson tests pass.

Only the AES-ICM backend initialized PSA. AES-GCM and HMAC relied on it
having run first, which srtp_init() guarantees by self-testing AES-ICM
before them. Using either type without srtp_init() made the key import
fail: AES-GCM init returned srtp_err_status_init_fail and HMAC init
srtp_err_status_auth_fail.

Call psa_crypto_init() in both, as AES-ICM does. It is idempotent.
2_x_dev already does this (cisco#813).

Add mbedtls_init_driver, which initializes both types without
srtp_init() in its own process.
@vikramdattu

Copy link
Copy Markdown
Contributor Author

@pabuhler PTAL

@pabuhler pabuhler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As you state this code is not really needed at present as there is no public API that needs these extra inits.
I think it is ok to add it but am not sure if the test is needed, it creates overhead for something that will get caught if any public API is ever added.
Also the test only really tests one of the invocations, would need to run it twice once for HMAC and once for GCM.
I would suggest removing it or else fixing it, if you fix then please include top of file comments similar to other files in the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants