Fix a bunch of places we forget to aws_raise_error() (#622)

graebm · web-flow · commit 2a84a98dd773 · 2024-02-13T09:44:51.000-08:00
diff --git 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
@@ -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
@@ -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
@@ -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,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;
@@ -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,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;
diff --git 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
@@ -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
@@ -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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -743,7 +743,7 @@ int aws_channel_slot_insert_end(struct aws_channel *channel, struct aws_channel_`
`743`	`743`	`}`
`744`	`744`
`745`	`745`	`AWS_ASSERT(0);`
`746`		`- return AWS_OP_ERR;`
	`746`	`+ return aws_raise_error(AWS_ERROR_INVALID_STATE);`
`747`	`747`	`}`
`748`	`748`
`749`	`749`	`int aws_channel_slot_insert_left(struct aws_channel_slot slot, struct aws_channel_slot to_add) {`
Original file line number	Diff line number	Diff line change
`@@ -402,7 +402,7 @@ int aws_event_loop_fetch_local_object(`
`402`	`402`	`return AWS_OP_SUCCESS;`
`403`	`403`	`}`
`404`	`404`
`405`		`- return AWS_OP_ERR;`
	`405`	`+ return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);`
`406`	`406`	`}`
`407`	`407`
`408`	`408`	`int aws_event_loop_put_local_object(struct aws_event_loop event_loop, struct aws_event_loop_local_object obj) {`
Original file line number	Diff line number	Diff line change
`@@ -1892,7 +1892,7 @@ int aws_socket_get_error(struct aws_socket *socket) {`
`1892`	`1892`	`socklen_t result_length = sizeof(connect_result);`
`1893`	`1893`
`1894`	`1894`	`if (getsockopt(socket->io_handle.data.fd, SOL_SOCKET, SO_ERROR, &connect_result, &result_length) < 0) {`
`1895`		`- return AWS_OP_ERR;`
	`1895`	`+ return s_determine_socket_error(errno);`
`1896`	`1896`	`}`
`1897`	`1897`
`1898`	`1898`	`if (connect_result) {`
Original file line number	Diff line number	Diff line change
`@@ -635,7 +635,7 @@ static int s_unsubscribe_from_io_events(struct aws_event_loop *event_loop, struc`
`635`	`635`	`"id=%p: failed to un-subscribe from events on handle %p",`
`636`	`636`	`(void *)event_loop,`
`637`	`637`	`(void *)handle->data.handle);`
`638`		`- return AWS_OP_ERR;`
	`638`	`+ return aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE);`
`639`	`639`	`}`
`640`	`640`
`641`	`641`	`static void s_free_io_event_resources(void *user_data) {`