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

mbedtls and detecting non-blocking reads during handshaking #424

Closed
nkolban opened this issue Mar 13, 2017 · 2 comments
Closed

mbedtls and detecting non-blocking reads during handshaking #424

nkolban opened this issue Mar 13, 2017 · 2 comments

Comments

@nkolban
Copy link
Contributor

nkolban commented Mar 13, 2017

If we attempt to use a non-blocking socket with mbedtls we receive a failure. I dug deeply into this and believe I have found the resolution.

If we look at this function:

https://github.com/espressif/esp-idf/blob/master/components/mbedtls/port/net.c#L204

The intent is to determine if an error reported by a previous socket call was caused because the socket is non blocking. The return code is 0 if the last error was NOT caused by being a non-blocking socket while the return code is 1 if the last error was because we WERE a non-blocking socket. The logic is:

  1. If the socket is a blocking socket , return 0
  2. If the last error was because we were non-blocking, return 1.
  3. Else ... return 0

This sound sensible enough, however there is perhaps a flaw in the implementation. The we check "if the last error was because we were non-blocking" is to ASK the socket about its last error. We don't use errno because we are multi-threaded and premptable. Again, on the surface, that took makes sense. However in test number 1, we perform an fcntl against the socket to determine if it is blocking or non-blocking ... and that resets the error condition that was present on the socket when the function was called. What this means is that the original error is lost.

@projectgus
Copy link
Contributor

projectgus commented Mar 13, 2017

Thanks @nkolban, this looks good (and correct) to me.

Are you able to please rewrite the commit message so it summarises the fix? something like:

mbedtls port: Fix non-blocking socket erro detection

For knowing when to return MBEDTLS_ERR_SSL_WANT_READ/MBEDTLS_ERR_SSL_WANT_WRITE.
Checking non-blocking socket status clears errno, so save errno first.

Closes #424

Would be ideal. If you're not comfortable rewriting git history in this way then that's OK, I can rewrite the message when I push it into our internal merge queue.

@nkolban
Copy link
Contributor Author

nkolban commented Mar 13, 2017

Sorry sir, I'm not good at Git ... :-(

@projectgus projectgus added the Status: In Progress Work is in progress label Mar 13, 2017
@igrr igrr closed this as completed in 16e1a27 Mar 15, 2017
igrr added a commit that referenced this issue Mar 15, 2017
mbedtls port: Fix detection of EWOULDBLOCK/EAGAIN with non-blocking sockets

Previous code read non-blocking status via fcntl first, which resets errno.

* Closes #424 #424
* Merges #425 #425

See merge request !575
@igrr igrr removed the Status: In Progress Work is in progress label Dec 14, 2017
turmary pushed a commit to Seeed-Studio/Seeed_Arduino_mbedtls that referenced this issue Jan 22, 2020
…ockets

Previous code read non-blocking status via fcntl first, which resets errno.

Closes #424 espressif/esp-idf#424
Merges #425 espressif/esp-idf#425
turmary pushed a commit to Seeed-Studio/Seeed_Arduino_mbedtls that referenced this issue Jan 22, 2020
mbedtls port: Fix detection of EWOULDBLOCK/EAGAIN with non-blocking sockets

Previous code read non-blocking status via fcntl first, which resets errno.

* Closes #424 espressif/esp-idf#424
* Merges #425 espressif/esp-idf#425

See merge request !575
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

No branches or pull requests

3 participants