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

PSA crypto - global variable - no concurrency #7946

Closed
lhuang04 opened this issue Jul 18, 2023 · 6 comments
Closed

PSA crypto - global variable - no concurrency #7946

lhuang04 opened this issue Jul 18, 2023 · 6 comments
Labels
needs-info An issue or PR which needs further info from the reporter / author

Comments

@lhuang04
Copy link
Contributor

Suggested enhancement

Per suggestion from @paul-elliott-arm, open a separate issue here to track the ones discussed in global variable issue in PSA crypto.

Justification

Mbed TLS needs this because

@gilles-peskine-arm
Copy link
Contributor

Can you please clarify what the requirement is here? I have no idea what you mean by “there is only one slot for a volatile key”.

We already have an issue to track protecting access to keys from concurrent threads.

@gilles-peskine-arm gilles-peskine-arm added the needs-info An issue or PR which needs further info from the reporter / author label Jul 18, 2023
@lhuang04
Copy link
Contributor Author

I have no idea what you mean by “there is only one slot for a volatile key”.

Sorry, I was in the wrong expression that all libraries that use mbedtls will access one single volatile key slot. It sounds like the slot is per key.

Can you please clarify what the requirement is here?

I just want to counter the point in issue 3263

In a nutshell, it's up to the application to not use operation objects concurrently,

As I explained in issue 77. Our application does want to use operation objects concurrently. We have multiple libraries in our app that use mbedtls for different work loads.

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Jul 31, 2023

Ok, but I'm still not sure exactly what you want. Which scenario(s) do you want?

  1. Thread A uses the PSA API and creates a key. Thread B uses the PSA API and creates a key.
    The PSA API specification states that this should work, but it's a known bug that it doesn't work in Mbed TLS . We intend to fix in Mbed TLS 3.6.
    A partial workaround is to put your own mutex around key creation and destruction, as well as around driver calls if you have drivers (including an external RNG).

  2. Thread A uses the API API and creates a key. It passes the handle of that key to other threads which use the key.
    The PSA API specification states that this should work. In Mbed TLS, we don't yet guarantee anything, but in practice it does work. This will be fixed when (1) is fixed.

  3. Thread A uses the PSA API and starts a multipart operation. Thread A passes the multipart operation object to thread B which continues the operation.
    The PSA API specification states that this should work, provided that the threads don't try to access the operation object concurrently. In Mbed TLS, we don't yet guarantee anything, but in practice it does work. This will be fixed when (1) is fixed.

  4. Thread A uses the PSA API and starts a multipart operation. Thread A passes the multipart operation object to thread B. Threads A and B use the multipart operation object without doing any explicit synchronization.
    I can't think of a scenario where this would be useful, since there's no control over the order in which the threads access the multipart operation.

  5. Something else?

@lhuang04
Copy link
Contributor Author

lhuang04 commented Jul 31, 2023

Which scenario(s) do you want?

We should be mostly in case 1. We don't have cases that a key will be passed from one thread to another. And we do need randomness. So we may need the fix for issue 3263 and issue 3391.

@gilles-peskine-arm
Copy link
Contributor

We should be mostly in case 1. We don't have cases that a key will be passed from one thread to another.

So this issue is a duplicate of #3263 then?

And we do need randomness

My bad, I was much more pessimistic about the current situation in my previous message. Use of randomness is ok if the random generator itself is protected, and this is the case as long as you enable MBEDTLS_THREADING_C. We're only missing automatic protection around a custom RNG if there is one.

Also, I forgot to mention a scenario: multiple threads doing the initial call to psa_crypto_init at the same time. That one is not included in #3263. It's difficult to resolve portably because there's no portable thread-safe way to initialize a mutex.

@paul-elliott-arm
Copy link
Member

Closing as I think this is resolved - please re-open or open a new ticket if necessary,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-info An issue or PR which needs further info from the reporter / author
Projects
None yet
Development

No branches or pull requests

3 participants