Descriptor range check fails in Winsock

[hkluis]

---

Description

• Type: Bug
• Priority: Major

---

Bug

The recently added range check in net_sockets.c on the socket descriptor fd fails in Windows because under winsock, the socket descriptor is a handle, not a file descriptor. See description in https://docs.microsoft.com/en-us/windows/win32/winsock/socket-data-type-2.

OS
windows

mbed TLS build:
Version: 2.16.10 git ddf4374
OS version: Windows 10 19042.964
Configuration:
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
VisualStudio 2017, C/C++ project, but the version doesn't matter.

Peer device TLS stack and version
any
Version:
any

Expected behavior
TLS connection should succeed

Actual behavior
TLS connection fails because MBEDTLS_ERR_NET_POLL_FAILED is returned from mbedtls_net_poll() and mbedtls_net_recv_timeout()

Steps to reproduce
Build library using VisualStudio 2017 and connect to server running mbedtls

Suggested fix
Exclude range check when compiling under winsock:
#if !( defined(WINSOCKAPI) /* winsock fd is a handle, not file descriptor */
if( fd >= FD_SETSIZE )
return( MBEDTLS_ERR_NET_POLL_FAILED );
#endif

Alternatively, include the range check only under those platforms where the socket descriptor is known to be an actual file descriptor.

---

[n00bmind]

Seeing the same issue in v.2.26.0 compiled under VS 2019.
This makes the library unusable for me!

[gilles-peskine-arm]

We'll try to have the fix in the next release. Since our CI doesn't test this, can you please confirm that the patch in #4688 (2.26) or #4689 (2.16) works for you?

[n00bmind]

Thanks for addressing this promptly.
However, I don't see any special casing for Windows in the new check_fd function (no checking for WINSOCKAPI as suggested by @hkluis), so I'd expect this to still fail!

[gilles-peskine-arm]

Ooops. I did all but the actual bug fix. Thanks, please try the updated version.

I used the same condition that we use to decide whether to #include <winsock2.h>. Is this wrong? Are there cases where (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(EFIX64) && !defined(EFI32), and where a winsock2.h header exists, but a socket is a small integer and fd_set has limited range?

[hkluis]

Either way would work, but defined(WINSOCKAPI) is more self-documenting since it is the WinSock API, not WIN32 per se, that defines the sock()'s return value to be a handle rather than file descriptor. So the equivalent check is (defined(WINSOCKAPI) && !defined(EFIX64) && !defined(EFI32)). I don't think there would be a case of including winsock2.h but a socket return that is incompatible with what is defined in that file.

[n00bmind]

A bit late to this.. from what I can see the changes are still using _WIN32, not WINSOCKAPI. Just wanted to point out that the latter isn't actually defined anywhere, afaict, in case anyone decides to use it.
It should be _WINSOCKAPI_, with underscores. Both winsock.h and winsock2.h define this.

[hkluis]

Correct. It should have been WINSOCKAPI in my original report.