8000 extmod/modussl_mbedtls: Fix support for ioctl(MP_STREAM_POLL). · micropython/micropython@ed58d6e · GitHub
[go: up one dir, main page]
Skip to content

Commit ed58d6e

Browse files
damzdamz
committed
extmod/modussl_mbedtls: Fix support for ioctl(MP_STREAM_POLL).
During the initial handshake or subsequent renegotiation, the protocol might need to read in order to write (or conversely to write in order to read). It might be blocked from doing so by the state of the underlying socket (i.e. there is no data to read, or there is no space to write). The library indicates this condition by returning one of the errors `MBEDTLS_ERR_SSL_WANT_READ` or `MBEDTLS_ERR_SSL_WANT_WRITE`. When that happens, we need to enforce that the next poll operation only considers the direction that the library indicated. In addition, mbedtls does its own read buffering that we need to take into account while polling, and we need to save the last error between read()/write() and ioctl().
1 parent 988b6e2 commit ed58d6e

File tree

5 files changed

+265
-2
lines changed

5 files changed

+265
-2
lines changed

‎extmod/modussl_mbedtls.c

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
#include "mbedtls/debug.h"
4747
#include "mbedtls/error.h"
4848

49+
#define MP_STREAM_POLL_RDWR (MP_STREAM_POLL_RD | MP_STREAM_POLL_WR)
50+
4951
typedef struct _mp_obj_ssl_socket_t {
5052
mp_obj_base_t base;
5153
mp_obj_t sock;
@@ -56,6 +58,9 @@ typedef struct _mp_obj_ssl_socket_t {
5658
mbedtls_x509_crt cacert;
5759
mbedtls_x509_crt cert;
5860
mbedtls_pk_context pkey;
61+
62+
uintptr_t poll_mask; // Indicates which read or write operations the protocol needs next
63+
int last_error; // The last error code, if any
5964
} mp_obj_ssl_socket_t;
6065

6166
struct ssl_args {
@@ -165,6 +170,8 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) {
165170
#endif
166171
o->base.type = &ussl_socket_type;
167172
o->sock = sock;
173+
o->poll_mask = 0;
174+
o->last_error = 0;
168175

169176
int ret;
170177
mbedtls_ssl_init(&o->ssl);
@@ -306,6 +313,12 @@ STATIC void socket_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kin
306313

307314
STATIC mp_uint_t socket_read(mp_obj_t o_in, void *buf, mp_uint_t size, int *errcode) {
308315
mp_obj_ssl_socket_t *o = MP_OBJ_TO_PTR(o_in);
316+
o->poll_mask = 0;
317+
318+
if (o->last_error) {
319+
*errcode = o->last_error;
320+
return MP_STREAM_ERROR;
321+
}
309322

310323
int ret = mbedtls_ssl_read(&o->ssl, buf, size);
311324
if (ret == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY) {
@@ -322,13 +335,22 @@ STATIC mp_uint_t socket_read(mp_obj_t o_in, void *buf, mp_uint_t size, int *errc
322335
// wanting to write next handshake message. The same may happen with
323336
// renegotation.
324337
ret = MP_EWOULDBLOCK;
338+
o->poll_mask = MP_STREAM_POLL_WR;
339+
} else {
340+
o->last_error = ret;
325341
}
326342
*errcode = ret;
327343
return MP_STREAM_ERROR;
328344
}
329345

330346
STATIC mp_uint_t socket_write(mp_obj_t o_in, const void *buf, mp_uint_t size, int *errcode) {
331347
mp_obj_ssl_socket_t *o = MP_OBJ_TO_PTR(o_in);
348+
o->poll_mask = 0;
349+
350+
if (o->last_error) {
351+
*errcode = o->last_error;
352+
return MP_STREAM_ERROR;
353+
}
332354

333355
int ret = mbedtls_ssl_write(&o->ssl, buf, size);
334356
if (ret >= 0) {
@@ -341,6 +363,9 @@ STATIC mp_uint_t socket_write(mp_obj_t o_in, const void *buf, mp_uint_t size, in
341363
// wanting to read next handshake message. The same may happen with
342364
// renegotation.
343365
ret = MP_EWOULDBLOCK;
366+
o->poll_mask = MP_STREAM_POLL_RD;
367+
} else {
368+
o->last_error = ret;
344369
}
345370
*errcode = ret;
346371
return MP_STREAM_ERROR;
@@ -358,17 +383,56 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(socket_setblocking_obj, socket_setblocking);
358383

359384
STATIC mp_uint_t socket_ioctl(mp_obj_t o_in, mp_uint_t request, uintptr_t arg, int *errcode) {
360385
mp_obj_ssl_socket_t *self = MP_OBJ_TO_PTR(o_in);
386+
mp_uint_t ret = 0;
387+
uintptr_t saved_arg = 0;
388+
mp_obj_t sock = self->sock;
389+
if (sock == MP_OBJ_NULL || (request != MP_STREAM_CLOSE && self->last_error != 0)) {
390+
// Closed or error socket:
391+
return MP_STREAM_POLL_NVAL;
392+
}
393+
361394
if (request == MP_STREAM_CLOSE) {
395+
self->sock = MP_OBJ_NULL;
362396
mbedtls_pk_free(&self->pkey);
363397
mbedtls_x509_crt_free(&self->cert);
364398
mbedtls_x509_crt_free(&self->cacert);
365399
mbedtls_ssl_free(&self->ssl);
366400
mbedtls_ssl_config_free(&self->conf);
367401
mbedtls_ctr_drbg_free(&self->ctr_drbg);
368402
mbedtls_entropy_free(&self->entropy);
403+
} else if (request == MP_STREAM_POLL) {
404+
// If the library signaled us that it needs reading or writing, only check that direction,
405+
// but save what the caller asked because we need to restore it later
406+
if (self->poll_mask && (arg & MP_STREAM_POLL_RDWR)) {
407+
saved_arg = arg & MP_STREAM_POLL_RDWR;
408+
arg = (arg & ~saved_arg) | self->poll_mask;
409+
}
410+
411+
// Take into account that the library might have buffered data already
412+
int has_pending = 0;
413+
if (arg & MP_STREAM_POLL_RD) {
414+
has_pending = mbedtls_ssl_check_pending(&self->ssl);
415+
if (has_pending) {
416+
ret |= MP_STREAM_POLL_RD;
417+
if (arg == MP_STREAM_POLL_RD) {
418+
// Shortcut if we only need to read and we have buffered data, no need to go to the underlying socket
419+
return MP_STREAM_POLL_RD;
420+
}
421+
}
422+
}
369423
}
424+
370425
// Pass all requests down to the underlying socket
371-
return mp_get_stream(self->sock)->ioctl(self->sock, request, arg, errcode);
426+
ret |= mp_get_stream(sock)->ioctl(sock, request, arg, errcode);
427+
428+
if (request == MP_STREAM_POLL) {
429+
// The direction the library needed is available, return a fake result to the caller so that
430+
// it reenters a read or a write to allow the handshake to progress
431+
if (ret & self->poll_mask) {
432+
ret |= saved_arg;
433+
}
434+
}
435+
return ret;
372436
}
373437

374438
STATIC const mp_rom_map_elem_t ussl_socket_locals_dict_table[] = {
@@ -381,6 +445,9 @@ STATIC const mp_rom_map_elem_t ussl_socket_locals_dict_table[] = {
381445
#if MICROPY_PY_USSL_FINALISER
382446
{ MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&mp_stream_close_obj) },
383447
#endif
448+
#if MICROPY_UNIX_COVERAGE
449+
{ MP_ROM_QSTR(MP_QSTR_ioctl), MP_ROM_PTR(&mp_stream_ioctl_obj) },
450+
#endif
384451
{ MP_ROM_QSTR(MP_QSTR_getpeercert), MP_ROM_PTR(&mod_ssl_getpeercert_obj) },
385452
};
386453

‎tests/extmod/ussl_basic.py.exp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,5 @@ OSError: client
33
TestSocket.setblocking(False)
44
TestSocket.setblocking(True)
55
TestSocket.ioctl 4 0
6-
TestSocket.ioctl 4 0
76
OSError: read
87
OSError: write

‎tests/extmod/ussl_poll.py

Lines changed: 196 additions & 0 deletions
9241
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
try:
2+
import uselect
3+
import ussl
4+
import io
5+
import ubinascii as binascii
6+
except ImportError:
7+
print("SKIP")
8+
raise SystemExit
9+
10+
from micropython import const
11+
12+
_MP_STREAM_POLL_RD = const(0x0001)
13+
_MP_STREAM_POLL_WR = const(0x0004)
14+
_MP_STREAM_POLL_NVAL = const(0x0020)
15+
_MP_STREAM_POLL = const(3)
16+
_MP_STREAM_CLOSE = const(4)
17+
18+
19+
# This self-signed key/cert pair is randomly generated and to be used for
20+
# testing/demonstration only. You should always generate your own key/cert.
21+
key = binascii.unhexlify(
22+
b"3082013b020100024100cc20643fd3d9c21a0acba4f48f61aadd675f52175a9dcf07fbef"
23+
b"610a6a6ba14abb891745cd18a1d4c056580d8ff1a639460f867013c8391cdc9f2e573b0f"
24+
b"872d0203010001024100bb17a54aeb3dd7ae4edec05e775ca9632cf02d29c2a089b563b0"
25+
b"d05cdf95aeca507de674553f28b4eadaca82d5549a86058f9996b07768686a5b02cb240d"
26+
b"d9f1022100f4a63f5549e817547dca97b5c658038e8593cb78c5aba3c4642cc4cd031d86"
27+
b"8f022100d598d870ffe4a34df8de57047a50b97b71f4d23e323f527837c9edae88c79483"
28+
b"02210098560c89a70385c36eb07fd7083235c4c1184e525d838aedf7128958bedfdbb102"
29+
b"2051c0dab7057a8176ca966f3feb81123d4974a733df0f958525f547dfd1c271f9022044"
30+
b"6c2cafad455a671a8cf398e642e1be3b18a3d3aec2e67a9478f83c964c4f1f"
31+
)
32+
cert = binascii.unhexlify(
33+
b"308201d53082017f020203e8300d06092a864886f70d01010505003075310b3009060355"
34+
b"0406130258583114301206035504080c0b54686550726f76696e63653110300e06035504"
35+
b"070c075468654369747931133011060355040a0c0a436f6d70616e7958595a3113301106"
36+
b"0355040b0c0a436f6d70616e7958595a3114301206035504030c0b546865486f73744e61"
37+
b"6d65301e170d3139313231383033333935355a170d3239313231353033333935355a3075"
38+
b"310b30090603550406130258583114301206035504080c0b54686550726f76696e636531"
39+
b"10300e06035504070c075468654369747931133011060355040a0c0a436f6d70616e7958"
40+
b"595a31133011060355040b0c0a436f6d70616e7958595a3114301206035504030c0b5468"
41+
b"65486f73744e616d65305c300d06092a864886f70d0101010500034b003048024100cc20"
42+
b"643fd3d9c21a0acba4f48f61aadd675f52175a9dcf07fbef610a6a6ba14abb891745cd18"
43+
b"a1d4c056580d8ff1a639460f867013c8391cdc9f2e573b0f872d0203010001300d06092a"
44+
b"864886f70d0101050500034100b0513fe2829e9ecbe55b6dd14c0ede7502bde5d46153c8"
45+
b"e960ae3ebc247371b525caeb41bbcf34686015a44c50d226e66aef0a97a63874ca5944ef"
46+
b"979b57f0b3"
47+
)
48+
49+
50+
class _Pipe(io.IOBase):
51+
def __init__(self):
52+
self._other = None
53+
self.block_reads = False
54+
self.block_writes = False
55+
56+
self.write_buffers = []
57+
self.last_poll_arg = None
58+
59+
def readinto(self, buf):
60+
if self.block_reads or len(self._other.write_buffers) == 0:
61+
return None
62+
63+
read_buf = self._other.write_buffers[0]
64+
l = min(len(buf), len(read_buf))
65+
buf[:l] = read_buf[:l]
66+
if l == len(read_buf):
67+
self._other.write_buffers.pop(0)
68+
else:
69+
self._other.write_buffers[0] = read_buf[l:]
70+
return l
71+
72+
def write(self, buf):
73+
if self.block_writes:
74+
return None
75+
76+
self.write_buffers.append(memoryview(bytes(buf)))
77+
return len(buf)
78+
79+
def ioctl(self, request, arg):
80+
if request == _MP_STREAM_POLL:
81+
self.last_poll_arg = arg
82+
ret = 0
83+
if arg & _MP_STREAM_POLL_RD:
84+
if not self.block_reads and self._other.write_buffers:
85+
ret |= _MP_STREAM_POLL_RD
86+
if arg & _MP_STREAM_POLL_WR:
87+
if not self.block_writes:
88+
ret |= _MP_STREAM_POLL_WR
89+
return ret
90+
91+
elif request == _MP_STREAM_CLOSE:
92+
return 0
93+
94+
raise NotImplementedError()
95+
96+
@classmethod
97+
def new_pair(cls):
98+
p1 = cls()
99+
p2 = cls()
100+
p1._other = p2
101+
p2._other = p1
102+
return p1, p2
103+
104+
105+
def assert_poll(s, i, arg, expected_arg, expected_ret):
106+
ret = s.ioctl(_MP_STREAM_POLL, arg)
107+
assert i.last_poll_arg == expected_arg
108+
i.last_poll_arg = None
109+
assert ret == expected_ret
110+
111+
112+
def assert_raises(cb, *args, **kwargs):
113+
try:
114+
cb(*args, **kwargs)
115+
raise AssertionError("should have raised")
116+
except Exception as exc:
117+
pass
118+
119+
120+
client_io, server_io = _Pipe.new_pair()
121+
122+
client_io.block_reads = True
123+
client_io.block_writes = True
124+
client_sock = ussl.wrap_socket(client_io, do_handshake=False)
125+
126+
server_sock = ussl.wrap_socket(server_io, key=key, cert=cert, server_side=True, do_handshake=False)
127+
128+
# Do a test read, at this point the TLS handshake wants to write,
129+
# so it returns None:
130+
assert client_sock.read(128) is None
131+
132+
# Polling for either read or write actually check if the underlying socket can write:
133+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_RD, _MP_STREAM_POLL_WR, 0)
134+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_WR, _MP_STREAM_POLL_WR, 0)
135+
136+
# Mark the socket as writable, and do another test read:
137+
client_io.block_writes = False
138+
assert client_sock.read(128) is None
139+
140+
# The client wrote the CLIENT_HELLO message
141+
assert len(client_io.write_buffers) == 1
142+
143+
# At this point the TLS handshake wants to read, but we don't know that yet:
144+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_RD, _MP_STREAM_POLL_RD, 0)
145+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_WR, _MP_STREAM_POLL_WR, _MP_STREAM_POLL_WR)
146+
147+
# Do a test write
148+
client_sock.write(b"foo")
149+
150+
# Now we know that we want to read:
151+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_RD, _MP_STREAM_POLL_RD, 0)
152+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_WR, _MP_STREAM_POLL_RD, 0)
153+
154+
# Unblock reads and nudge the two sockets:
155+
client_io.block_reads = False
156+
while server_io.write_buffers or client_io.write_buffers:
157+
if server_io.write_buffers:
158+
assert client_sock.read(128) is None
159+
if client_io.write_buffers:
160+
assert server_sock.read(128) is None
161+
162+
# At this point, the handshake is done, try writing data:
163+
client_sock.write(b"foo")
164+
assert server_sock.read(3) == b"foo"
165+
166+
# Test reading partial data:
167+
client_sock.write(b"foobar")
168+
assert server_sock.read(3) == b"foo"
169+
server_io.block_reads = True
170+
assert_poll(
171+
server_sock, server_io, _MP_STREAM_POLL_RD, None, _MP_STREAM_POLL_RD
172+
) # Did not go to the socket, just consumed buffered data
173+
assert server_sock.read(3) == b"bar"
174+
175+
176+
# Polling on a closed socket errors out:
177+
client_io, _ = _Pipe.new_pair()
178+
client_sock = ussl.wrap_socket(client_io, do_handshake=False)
179+
client_sock.close()
180+
assert_poll(
181+
client_sock, client_io, _MP_STREAM_POLL_RD, None, _MP_STREAM_POLL_NVAL
182+
) # Did not go to the socket
183+
184+
185+
# Errors propagates to poll:
186+
client_io, server_io = _Pipe.new_pair()
187+
client_sock = ussl.wrap_socket(client_io, do_handshake=False)
188+
189+
# The server returns garbage:
190+
server_io.write(b"fooba") # Needs to be exactly 5 bytes
191+
192+
assert_poll(client_sock, client_io, _MP_STREAM_POLL_RD, _MP_STREAM_POLL_RD, _MP_STREAM_POLL_RD)
193+
assert_raises(client_sock.read, 128)
194+
assert_poll(
195+
client_sock, client_io, _MP_STREAM_POLL_RD, None, _MP_STREAM_POLL_NVAL
196+
) # Did not go to the socket

‎tests/extmod/ussl_poll.py.exp

Whitespace-only changes.

‎tests/run-tests.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,7 @@ def run_tests(pyb, tests, args, result_dir, num_threads=1):
541541
if not has_coverage:
542542
skip_tests.add("cmdline/cmd_parsetree.py")
543543
skip_tests.add("cmdline/repl_sys_ps1_ps2.py")
544+
skip_tests.add("extmod/ussl_poll.py")
544545

545546
# Some tests shouldn't be run on a PC
546547
if args.target == "unix":

0 commit comments

Comments
 (0)
0