From 2ea02a5de51c012ce2532e6ad5cc0f5115b0ce89 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Tue, 13 Feb 2024 09:44:51 -0800 Subject: [PATCH] Fix a bunch of places we forget to aws_raise_error() (#622) --- .github/workflows/ci.yml | 2 +- source/channel.c | 2 +- source/darwin/darwin_pki_utils.c | 87 ++++++++++++------- .../secure_transport_tls_channel_handler.c | 17 ++-- source/event_loop.c | 2 +- source/posix/socket.c | 2 +- source/windows/iocp/iocp_event_loop.c | 2 +- source/windows/secure_channel_tls_handler.c | 6 +- 8 files changed, 75 insertions(+), 45 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 28dac9707..99ca48dec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,7 @@ on: - 'main' env: - BUILDER_VERSION: v0.9.48 + BUILDER_VERSION: v0.9.55 BUILDER_SOURCE: releases BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-io diff --git a/source/channel.c b/source/channel.c index 4fb762114..9155bcd22 100644 --- a/source/channel.c +++ b/source/channel.c @@ -743,7 +743,7 @@ int aws_channel_slot_insert_end(struct aws_channel *channel, struct aws_channel_ } AWS_ASSERT(0); - return AWS_OP_ERR; + return aws_raise_error(AWS_ERROR_INVALID_STATE); } int aws_channel_slot_insert_left(struct aws_channel_slot *slot, struct aws_channel_slot *to_add) { diff --git a/source/darwin/darwin_pki_utils.c b/source/darwin/darwin_pki_utils.c index b18a757cb..479932d11 100644 --- a/source/darwin/darwin_pki_utils.c +++ b/source/darwin/darwin_pki_utils.c @@ -101,12 +101,8 @@ int aws_import_public_and_private_keys_to_identity( int result = AWS_OP_ERR; - CFDataRef cert_data = CFDataCreate(cf_alloc, public_cert_chain->ptr, public_cert_chain->len); - CFDataRef key_data = CFDataCreate(cf_alloc, private_key->ptr, private_key->len); - - if (!cert_data || !key_data) { - return aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); - } + CFDataRef cert_data = NULL; + CFDataRef key_data = NULL; CFArrayRef cert_import_output = NULL; CFArrayRef key_import_output = NULL; @@ -118,9 +114,26 @@ int aws_import_public_and_private_keys_to_identity( import_params.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION; import_params.passphrase = CFSTR(""); + struct aws_array_list cert_chain_list; + AWS_ZERO_STRUCT(cert_chain_list); + CFDataRef root_cert_data = NULL; SecCertificateRef certificate_ref = NULL; SecKeychainRef import_keychain = NULL; + cert_data = CFDataCreate(cf_alloc, public_cert_chain->ptr, public_cert_chain->len); + if (!cert_data) { + AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: failed creating public cert chain data."); + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; + } + + key_data = CFDataCreate(cf_alloc, private_key->ptr, private_key->len); + if (!key_data) { + AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: failed creating private key data."); + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; + } + # pragma clang diagnostic push # pragma clang diagnostic ignored "-Wdeprecated-declarations" /* SecKeychain functions are marked as deprecated. @@ -134,7 +147,8 @@ int aws_import_public_and_private_keys_to_identity( "static: error opening keychain \"%s\" with OSStatus %d", aws_string_c_str(keychain_path), keychain_status); - return AWS_OP_ERR; + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; } keychain_status = SecKeychainUnlock(import_keychain, 0, "", true); if (keychain_status != errSecSuccess) { @@ -143,14 +157,16 @@ int aws_import_public_and_private_keys_to_identity( "static: error unlocking keychain \"%s\" with OSStatus %d", aws_string_c_str(keychain_path), keychain_status); - return AWS_OP_ERR; + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; } } else { OSStatus keychain_status = SecKeychainCopyDefault(&import_keychain); if (keychain_status != errSecSuccess) { AWS_LOGF_ERROR( AWS_LS_IO_PKI, "static: error opening the default keychain with OSStatus %d", keychain_status); - return AWS_OP_ERR; + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; } } @@ -168,9 +184,6 @@ int aws_import_public_and_private_keys_to_identity( OSStatus key_status = SecItemImport(key_data, NULL, &format, &item_type, 0, &import_params, import_keychain, &key_import_output); - CFRelease(cert_data); - CFRelease(key_data); - if (cert_status != errSecSuccess && cert_status != errSecDuplicateItem) { AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: error importing certificate with OSStatus %d", (int)cert_status); result = aws_raise_error(AWS_IO_FILE_VALIDATION_FAILURE); @@ -201,11 +214,8 @@ int aws_import_public_and_private_keys_to_identity( AWS_LS_IO_PKI, "static: certificate has an existing certificate-key pair that was previously imported into the Keychain. " "Using key from Keychain instead of the one provided."); - struct aws_array_list cert_chain_list; - if (aws_pem_objects_init_from_file_contents(&cert_chain_list, alloc, *public_cert_chain)) { AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: decoding certificate PEM failed."); - aws_pem_objects_clean_up(&cert_chain_list); result = AWS_OP_ERR; goto done; } @@ -214,36 +224,46 @@ int aws_import_public_and_private_keys_to_identity( aws_array_list_get_at_ptr(&cert_chain_list, (void **)&root_cert_ptr, 0); AWS_ASSERT(root_cert_ptr); CFDataRef root_cert_data = CFDataCreate(cf_alloc, root_cert_ptr->data.buffer, root_cert_ptr->data.len); - - if (root_cert_data) { - certificate_ref = SecCertificateCreateWithData(cf_alloc, root_cert_data); - CFRelease(root_cert_data); + if (!root_cert_data) { + AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: failed creating root cert data."); + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; } - aws_pem_objects_clean_up(&cert_chain_list); + certificate_ref = SecCertificateCreateWithData(cf_alloc, root_cert_data); + if (!certificate_ref) { + AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: failed to create certificate."); + result = aws_raise_error(AWS_IO_FILE_VALIDATION_FAILURE); + goto done; + } } else { certificate_ref = (SecCertificateRef)CFArrayGetValueAtIndex(cert_import_output, 0); /* SecCertificateCreateWithData returns an object with +1 retain, so we need to match that behavior here */ CFRetain(certificate_ref); } - /* if we got a cert one way or the other, create the identity and return it */ - if (certificate_ref) { - SecIdentityRef identity_output; - OSStatus status = SecIdentityCreateWithCertificate(import_keychain, certificate_ref, &identity_output); - if (status == errSecSuccess) { - CFTypeRef certs[] = {identity_output}; - *identity = CFArrayCreate(cf_alloc, (const void **)certs, 1L, &kCFTypeArrayCallBacks); - result = AWS_OP_SUCCESS; - goto done; - } + /* we got a cert one way or the other, create the identity and return it */ + AWS_ASSERT(certificate_ref); + SecIdentityRef identity_output; + OSStatus status = SecIdentityCreateWithCertificate(import_keychain, certificate_ref, &identity_output); + if (status != errSecSuccess) { + AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: error creating identity with OSStatus %d", key_status); + result = aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + goto done; } + CFTypeRef certs[] = {identity_output}; + *identity = CFArrayCreate(cf_alloc, (const void **)certs, 1L, &kCFTypeArrayCallBacks); + result = AWS_OP_SUCCESS; + done: aws_mutex_unlock(&s_sec_mutex); if (certificate_ref) { CFRelease(certificate_ref); } + if (root_cert_data) { + CFRelease(root_cert_data); + } if (cert_import_output) { CFRelease(cert_import_output); } @@ -253,6 +273,13 @@ int aws_import_public_and_private_keys_to_identity( if (import_keychain) { CFRelease(import_keychain); } + if (cert_data) { + CFRelease(cert_data); + } + if (key_data) { + CFRelease(key_data); + } + aws_pem_objects_clean_up(&cert_chain_list); return result; } diff --git a/source/darwin/secure_transport_tls_channel_handler.c b/source/darwin/secure_transport_tls_channel_handler.c index 5846807e4..fe647dd24 100644 --- a/source/darwin/secure_transport_tls_channel_handler.c +++ b/source/darwin/secure_transport_tls_channel_handler.c @@ -335,7 +335,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { &secure_transport_handler->protocol, handler->alloc, (size_t)CFStringGetLength(protocol) + 1)) { CFRelease(protocol); s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } memset(secure_transport_handler->protocol.buffer, 0, secure_transport_handler->protocol.capacity); @@ -399,7 +399,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { if (secure_transport_handler->verify_peer) { if (!secure_transport_handler->ca_certs) { s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } SecTrustRef trust; @@ -407,7 +407,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { if (status != errSecSuccess) { s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } SecPolicyRef policy; @@ -428,7 +428,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { AWS_LOGF_ERROR(AWS_LS_IO_TLS, "id=%p: Failed to set trust policy %d\n", (void *)handler, (int)status); CFRelease(trust); s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } status = SecTrustSetAnchorCertificates(trust, secure_transport_handler->ca_certs); @@ -440,7 +440,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { (int)status); CFRelease(trust); s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } /* Use ONLY the custom CA bundle (ignoring system anchors) */ @@ -453,7 +453,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { (int)status); CFRelease(trust); s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } SecTrustResultType trust_eval = 0; @@ -471,7 +471,7 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { (void *)handler, (int)status, (int)trust_eval); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } return s_drive_negotiation(handler); /* if this is here, everything went wrong. */ @@ -479,9 +479,8 @@ static int s_drive_negotiation(struct aws_channel_handler *handler) { secure_transport_handler->negotiation_finished = false; AWS_LOGF_WARN(AWS_LS_IO_TLS, "id=%p: negotiation failed with OSStatus %d.", (void *)handler, (int)status); - aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); s_invoke_negotiation_callback(handler, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); - return AWS_OP_ERR; + return aws_raise_error(AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE); } return AWS_OP_SUCCESS; diff --git a/source/event_loop.c b/source/event_loop.c index a6d51da66..1e7aef676 100644 --- a/source/event_loop.c +++ b/source/event_loop.c @@ -402,7 +402,7 @@ int aws_event_loop_fetch_local_object( return AWS_OP_SUCCESS; } - return AWS_OP_ERR; + return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); } int aws_event_loop_put_local_object(struct aws_event_loop *event_loop, struct aws_event_loop_local_object *obj) { diff --git a/source/posix/socket.c b/source/posix/socket.c index dcf0c9d55..0dac9442c 100644 --- a/source/posix/socket.c +++ b/source/posix/socket.c @@ -1892,7 +1892,7 @@ int aws_socket_get_error(struct aws_socket *socket) { socklen_t result_length = sizeof(connect_result); if (getsockopt(socket->io_handle.data.fd, SOL_SOCKET, SO_ERROR, &connect_result, &result_length) < 0) { - return AWS_OP_ERR; + return s_determine_socket_error(errno); } if (connect_result) { diff --git a/source/windows/iocp/iocp_event_loop.c b/source/windows/iocp/iocp_event_loop.c index 9ccce3007..b6716a543 100644 --- a/source/windows/iocp/iocp_event_loop.c +++ b/source/windows/iocp/iocp_event_loop.c @@ -635,7 +635,7 @@ static int s_unsubscribe_from_io_events(struct aws_event_loop *event_loop, struc "id=%p: failed to un-subscribe from events on handle %p", (void *)event_loop, (void *)handle->data.handle); - return AWS_OP_ERR; + return aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); } static void s_free_io_event_resources(void *user_data) { diff --git a/source/windows/secure_channel_tls_handler.c b/source/windows/secure_channel_tls_handler.c index 6dd0d1eb8..e925b5636 100644 --- a/source/windows/secure_channel_tls_handler.c +++ b/source/windows/secure_channel_tls_handler.c @@ -1512,9 +1512,13 @@ 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 || outgoing_message->message_data.capacity <= upstream_overhead) { + 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); + } /* what if message is larger than one record? */ size_t original_message_fragment_to_process = outgoing_message->message_data.capacity - upstream_overhead;