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

TLS deliver buffer data during shutdown #650

Merged
merged 55 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
deccd6b
add test
TingDaoK Jun 27, 2024
113082f
a lot of lines
TingDaoK Jun 28, 2024
0034c24
use server lock
TingDaoK Jun 28, 2024
7219fea
woops... typo
TingDaoK Jun 28, 2024
0b7082b
make sure it shutdown
TingDaoK Jun 28, 2024
d90a40f
looks like tsan is not happy, let's notify within the lock
TingDaoK Jun 28, 2024
b2cad31
one possible way to handle this?
TingDaoK Jun 28, 2024
320272f
Mac & linux
TingDaoK Jul 9, 2024
a8ab76a
make sure server and client don't fall into the same thread
TingDaoK Jul 9, 2024
e8eccf3
windows
TingDaoK Jul 9, 2024
5277e9b
fix the build, but test finds another issue🥲
TingDaoK Jul 10, 2024
80a887c
Merge branch 'main' into socket-close
TingDaoK Jul 10, 2024
b9702f7
flush for all error code
TingDaoK Jul 10, 2024
4fcf74b
Merge branch 'socket-close' of github.com:awslabs/aws-c-io into socke…
TingDaoK Jul 10, 2024
ca84785
not socket erro
TingDaoK Jul 10, 2024
cd0a9ab
unless we finished negotiation
TingDaoK Jul 10, 2024
4fb3bde
kick off read
TingDaoK Jul 10, 2024
35ba824
fix build
TingDaoK Jul 10, 2024
0f9d185
Merge branch 'main' into socket-close
TingDaoK Jul 10, 2024
4f7cc91
read for all platforms
TingDaoK Jul 10, 2024
1b9c79c
fix build
TingDaoK Jul 10, 2024
e6a9caa
Merge branch 'main' into socket-close
TingDaoK Jul 11, 2024
f723101
break out inside the loop
TingDaoK Jul 12, 2024
4ea14b6
use latest builder?
TingDaoK Jul 12, 2024
c6bd4c8
unused line
TingDaoK Jul 12, 2024
bc91df5
only do the hack if the downstream window is larger than 0
TingDaoK Jul 12, 2024
7c1bf10
allow people to increment window during shutting down
TingDaoK Jul 12, 2024
3ad0547
Merge branch 'main' into socket-close
TingDaoK Jul 12, 2024
47ebf48
make sure it works after shutdown as well
TingDaoK Jul 12, 2024
2303ecc
adjust a bit
TingDaoK Jul 12, 2024
db3b36c
comments addressed
TingDaoK Jul 17, 2024
2cd59af
one more
TingDaoK Jul 17, 2024
f23300e
bring back the check for the next handler
TingDaoK Jul 17, 2024
e1ebefb
address comments and fix the skip
TingDaoK Jul 18, 2024
fa209e7
fix build
TingDaoK Jul 18, 2024
9f3bd93
revert the change
TingDaoK Jul 18, 2024
086eb7f
windows as well
TingDaoK Jul 18, 2024
6d02832
Merge branch 'main' into socket-close
TingDaoK Jul 18, 2024
bbf9fd1
format
TingDaoK Jul 18, 2024
1234fbc
man, i cannot write code without ide
TingDaoK Jul 18, 2024
f387e52
Apply suggestions from code review
TingDaoK Jul 24, 2024
ea1c052
comments addressed
TingDaoK Jul 24, 2024
cee28ae
protect against read after shutdown
TingDaoK Jul 24, 2024
a40c650
return a value
TingDaoK Jul 24, 2024
3c9261d
address comments
TingDaoK Jul 24, 2024
2c250e1
read state
TingDaoK Jul 24, 2024
8f212e5
fix build
TingDaoK Jul 24, 2024
9867c55
comments addressed
TingDaoK Jul 24, 2024
88dafd8
Merge branch 'main' into socket-close
TingDaoK Jul 24, 2024
f918526
try to make the loop more clear
TingDaoK Jul 25, 2024
e742c5a
oops
TingDaoK Jul 25, 2024
05b88dd
try to simplify some error-handling
graebm Jul 26, 2024
9d04799
fix my mistakes
graebm Jul 26, 2024
42d4400
trrrrivial
graebm Jul 26, 2024
463c563
respond to feedback
graebm Jul 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/aws/io/private/tls_channel_handler_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ struct aws_tls_channel_handler_shared {
struct aws_crt_statistics_tls stats;
};

enum aws_tls_handler_read_state {
AWS_TLS_HANDLER_OPEN,
AWS_TLS_HANDLER_READ_SHUTTING_DOWN,
AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE,
};

AWS_EXTERN_C_BEGIN

AWS_IO_API void aws_tls_channel_handler_shared_init(
Expand Down
20 changes: 11 additions & 9 deletions source/darwin/secure_transport_tls_channel_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ struct secure_transport_handler {
bool negotiation_finished;
bool verify_peer;
bool read_task_pending;
bool delay_shutdown_scheduled;
enum aws_tls_handler_read_state read_state;
int delay_shutdown_error_code;
bool read_shutdown_completed;
};

static OSStatus s_read_cb(SSLConnectionRef conn, void *data, size_t *len) {
Expand Down Expand Up @@ -574,7 +573,7 @@ static void s_initialize_read_delay_shutdown(
" Your application may hang if the read window never opens",
(void *)handler);
}
secure_transport_handler->delay_shutdown_scheduled = true;
secure_transport_handler->read_state = AWS_TLS_HANDLER_READ_SHUTTING_DOWN;
secure_transport_handler->delay_shutdown_error_code = error_code;
if (!secure_transport_handler->read_task_pending) {
/* Kick off read, in case data arrives with TLS negotiation. Shutdown starts right after negotiation.
Expand Down Expand Up @@ -604,7 +603,7 @@ static int s_handle_shutdown(
* data. */
return AWS_OP_SUCCESS;
}
secure_transport_handler->read_shutdown_completed = true;
secure_transport_handler->read_state = AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE;
} else {
/* Shutdown in write direction */
TingDaoK marked this conversation as resolved.
Show resolved Hide resolved
if (!abort_immediately && error_code != AWS_IO_SOCKET_CLOSED) {
Expand All @@ -627,7 +626,10 @@ static int s_process_read_message(
struct aws_io_message *message) {

struct secure_transport_handler *secure_transport_handler = handler->impl;
if (secure_transport_handler->read_shutdown_completed) {
if (secure_transport_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
if (message) {
aws_mem_release(message->allocator, message);
}
return AWS_OP_SUCCESS;
TingDaoK marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -695,7 +697,7 @@ static int s_process_read_message(

switch (status) {
case errSSLWouldBlock:
if (secure_transport_handler->delay_shutdown_scheduled) {
if (secure_transport_handler->read_state == AWS_TLS_HANDLER_READ_SHUTTING_DOWN) {
/* Propagate the shutdown as we blocked now. */
goto shutdown_channel;
} else {
Expand Down Expand Up @@ -726,13 +728,13 @@ static int s_process_read_message(
return AWS_OP_SUCCESS;

shutdown_channel:
if (secure_transport_handler->delay_shutdown_scheduled) {
if (secure_transport_handler->read_state == AWS_TLS_HANDLER_READ_SHUTTING_DOWN) {
if (secure_transport_handler->delay_shutdown_error_code != 0) {
/* Propagate the original error code if it is set. */
shutdown_error_code = secure_transport_handler->delay_shutdown_error_code;
}
/* Continue the shutdown process delayed before. */
secure_transport_handler->read_shutdown_completed = true;
secure_transport_handler->read_state = AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE;
aws_channel_slot_on_handler_shutdown_complete(slot, AWS_CHANNEL_DIR_READ, shutdown_error_code, false);
} else {
/* Starts the shutdown process */
Expand All @@ -753,7 +755,7 @@ static void s_run_read(struct aws_channel_task *task, void *arg, aws_task_status

static int s_increment_read_window(struct aws_channel_handler *handler, struct aws_channel_slot *slot, size_t size) {
struct secure_transport_handler *secure_transport_handler = handler->impl;
if (secure_transport_handler->read_shutdown_completed) {
if (secure_transport_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
return AWS_OP_SUCCESS;
}

Expand Down
21 changes: 11 additions & 10 deletions source/s2n/s2n_tls_channel_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,9 @@ struct s2n_handler {
} state;
struct aws_channel_task read_task;
bool read_task_pending;

bool is_shutting_down;
enum aws_tls_handler_read_state read_state;
int shutdown_error_code;
struct aws_channel_task delayed_shutdown_task;
bool read_shutdown_completed;
};

struct s2n_ctx {
Expand Down Expand Up @@ -523,7 +521,10 @@ static int s_s2n_handler_process_read_message(

struct s2n_handler *s2n_handler = handler->impl;

if (s2n_handler->read_shutdown_completed) {
if (s2n_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
if (message) {
aws_mem_release(message->allocator, message);
}
return AWS_OP_SUCCESS;
TingDaoK marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -590,7 +591,7 @@ static int s_s2n_handler_process_read_message(

/* the socket blocked so exit from the loop */
if (s2n_error_get_type(s2n_errno) == S2N_ERR_T_BLOCKED) {
if (s2n_handler->is_shutting_down) {
if (s2n_handler->read_state == AWS_TLS_HANDLER_READ_SHUTTING_DOWN) {
/* Propagate the shutdown as we blocked now. */
goto shutdown_channel;
TingDaoK marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -632,12 +633,12 @@ static int s_s2n_handler_process_read_message(
return AWS_OP_SUCCESS;

shutdown_channel:
if (s2n_handler->is_shutting_down) {
if (s2n_handler->read_state == AWS_TLS_HANDLER_READ_SHUTTING_DOWN) {
if (s2n_handler->shutdown_error_code != 0) {
/* Propagate the original error code if it is set. */
shutdown_error_code = s2n_handler->shutdown_error_code;
}
s2n_handler->read_shutdown_completed = true;
s2n_handler->read_state = AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE;
aws_channel_slot_on_handler_shutdown_complete(slot, AWS_CHANNEL_DIR_READ, shutdown_error_code, false);
TingDaoK marked this conversation as resolved.
Show resolved Hide resolved
} else {
/* Starts the shutdown process */
Expand Down Expand Up @@ -1046,7 +1047,7 @@ static void s_initialize_read_delay_shutdown(
" Your application may hang if the read window never opens",
(void *)handler);
}
s2n_handler->is_shutting_down = true;
s2n_handler->read_state = AWS_TLS_HANDLER_READ_SHUTTING_DOWN;
s2n_handler->shutdown_error_code = error_code;
if (!s2n_handler->read_task_pending) {
/* Kick off read, in case data arrives with TLS negotiation. Shutdown starts right after negotiation.
Expand Down Expand Up @@ -1080,7 +1081,7 @@ static int s_s2n_handler_shutdown(
s_initialize_read_delay_shutdown(handler, slot, error_code);
return AWS_OP_SUCCESS;
}
s2n_handler->read_shutdown_completed = true;
s2n_handler->read_state = AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE;
} else {
/* Shutdown in write direction */
if (!abort_immediately && error_code != AWS_IO_SOCKET_CLOSED) {
Expand All @@ -1105,7 +1106,7 @@ static int s_s2n_handler_increment_read_window(
size_t size) {
(void)size;
struct s2n_handler *s2n_handler = handler->impl;
if (s2n_handler->read_shutdown_completed) {
if (s2n_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
return AWS_OP_SUCCESS;
}

Expand Down
38 changes: 24 additions & 14 deletions source/windows/secure_channel_tls_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ struct secure_channel_handler {
bool verify_peer;
struct aws_channel_task read_task;
bool read_task_pending;
bool delay_shutdown_scheduled;
int delay_shutdown_error_code;
bool read_shutdown_completed;
enum aws_tls_handler_state read_state;
int shutdown_error_code;
};

static size_t s_message_overhead(struct aws_channel_handler *handler) {
Expand Down Expand Up @@ -1164,7 +1163,7 @@ static int s_do_application_data_decrypt(struct aws_channel_handler *handler) {

static int s_process_pending_output_messages(struct aws_channel_handler *handler) {
struct secure_channel_handler *sc_handler = handler->impl;
if (sc_handler->read_shutdown_completed) {
if (sc_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
return AWS_OP_SUCCESS;
}

Expand All @@ -1173,6 +1172,7 @@ static int s_process_pending_output_messages(struct aws_channel_handler *handler
if (sc_handler->slot->adj_right) {
downstream_window = aws_channel_slot_downstream_read_window(sc_handler->slot);
}
int ret = AWS_OP_ERR;

AWS_LOGF_TRACE(
AWS_LS_IO_TLS,
Expand Down Expand Up @@ -1207,7 +1207,7 @@ static int s_process_pending_output_messages(struct aws_channel_handler *handler
}
if (aws_channel_slot_send_message(sc_handler->slot, read_out_msg, AWS_CHANNEL_DIR_READ)) {
aws_mem_release(read_out_msg->allocator, read_out_msg);
return AWS_OP_ERR;
goto done;
}

if (sc_handler->slot->adj_right) {
Expand All @@ -1222,14 +1222,21 @@ static int s_process_pending_output_messages(struct aws_channel_handler *handler
sc_handler->buffered_read_out_data_buf.len = 0;
}
}
if (sc_handler->buffered_read_out_data_buf.len == 0 && sc_handler->delay_shutdown_scheduled) {
sc_handler->read_shutdown_completed = true;
ret = AWS_OP_SUCCESS;
if (sc_handler->buffered_read_out_data_buf.len > 0) {
/* Still have more data to be delivered */
return ret;
}

done:
if (sc_handler->read_state == AWS_TLS_HANDLER_READ_SHUTTING_DOWN) {
sc_handler->read_state = AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE;
/* Continue the shutdown process delayed before. */
aws_channel_slot_on_handler_shutdown_complete(
sc_handler->slot, AWS_CHANNEL_DIR_READ, sc_handler->delay_shutdown_error_code, false);
sc_handler->slot, AWS_CHANNEL_DIR_READ, sc_handler->shutdown_error_code, false);
graebm marked this conversation as resolved.
Show resolved Hide resolved
}

return AWS_OP_SUCCESS;
return ret;
}

static void s_process_pending_output_task(struct aws_channel_task *task, void *arg, enum aws_task_status status) {
Expand All @@ -1252,7 +1259,10 @@ static int s_process_read_message(
struct aws_io_message *message) {

struct secure_channel_handler *sc_handler = handler->impl;
if (sc_handler->read_shutdown_completed) {
if (sc_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
if (message) {
aws_mem_release(message->allocator, message);
}
return AWS_OP_SUCCESS;
graebm marked this conversation as resolved.
Show resolved Hide resolved
}

graebm marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -1480,7 +1490,7 @@ static int s_process_write_message(
static int s_increment_read_window(struct aws_channel_handler *handler, struct aws_channel_slot *slot, size_t size) {
(void)size;
struct secure_channel_handler *sc_handler = handler->impl;
if (sc_handler->read_shutdown_completed) {
if (sc_handler->read_state == AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE) {
return AWS_OP_SUCCESS;
}
AWS_LOGF_TRACE(AWS_LS_IO_TLS, "id=%p: Increment read window message received %zu", (void *)handler, size);
Expand Down Expand Up @@ -1564,8 +1574,8 @@ static void s_initialize_read_delay_shutdown(
" Your application may hang if the read window never opens",
(void *)handler);
}
sc_handler->delay_shutdown_scheduled = true;
sc_handler->delay_shutdown_error_code = error_code;
sc_handler->read_state = AWS_TLS_HANDLER_READ_SHUTTING_DOWN;
sc_handler->shutdown_error_code = error_code;
if (!sc_handler->read_task_pending) {
/* Kick off read, in case data arrives with TLS negotiation. Shutdown starts right after negotiation.
* Nothing will kick off read in that case. */
Expand Down Expand Up @@ -1608,7 +1618,7 @@ static int s_handler_shutdown(
s_initialize_read_delay_shutdown(handler, slot, error_code);
return AWS_OP_SUCCESS;
}
sc_handler->read_shutdown_completed = true;
sc_handler->read_state = AWS_TLS_HANDLER_READ_SHUT_DOWN_COMPLETE;
} else {
/* Shutdown in write direction */
if (!abort_immediately && error_code != AWS_IO_SOCKET_CLOSED) {
Expand Down