diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aeac60f99..bee9ab645 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,7 +107,15 @@ jobs: - name: Build ${{ env.PACKAGE_NAME }} + consumers run: | python -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')" - python builder.pyz build -p ${{ env.PACKAGE_NAME }} --compiler msvc-16 + python builder.pyz build -p ${{ env.PACKAGE_NAME }} + + windows-debug: + runs-on: windows-2022 # latest + steps: + - name: Build ${{ env.PACKAGE_NAME }} + consumers + run: | + python -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')" + python builder.pyz build -p ${{ env.PACKAGE_NAME }} --config Debug windows-vc14: runs-on: windows-2019 # windows-2019 is last env with Visual Studio 2015 (v14.0) diff --git a/include/aws/io/channel.h b/include/aws/io/channel.h index 1f8401f9a..dc48cd054 100644 --- a/include/aws/io/channel.h +++ b/include/aws/io/channel.h @@ -272,7 +272,7 @@ int aws_channel_remove_local_object( /** * Acquires a message from the event loop's message pool. size_hint is merely a hint, it may be smaller than you * requested and you are responsible for checking the bounds of it. If the returned message is not large enough, you - * must send multiple messages. + * must send multiple messages. This cannot fail, it never returns NULL. */ AWS_IO_API struct aws_io_message *aws_channel_acquire_message_from_pool( @@ -403,7 +403,7 @@ int aws_channel_slot_send_message( /** * Convenience function that invokes aws_channel_acquire_message_from_pool(), * asking for the largest reasonable DATA message that can be sent in the write direction, - * with upstream overhead accounted for. + * with upstream overhead accounted for. This cannot fail, it never returns NULL. */ AWS_IO_API struct aws_io_message *aws_channel_slot_acquire_max_message_for_write(struct aws_channel_slot *slot); diff --git a/source/channel.c b/source/channel.c index 36a3975b2..c387844f4 100644 --- a/source/channel.c +++ b/source/channel.c @@ -422,18 +422,15 @@ struct aws_io_message *aws_channel_acquire_message_from_pool( size_t size_hint) { struct aws_io_message *message = aws_message_pool_acquire(channel->msg_pool, message_type, size_hint); - - if (AWS_LIKELY(message)) { - message->owning_channel = channel; - AWS_LOGF_TRACE( - AWS_LS_IO_CHANNEL, - "id=%p: acquired message %p of capacity %zu from pool %p. Requested size was %zu", - (void *)channel, - (void *)message, - message->message_data.capacity, - (void *)channel->msg_pool, - size_hint); - } + message->owning_channel = channel; + AWS_LOGF_TRACE( + AWS_LS_IO_CHANNEL, + "id=%p: acquired message %p of capacity %zu from pool %p. Requested size was %zu", + (void *)channel, + (void *)message, + message->message_data.capacity, + (void *)channel->msg_pool, + size_hint); return message; } @@ -818,12 +815,8 @@ struct aws_io_message *aws_channel_slot_acquire_max_message_for_write(struct aws AWS_PRECONDITION(aws_channel_thread_is_callers_thread(slot->channel)); const size_t overhead = aws_channel_slot_upstream_message_overhead(slot); - if (overhead >= g_aws_channel_max_fragment_size) { - AWS_LOGF_ERROR( - AWS_LS_IO_CHANNEL, "id=%p: Upstream overhead exceeds channel's max message size.", (void *)slot->channel); - aws_raise_error(AWS_ERROR_INVALID_STATE); - return NULL; - } + AWS_FATAL_ASSERT( + overhead < g_aws_channel_max_fragment_size && "Upstream overhead cannot exceed channel's max message size"); const size_t size_hint = g_aws_channel_max_fragment_size - overhead; return aws_channel_acquire_message_from_pool(slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, size_hint); diff --git a/source/darwin/secure_transport_tls_channel_handler.c b/source/darwin/secure_transport_tls_channel_handler.c index 85fe5ea57..7ab3d0ac5 100644 --- a/source/darwin/secure_transport_tls_channel_handler.c +++ b/source/darwin/secure_transport_tls_channel_handler.c @@ -166,7 +166,8 @@ static OSStatus s_write_cb(SSLConnectionRef conn, const void *data, size_t *len) struct aws_io_message *message = aws_channel_acquire_message_from_pool( handler->parent_slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, message_size_hint); - if (!message || message->message_data.capacity <= overhead) { + if (message->message_data.capacity <= overhead) { + aws_mem_release(message->allocator, message); return errSecMemoryError; } @@ -612,11 +613,6 @@ static int s_process_read_message( struct aws_io_message *outgoing_read_message = aws_channel_acquire_message_from_pool( slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, downstream_window - processed); - if (!outgoing_read_message) { - /* even though this is a failure, this handler has taken ownership of the message */ - aws_channel_shutdown(secure_transport_handler->parent_slot->channel, aws_last_error()); - return AWS_OP_SUCCESS; - } size_t read = 0; status = SSLRead( diff --git a/source/message_pool.c b/source/message_pool.c index 3090f57e6..3fc9c4dfa 100644 --- a/source/message_pool.c +++ b/source/message_pool.c @@ -154,13 +154,10 @@ struct aws_io_message *aws_message_pool_acquire( break; default: AWS_ASSERT(0); - aws_raise_error(AWS_IO_CHANNEL_UNKNOWN_MESSAGE_TYPE); - return NULL; + break; } - if (!message_wrapper) { - return NULL; - } + AWS_FATAL_ASSERT(message_wrapper); message_wrapper->message.message_type = message_type; message_wrapper->message.message_tag = 0; diff --git a/source/s2n/s2n_tls_channel_handler.c b/source/s2n/s2n_tls_channel_handler.c index 14839d19f..d72449eab 100644 --- a/source/s2n/s2n_tls_channel_handler.c +++ b/source/s2n/s2n_tls_channel_handler.c @@ -328,7 +328,8 @@ static int s_generic_send(struct s2n_handler *handler, struct aws_byte_buf *buf) struct aws_io_message *message = aws_channel_acquire_message_from_pool( handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, message_size_hint); - if (!message || message->message_data.capacity <= overhead) { + if (message->message_data.capacity <= overhead) { + aws_mem_release(message->allocator, message); errno = ENOMEM; return -1; } @@ -554,9 +555,6 @@ static int s_s2n_handler_process_read_message( struct aws_io_message *outgoing_read_message = aws_channel_acquire_message_from_pool( slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, downstream_window - processed); - if (!outgoing_read_message) { - return AWS_OP_ERR; - } ssize_t read = s2n_recv( s2n_handler->connection, diff --git a/source/socket_channel_handler.c b/source/socket_channel_handler.c index c327f6f35..27f788a28 100644 --- a/source/socket_channel_handler.c +++ b/source/socket_channel_handler.c @@ -150,11 +150,6 @@ static void s_do_read(struct socket_handler *socket_handler) { struct aws_io_message *message = aws_channel_acquire_message_from_pool( socket_handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, iter_max_read); - if (!message) { - last_error = aws_last_error(); - break; - } - if (aws_socket_read(socket_handler->socket, &message->message_data, &read)) { last_error = aws_last_error(); aws_mem_release(message->allocator, message); diff --git a/source/windows/secure_channel_tls_handler.c b/source/windows/secure_channel_tls_handler.c index b62b1a0b7..b3b42490e 100644 --- a/source/windows/secure_channel_tls_handler.c +++ b/source/windows/secure_channel_tls_handler.c @@ -476,11 +476,6 @@ static int s_fillin_alpn_data( return AWS_OP_SUCCESS; } -static int s_process_connection_state(struct aws_channel_handler *handler) { - struct secure_channel_handler *sc_handler = handler->impl; - return sc_handler->s_connection_state_fn(handler); -} - static int s_do_application_data_decrypt(struct aws_channel_handler *handler); static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handler); @@ -493,6 +488,9 @@ static int s_do_server_side_negotiation_step_1(struct aws_channel_handler *handl aws_on_drive_tls_negotiation(&sc_handler->shared_state); + /* set this, and goto cleanup, if function encounters an error */ + int aws_error = 0; + unsigned char alpn_buffer_data[128] = {0}; SecBuffer input_bufs[] = { { @@ -513,12 +511,25 @@ static int s_do_server_side_negotiation_step_1(struct aws_channel_handler *handl .pBuffers = input_bufs, }; + SecBuffer output_buffer = { + .pvBuffer = NULL, + .cbBuffer = 0, + .BufferType = SECBUFFER_TOKEN, + }; + + SecBufferDesc output_buffer_desc = { + .ulVersion = SECBUFFER_VERSION, + .cBuffers = 1, + .pBuffers = &output_buffer, + }; + #ifdef SECBUFFER_APPLICATION_PROTOCOLS if (sc_handler->alpn_list && aws_tls_is_alpn_available()) { AWS_LOGF_DEBUG(AWS_LS_IO_TLS, "id=%p: Setting ALPN to %s", handler, aws_string_c_str(sc_handler->alpn_list)); size_t extension_length = 0; if (s_fillin_alpn_data(handler, alpn_buffer_data, sizeof(alpn_buffer_data), &extension_length)) { - return AWS_OP_ERR; + aws_error = aws_last_error(); + goto cleanup; } input_bufs[1].pvBuffer = alpn_buffer_data, input_bufs[1].cbBuffer = (unsigned long)extension_length, @@ -537,18 +548,6 @@ static int s_do_server_side_negotiation_step_1(struct aws_channel_handler *handl sc_handler->ctx_req |= ASC_REQ_MUTUAL_AUTH; } - SecBuffer output_buffer = { - .pvBuffer = NULL, - .cbBuffer = 0, - .BufferType = SECBUFFER_TOKEN, - }; - - SecBufferDesc output_buffer_desc = { - .ulVersion = SECBUFFER_VERSION, - .cBuffers = 1, - .pBuffers = &output_buffer, - }; - /* process the client hello. */ SECURITY_STATUS status = AcceptSecurityContext( &sc_handler->creds, @@ -567,10 +566,8 @@ static int s_do_server_side_negotiation_step_1(struct aws_channel_handler *handl "id=%p: error during processing of the ClientHello. SECURITY_STATUS is %d", (void *)handler, (int)status); - int error = s_determine_sspi_error(status); - aws_raise_error(error); - s_invoke_negotiation_error(handler, error); - return AWS_OP_ERR; + aws_error = s_determine_sspi_error(status); + goto cleanup; } size_t data_to_write_len = output_buffer.cbBuffer; @@ -579,25 +576,25 @@ static int s_do_server_side_negotiation_step_1(struct aws_channel_handler *handl /* send the server hello. */ struct aws_io_message *outgoing_message = aws_channel_acquire_message_from_pool( sc_handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, data_to_write_len); - if (!outgoing_message) { - FreeContextBuffer(output_buffer.pvBuffer); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; - } - AWS_ASSERT(outgoing_message->message_data.capacity >= data_to_write_len); + AWS_FATAL_ASSERT(outgoing_message->message_data.capacity >= data_to_write_len); memcpy(outgoing_message->message_data.buffer, output_buffer.pvBuffer, output_buffer.cbBuffer); outgoing_message->message_data.len = output_buffer.cbBuffer; - FreeContextBuffer(output_buffer.pvBuffer); if (aws_channel_slot_send_message(sc_handler->slot, outgoing_message, AWS_CHANNEL_DIR_WRITE)) { + aws_error = aws_last_error(); aws_mem_release(outgoing_message->allocator, outgoing_message); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; + goto cleanup; } sc_handler->s_connection_state_fn = s_do_server_side_negotiation_step_2; +cleanup: + FreeContextBuffer(output_buffer.pvBuffer); + if (aws_error != 0) { + s_invoke_negotiation_error(handler, aws_error); + return aws_raise_error(aws_error); + } return AWS_OP_SUCCESS; } @@ -605,6 +602,9 @@ static int s_do_server_side_negotiation_step_1(struct aws_channel_handler *handl static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handler) { struct secure_channel_handler *sc_handler = handler->impl; + /* set this, and goto cleanup, if function encounters an error */ + int aws_error = 0; + AWS_LOGF_TRACE( AWS_LS_IO_TLS, "id=%p: running step 2 of negotiation (cipher change, key exchange etc...)", (void *)handler); SecBuffer input_buffers[] = { @@ -656,17 +656,16 @@ static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handl if (status != SEC_E_INCOMPLETE_MESSAGE && status != SEC_I_CONTINUE_NEEDED && status != SEC_E_OK) { AWS_LOGF_ERROR( AWS_LS_IO_TLS, "id=%p: Error during negotiation. SECURITY_STATUS is %d", (void *)handler, (int)status); - int aws_error = s_determine_sspi_error(status); - aws_raise_error(aws_error); - s_invoke_negotiation_error(handler, aws_error); - return AWS_OP_ERR; + aws_error = s_determine_sspi_error(status); + goto cleanup; } if (status == SEC_E_INCOMPLETE_MESSAGE) { AWS_LOGF_TRACE( AWS_LS_IO_TLS, "id=%p: Last processed buffer was incomplete, waiting on more data.", (void *)handler); sc_handler->estimated_incomplete_size = input_buffers[1].cbBuffer; - return aws_raise_error(AWS_IO_READ_WOULD_BLOCK); + aws_error = AWS_IO_READ_WOULD_BLOCK; + goto cleanup; }; /* any output buffers that were filled in with SECBUFFER_TOKEN need to be sent, SECBUFFER_EXTRA means we need to account for extra data and shift everything for the next run. */ @@ -678,20 +677,14 @@ static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handl struct aws_io_message *outgoing_message = aws_channel_acquire_message_from_pool( sc_handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, buf_ptr->cbBuffer); - if (!outgoing_message) { - FreeContextBuffer(buf_ptr->pvBuffer); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; - } - + AWS_FATAL_ASSERT(outgoing_message->message_data.capacity >= buf_ptr->cbBuffer); memcpy(outgoing_message->message_data.buffer, buf_ptr->pvBuffer, buf_ptr->cbBuffer); outgoing_message->message_data.len = buf_ptr->cbBuffer; - FreeContextBuffer(buf_ptr->pvBuffer); if (aws_channel_slot_send_message(sc_handler->slot, outgoing_message, AWS_CHANNEL_DIR_WRITE)) { + aws_error = aws_last_error(); aws_mem_release(outgoing_message->allocator, outgoing_message); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; + goto cleanup; } } } @@ -716,9 +709,8 @@ static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handl (void *)handler); if (s_manually_verify_peer_cert(handler)) { - aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - s_invoke_negotiation_error(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + aws_error = AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE; + goto cleanup; } } sc_handler->negotiation_finished = true; @@ -748,8 +740,6 @@ static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handl "id=%p: Error retrieving negotiated protocol. SECURITY_STATUS is %d", handler, (int)status); - int aws_error = s_determine_sspi_error(status); - aws_raise_error(aws_error); } } #endif @@ -758,6 +748,17 @@ static int s_do_server_side_negotiation_step_2(struct aws_channel_handler *handl s_on_negotiation_success(handler); } +cleanup: + for (size_t i = 0; i < output_buffers_desc.cBuffers; ++i) { + FreeContextBuffer(output_buffers[i].pvBuffer); + } + + if (aws_error != 0) { + if (aws_error != AWS_IO_READ_WOULD_BLOCK) { + s_invoke_negotiation_error(handler, aws_error); + } + return aws_raise_error(aws_error); + } return AWS_OP_SUCCESS; } @@ -770,6 +771,9 @@ static int s_do_client_side_negotiation_step_1(struct aws_channel_handler *handl aws_on_drive_tls_negotiation(&sc_handler->shared_state); + /* set this, and goto cleanup, if function encounters an error */ + int aws_error = 0; + unsigned char alpn_buffer_data[128] = {0}; SecBuffer input_buf = { .pvBuffer = NULL, @@ -783,6 +787,18 @@ static int s_do_client_side_negotiation_step_1(struct aws_channel_handler *handl .pBuffers = &input_buf, }; + SecBuffer output_buffer = { + .pvBuffer = NULL, + .cbBuffer = 0, + .BufferType = SECBUFFER_EMPTY, + }; + + SecBufferDesc output_buffer_desc = { + .ulVersion = SECBUFFER_VERSION, + .cBuffers = 1, + .pBuffers = &output_buffer, + }; + SecBufferDesc *alpn_sspi_data = NULL; /* add alpn data to the client hello if it's supported. */ @@ -792,8 +808,8 @@ static int s_do_client_side_negotiation_step_1(struct aws_channel_handler *handl AWS_LS_IO_TLS, "id=%p: Setting ALPN data as %s", handler, aws_string_c_str(sc_handler->alpn_list)); size_t extension_length = 0; if (s_fillin_alpn_data(handler, alpn_buffer_data, sizeof(alpn_buffer_data), &extension_length)) { - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; + aws_error = aws_last_error(); + goto cleanup; } input_buf.pvBuffer = alpn_buffer_data, input_buf.cbBuffer = (unsigned long)extension_length, @@ -806,18 +822,6 @@ static int s_do_client_side_negotiation_step_1(struct aws_channel_handler *handl sc_handler->ctx_req = ISC_REQ_SEQUENCE_DETECT | ISC_REQ_REPLAY_DETECT | ISC_REQ_CONFIDENTIALITY | ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_STREAM; - SecBuffer output_buffer = { - .pvBuffer = NULL, - .cbBuffer = 0, - .BufferType = SECBUFFER_EMPTY, - }; - - SecBufferDesc output_buffer_desc = { - .ulVersion = SECBUFFER_VERSION, - .cBuffers = 1, - .pBuffers = &output_buffer, - }; - char server_name_cstr[256]; AWS_ZERO_ARRAY(server_name_cstr); AWS_ASSERT(sc_handler->server_name.len < 256); @@ -843,10 +847,8 @@ static int s_do_client_side_negotiation_step_1(struct aws_channel_handler *handl "id=%p: Error sending client/receiving server handshake data. SECURITY_STATUS is %d", (void *)handler, (int)status); - int aws_error = s_determine_sspi_error(status); - aws_raise_error(aws_error); - s_invoke_negotiation_error(handler, aws_error); - return AWS_OP_ERR; + aws_error = s_determine_sspi_error(status); + goto cleanup; } size_t data_to_write_len = output_buffer.cbBuffer; @@ -855,25 +857,25 @@ static int s_do_client_side_negotiation_step_1(struct aws_channel_handler *handl struct aws_io_message *outgoing_message = aws_channel_acquire_message_from_pool( sc_handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, data_to_write_len); - if (!outgoing_message) { - FreeContextBuffer(output_buffer.pvBuffer); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; - } - AWS_ASSERT(outgoing_message->message_data.capacity >= data_to_write_len); + AWS_FATAL_ASSERT(outgoing_message->message_data.capacity >= data_to_write_len); memcpy(outgoing_message->message_data.buffer, output_buffer.pvBuffer, output_buffer.cbBuffer); outgoing_message->message_data.len = output_buffer.cbBuffer; - FreeContextBuffer(output_buffer.pvBuffer); if (aws_channel_slot_send_message(sc_handler->slot, outgoing_message, AWS_CHANNEL_DIR_WRITE)) { + aws_error = aws_last_error(); aws_mem_release(outgoing_message->allocator, outgoing_message); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; + goto cleanup; } sc_handler->s_connection_state_fn = s_do_client_side_negotiation_step_2; +cleanup: + FreeContextBuffer(output_buffer.pvBuffer); + if (aws_error != 0) { + s_invoke_negotiation_error(handler, aws_error); + return aws_raise_error(aws_error); + } return AWS_OP_SUCCESS; } @@ -885,6 +887,9 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl "id=%p: running step 2 of client-side negotiation (cipher change, key exchange etc...)", (void *)handler); + /* set this, and goto cleanup, if function encounters an error */ + int aws_error = 0; + SecBuffer input_buffers[] = { [0] = { @@ -944,10 +949,8 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl if (status != SEC_E_INCOMPLETE_MESSAGE && status != SEC_I_CONTINUE_NEEDED && status != SEC_E_OK) { AWS_LOGF_ERROR( AWS_LS_IO_TLS, "id=%p: Error during negotiation. SECURITY_STATUS is %d", (void *)handler, (int)status); - int aws_error = s_determine_sspi_error(status); - aws_raise_error(aws_error); - s_invoke_negotiation_error(handler, aws_error); - return AWS_OP_ERR; + aws_error = s_determine_sspi_error(status); + goto cleanup; } if (status == SEC_E_INCOMPLETE_MESSAGE) { @@ -957,7 +960,8 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl "id=%p: Incomplete buffer recieved. Incomplete size is %zu. Waiting for more data.", (void *)handler, sc_handler->estimated_incomplete_size); - return aws_raise_error(AWS_IO_READ_WOULD_BLOCK); + aws_error = AWS_IO_READ_WOULD_BLOCK; + goto cleanup; } if (status == SEC_I_CONTINUE_NEEDED || status == SEC_E_OK) { @@ -968,20 +972,14 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl struct aws_io_message *outgoing_message = aws_channel_acquire_message_from_pool( sc_handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, buf_ptr->cbBuffer); - if (!outgoing_message) { - FreeContextBuffer(buf_ptr->pvBuffer); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; - } - + AWS_FATAL_ASSERT(outgoing_message->message_data.capacity >= buf_ptr->cbBuffer); memcpy(outgoing_message->message_data.buffer, buf_ptr->pvBuffer, buf_ptr->cbBuffer); outgoing_message->message_data.len = buf_ptr->cbBuffer; - FreeContextBuffer(buf_ptr->pvBuffer); if (aws_channel_slot_send_message(sc_handler->slot, outgoing_message, AWS_CHANNEL_DIR_WRITE)) { aws_mem_release(outgoing_message->allocator, outgoing_message); - s_invoke_negotiation_error(handler, aws_last_error()); - return AWS_OP_ERR; + aws_error = aws_last_error(); + goto cleanup; } } } @@ -1005,9 +1003,8 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl "id=%p: Custom CA was configured, evaluating trust before completing connection", (void *)handler); if (s_manually_verify_peer_cert(handler)) { - aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - s_invoke_negotiation_error(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + aws_error = AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE; + goto cleanup; } } sc_handler->negotiation_finished = true; @@ -1033,8 +1030,6 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl "id=%p: Error retrieving negotiated protocol. SECURITY_STATUS is %d", handler, (int)status); - int aws_error = s_determine_sspi_error(status); - aws_raise_error(aws_error); } } #endif @@ -1043,6 +1038,17 @@ static int s_do_client_side_negotiation_step_2(struct aws_channel_handler *handl s_on_negotiation_success(handler); } +cleanup: + for (size_t i = 0; i < output_buffers_desc.cBuffers; ++i) { + FreeContextBuffer(output_buffers[i].pvBuffer); + } + + if (aws_error != 0) { + if (aws_error != AWS_IO_READ_WOULD_BLOCK) { + s_invoke_negotiation_error(handler, aws_error); + } + return aws_raise_error(aws_error); + } return AWS_OP_SUCCESS; } @@ -1176,10 +1182,6 @@ static int s_process_pending_output_messages(struct aws_channel_handler *handler struct aws_io_message *read_out_msg = aws_channel_acquire_message_from_pool( sc_handler->slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, requested_message_size); - if (!read_out_msg) { - return AWS_OP_ERR; - } - size_t copy_size = read_out_msg->message_data.capacity < requested_message_size ? read_out_msg->message_data.capacity : requested_message_size; @@ -1367,9 +1369,6 @@ static int s_process_write_message( struct aws_io_message *outgoing_message = aws_channel_acquire_message_from_pool(slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, to_write); - if (!outgoing_message) { - return AWS_OP_ERR; - } if (outgoing_message->message_data.capacity <= upstream_overhead) { aws_mem_release(outgoing_message->allocator, outgoing_message); return aws_raise_error(AWS_ERROR_INVALID_STATE); @@ -1520,6 +1519,18 @@ static int s_handler_shutdown( bool abort_immediately) { struct secure_channel_handler *sc_handler = handler->impl; + SecBuffer output_buffer = { + .pvBuffer = NULL, + .cbBuffer = 0, + .BufferType = SECBUFFER_EMPTY, + }; + + SecBufferDesc output_buffer_desc = { + .ulVersion = SECBUFFER_VERSION, + .cBuffers = 1, + .pBuffers = &output_buffer, + }; + if (dir == AWS_CHANNEL_DIR_WRITE) { if (!abort_immediately && error_code != AWS_IO_SOCKET_CLOSED) { AWS_LOGF_DEBUG(AWS_LS_IO_TLS, "id=%p: Shutting down the write direction", (void *)handler); @@ -1544,23 +1555,10 @@ static int s_handler_shutdown( status = ApplyControlToken(&sc_handler->sec_handle, &shutdown_buffer_desc); if (status != SEC_E_OK) { - aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); - return aws_channel_slot_on_handler_shutdown_complete( - slot, dir, AWS_ERROR_SYS_CALL_FAILURE, abort_immediately); + error_code = AWS_ERROR_SYS_CALL_FAILURE; + goto cleanup; } - SecBuffer output_buffer = { - .pvBuffer = NULL, - .cbBuffer = 0, - .BufferType = SECBUFFER_EMPTY, - }; - - SecBufferDesc output_buffer_desc = { - .ulVersion = SECBUFFER_VERSION, - .cBuffers = 1, - .pBuffers = &output_buffer, - }; - struct aws_byte_buf server_name = aws_tls_handler_server_name(handler); char server_name_cstr[256]; AWS_ZERO_ARRAY(server_name_cstr); @@ -1585,8 +1583,11 @@ static int s_handler_shutdown( struct aws_io_message *outgoing_message = aws_channel_acquire_message_from_pool( slot->channel, AWS_IO_MESSAGE_APPLICATION_DATA, output_buffer.cbBuffer); - if (!outgoing_message || outgoing_message->message_data.capacity < output_buffer.cbBuffer) { - return aws_channel_slot_on_handler_shutdown_complete(slot, dir, aws_last_error(), true); + if (outgoing_message->message_data.capacity < output_buffer.cbBuffer) { + aws_mem_release(outgoing_message->allocator, outgoing_message); + error_code = AWS_IO_TLS_ERROR_WRITE_FAILURE; + abort_immediately = true; + goto cleanup; } memcpy(outgoing_message->message_data.buffer, output_buffer.pvBuffer, output_buffer.cbBuffer); outgoing_message->message_data.len = output_buffer.cbBuffer; @@ -1594,11 +1595,14 @@ static int s_handler_shutdown( /* we don't really care if this succeeds or not, it's just sending the TLS alert. */ if (aws_channel_slot_send_message(slot, outgoing_message, AWS_CHANNEL_DIR_WRITE)) { aws_mem_release(outgoing_message->allocator, outgoing_message); + goto cleanup; } } } } +cleanup: + FreeContextBuffer(output_buffer.pvBuffer); return aws_channel_slot_on_handler_shutdown_complete(slot, dir, error_code, abort_immediately); } diff --git a/tests/tls_handler_test.c b/tests/tls_handler_test.c index 1a7f94ddf..e6db170a7 100644 --- a/tests/tls_handler_test.c +++ b/tests/tls_handler_test.c @@ -25,6 +25,22 @@ # include +/* badssl.com has occasional lags, make this timeout longer so we have a + * higher chance of actually testing something. */ +# define BADSSL_TIMEOUT_MS 10000 + +bool s_is_badssl_being_flaky(const struct aws_string *host_name, int error_code) { + if (strstr(aws_string_c_str(host_name), "badssl.com") != NULL) { + if (error_code == AWS_IO_SOCKET_TIMEOUT || error_code == AWS_IO_TLS_NEGOTIATION_TIMEOUT) { + fprintf( + AWS_TESTING_REPORT_FD, + "Warning: badssl.com is timing out right now. Maybe run the test again later?\n"); + return true; + } + } + return false; +} + struct tls_test_args { struct aws_allocator *allocator; struct aws_mutex *mutex; @@ -699,9 +715,7 @@ static int s_verify_negotiation_fails_helper( struct aws_socket_options options; AWS_ZERO_STRUCT(options); - /* badssl.com is great but has occasional lags, make this timeout longer so we have a - higher chance of actually testing something. */ - options.connect_timeout_ms = 10000; + options.connect_timeout_ms = BADSSL_TIMEOUT_MS; options.type = AWS_SOCKET_STREAM; options.domain = AWS_SOCKET_IPV4; @@ -735,16 +749,12 @@ static int s_verify_negotiation_fails_helper( ASSERT_TRUE(outgoing_args.error_invoked); - /* we're talking to an external internet endpoint, yeah this sucks... we don't know for sure that - this failed for the right reasons, but there's not much we can do about it.*/ - if (outgoing_args.last_error_code != AWS_IO_SOCKET_TIMEOUT) { - ASSERT_INT_EQUALS(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE, outgoing_args.last_error_code); - } else { - fprintf( - stderr, - "Warning: the connection timed out and we're not completely certain" - " that this fails for the right reasons. Maybe run the test again?\n"); + if (s_is_badssl_being_flaky(host_name, outgoing_args.last_error_code)) { + return AWS_OP_SKIP; } + + ASSERT_INT_EQUALS(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE, outgoing_args.last_error_code); + aws_client_bootstrap_release(client_bootstrap); aws_tls_ctx_release(client_ctx); @@ -1126,7 +1136,7 @@ static int s_verify_good_host( struct aws_socket_options options; AWS_ZERO_STRUCT(options); - options.connect_timeout_ms = 10000; + options.connect_timeout_ms = BADSSL_TIMEOUT_MS; options.type = AWS_SOCKET_STREAM; options.domain = AWS_SOCKET_IPV4; @@ -1159,6 +1169,10 @@ static int s_verify_good_host( &c_tester.condition_variable, &c_tester.mutex, s_tls_channel_setup_predicate, &outgoing_args)); ASSERT_SUCCESS(aws_mutex_unlock(&c_tester.mutex)); + if (s_is_badssl_being_flaky(host_name, outgoing_args.last_error_code)) { + return AWS_OP_SKIP; + } + ASSERT_FALSE(outgoing_args.error_invoked); struct aws_byte_buf expected_protocol = aws_byte_buf_from_c_str("http/1.1"); /* check ALPN and SNI was properly negotiated */