Skip to content

Commit

Permalink
Fix a bunch of places we forget to aws_raise_error()
Browse files Browse the repository at this point in the history
  • Loading branch information
graebm committed Feb 9, 2024
1 parent 4c65ce5 commit 9fe5e9b
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 44 deletions.
2 changes: 1 addition & 1 deletion source/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
87 changes: 57 additions & 30 deletions source/darwin/darwin_pki_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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) {
Expand All @@ -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;
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
Expand All @@ -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;
}
Expand Down
17 changes: 8 additions & 9 deletions source/darwin/secure_transport_tls_channel_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -399,15 +399,15 @@ 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;
status = SSLCopyPeerTrust(secure_transport_handler->ctx, &trust);

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;
Expand All @@ -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);
Expand All @@ -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) */
Expand All @@ -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;
Expand All @@ -471,17 +471,16 @@ 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. */
} else if (status != errSSLWouldBlock) {
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;
Expand Down
2 changes: 1 addition & 1 deletion source/event_loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion source/posix/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion source/windows/iocp/iocp_event_loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion source/windows/secure_channel_tls_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -1367,9 +1367,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;
Expand Down

0 comments on commit 9fe5e9b

Please sign in to comment.