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

Simplify non-error errors in TLS #9516

Open
gilles-peskine-arm opened this issue Aug 29, 2024 · 0 comments
Open

Simplify non-error errors in TLS #9516

gilles-peskine-arm opened this issue Aug 29, 2024 · 0 comments
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-tls component-tls13 enhancement size-m Estimated task size: medium (~1w)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Aug 29, 2024

TLS functions can return “non-error errors” that do not invalidate the context, and instead tell the caller that they should do something else before calling the function again:

  • MBEDTLS_ERR_SSL_WANT_READ — wait until data is received on the underlying transport.
  • MBEDTLS_ERR_SSL_WANT_WRITE — wait until data can be sent on the underlying transport.
  • MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS — wait until the external cryptoprocessor replies to the asynchronous request.
  • MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS — wait until you feel comfortable spending more CPU time.
  • MBEDTLS_ERR_SSL_RECEIVED_EARLY_DATA — go and read some early data first.
  • MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET — go and save the session first.
  • MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO — ???

This interface is error-prone, because new non-error errors are added to the API from time to time, so there's no definitive way to know whether a nonzero return value indicates that the SSL context has been invalidated. Not all applications care about the details: for example, many applications can just call select() and handle whatever is ready, then try the SSL function again, without caring precisely what the SSL function complained about.

The goal of this issue is to devise an interface that's safer and easier to use.

This overlaps #8501 in that all of these errors are a close match to PSA_OPERATION_INCOMPLETE. MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS is exactly PSA_OPERATION_INCOMPLETE (call again when you want to spend the CPU time), while the others are a variant of it (call again when some subsystem is ready). A possible solution would be to always return PSA_OPERATION_INCOMPLETE and have a field in the SSL context indicating what needs to be done. Then we can have the property that any return value other than PSA_SUCCESS and PSA_OPERATION_INCOMPLETE (and perhaps a third code PSA_OPERATION_BLOCKED) is an error.

For MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET and MBEDTLS_ERR_SSL_RECEIVED_EARLY_DATA, we should not return an error, but call a callback function instead, because the thing that needs to be done is non-blocking. See #6640 and #8749 for NewSessionTicket handling.

My very preliminary size estimate is M for the amount of library code (not much), documentation (comparatively quite a bit) and test code to change. This includes the overlap with #8501 which calls for a general overhaul of error codes.

We've talked in the past of keeping track whether a context has been invalidated, and both checking it inside the library and having a public function to check. This is complementary but out of scope here since it can be done without breaking the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-tls component-tls13 enhancement size-m Estimated task size: medium (~1w)
Projects
Status: Design needed
Development

No branches or pull requests

1 participant