lib/kadm5: improve kadm_c_ error handling

Perform error checking for each function call and consistently return
errors at the point of failure.

Refactor functions to use a common exit path.  Preserve error messages
stored in the kadm5_client_context.context when appropriate.

Change-Id: I7aa04020e4de3454066f0d88ba805fed999dbd1a

lib/kadm5/chpass_c.c

@@ -59,36 +59,49 @@ kadm5_c_chpass_principal(void *server_handle,
        return KADM5_KS_TUPLE_NOSUPP;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_chpass);
+    ret = krb5_store_int32(sp, kadm_chpass);
-    krb5_store_principal(sp, princ);
-    krb5_store_string(sp, password);
-    krb5_store_int32(sp, keepold); /* extension */
-    ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
-	return ret;
+	goto out;
+    ret = krb5_store_principal(sp, princ);
+    if (ret)
+	goto out;
+    ret = krb5_store_string(sp, password);
+    if (ret)
+	goto out;
+    ret = krb5_store_int32(sp, keepold); /* extension */
+    if (ret)
+	goto out;
+    ret = _kadm5_client_send(context, sp);
+    if (ret)
+	goto out_keep_error;
     ret = _kadm5_client_recv(context, &reply);
-    if(ret)
+    if (ret)
-	return ret;
+	goto out_keep_error;
-    sp = krb5_storage_from_data (&reply);
+    krb5_storage_free(sp);
+    sp = krb5_storage_from_data(&reply);
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	krb5_data_free (&reply);
+	goto out;
-	return ENOMEM;
     }
-    krb5_ret_int32(sp, &tmp);
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
+
+  out:
     krb5_clear_error_message(context->context);
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    krb5_data_free(&reply);
-    return tmp;
+    return ret;
 }
 
 kadm5_ret_t
@@ -107,36 +120,54 @@ kadm5_c_chpass_principal_with_key(void *server_handle,
     int i;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_chpass_with_key);
+    ret = krb5_store_int32(sp, kadm_chpass_with_key);
-    krb5_store_principal(sp, princ);
-    krb5_store_int32(sp, n_key_data);
-    for (i = 0; i < n_key_data; ++i)
-	kadm5_store_key_data (sp, &key_data[i]);
-    krb5_store_int32(sp, keepold); /* extension */
-    ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
-	return ret;
+	goto out;
+    ret = krb5_store_principal(sp, princ);
+    if (ret)
+	goto out;
+    ret = krb5_store_int32(sp, n_key_data);
+    if (ret)
+	goto out;
+    for (i = 0; i < n_key_data; ++i) {
+	ret = kadm5_store_key_data (sp, &key_data[i]);
+	if (ret)
+	    goto out;
+    }
+    ret = krb5_store_int32(sp, keepold); /* extension */
+    if (ret)
+	goto out;
+    ret = _kadm5_client_send(context, sp);
+    if (ret)
+	goto out_keep_error;
     ret = _kadm5_client_recv(context, &reply);
-    if(ret)
+    if (ret)
-	return ret;
+	goto out_keep_error;
+    krb5_storage_free(sp);
     sp = krb5_storage_from_data (&reply);
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	krb5_data_free (&reply);
+	goto out;
-	return ENOMEM;
     }
-    krb5_ret_int32(sp, &tmp);
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
+
+  out:
     krb5_clear_error_message(context->context);
+
+  out_keep_error:
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    krb5_data_free(&reply);
-    return tmp;
+    return ret;
 }

lib/kadm5/create_c.c

@@ -59,35 +59,50 @@ kadm5_c_create_principal(void *server_handle,
 	return KADM5_KS_TUPLE_NOSUPP;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_create);
+    ret = krb5_store_int32(sp, kadm_create);
-    kadm5_store_principal_ent(sp, princ);
-    krb5_store_int32(sp, mask);
-    krb5_store_string(sp, password);
-    ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
-	return ret;
+	goto out;
+    ret = kadm5_store_principal_ent(sp, princ);
+    if (ret)
+	goto out;
+    ret = krb5_store_int32(sp, mask);
+    if (ret)
+	goto out;
+    ret = krb5_store_string(sp, password);
+    if (ret)
+	goto out;
+    ret = _kadm5_client_send(context, sp);
+    if (ret)
+	goto out_keep_error;
     ret = _kadm5_client_recv(context, &reply);
-    if(ret)
+    if (ret)
-	return ret;
+	goto out_keep_error;
-    sp = krb5_storage_from_data (&reply);
-    if (sp == NULL) {
-	krb5_clear_error_message(context->context);
-	krb5_data_free (&reply);
-	return ENOMEM;
-    }
-    krb5_ret_int32(sp, &tmp);
-    krb5_clear_error_message(context->context);
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    sp = krb5_storage_from_data(&reply);
-    return tmp;
+    if (sp == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
+    krb5_storage_free(sp);
+    krb5_data_free(&reply);
+    return ret;
 }
 

lib/kadm5/delete_c.c

@@ -46,32 +46,43 @@ kadm5_c_delete_principal(void *server_handle, krb5_principal princ)
     krb5_data reply;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_delete);
+    ret = krb5_store_int32(sp, kadm_delete);
-    krb5_store_principal(sp, princ);
-    ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
-	return ret;
+	goto out;
+    ret = krb5_store_principal(sp, princ);
+    if (ret)
+	goto out;
+    ret = _kadm5_client_send(context, sp);
+    if (ret)
+	goto out_keep_error;
     ret = _kadm5_client_recv(context, &reply);
     if (ret)
-	return ret;
+	goto out_keep_error;
-    sp = krb5_storage_from_data (&reply);
-    if(sp == NULL) {
-	krb5_clear_error_message(context->context);
-	krb5_data_free (&reply);
-	return ENOMEM;
-    }
-    krb5_ret_int32(sp, &tmp);
-    krb5_clear_error_message(context->context);
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    sp = krb5_storage_from_data(&reply);
-    return tmp;
+    if (sp == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
+    krb5_storage_free(sp);
+    krb5_data_free(&reply);
+    return ret;
 }

lib/kadm5/get_c.c

@@ -38,7 +38,7 @@ RCSID("$Id$");
 kadm5_ret_t
 kadm5_c_get_principal(void *server_handle,
 		      krb5_principal princ,
-		      kadm5_principal_ent_t out,
+		      kadm5_principal_ent_t ent,
 		      uint32_t mask)
 {
     kadm5_client_context *context = server_handle;
@@ -49,36 +49,48 @@ kadm5_c_get_principal(void *server_handle,
     krb5_data reply;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_get);
+    ret = krb5_store_int32(sp, kadm_get);
-    krb5_store_principal(sp, princ);
+    if (ret)
-    krb5_store_int32(sp, mask);
+	goto out;
+    ret = krb5_store_principal(sp, princ);
+    if (ret)
+	goto out;
+    ret = krb5_store_int32(sp, mask);
+    if (ret)
+	goto out;
     ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
+    if (ret)
-    if(ret)
+	goto out_keep_error;
-	return ret;
     ret = _kadm5_client_recv(context, &reply);
     if (ret)
-	return ret;
+	goto out_keep_error;
-    sp = krb5_storage_from_data (&reply);
-    if (sp == NULL) {
-	krb5_clear_error_message(context->context);
-	krb5_data_free (&reply);
-	return ENOMEM;
-    }
-    krb5_ret_int32(sp, &tmp);
-    ret = tmp;
-    krb5_clear_error_message(context->context);
-    if(ret == 0)
-	kadm5_ret_principal_ent(sp, out);
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    sp = krb5_storage_from_data(&reply);
+    if (sp == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
+    if (ret == 0)
+	kadm5_ret_principal_ent(sp, ent);
+    krb5_storage_free(sp);
+    krb5_data_free(&reply);
     return ret;
 }

lib/kadm5/get_princs_c.c

@@ -47,46 +47,73 @@ kadm5_c_get_principals(void *server_handle,
     unsigned char buf[1024];
     int32_t tmp;
     krb5_data reply;
+    int i;
+
+    *count = 0;
+    *princs = NULL;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
-	return ret;
-
-    sp = krb5_storage_from_mem(buf, sizeof(buf));
-    if (sp == NULL)
-	return ENOMEM;
-    krb5_store_int32(sp, kadm_get_princs);
-    krb5_store_int32(sp, expression != NULL);
-    if(expression)
-	krb5_store_string(sp, expression);
-    ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
 	return ret;
+
+    krb5_data_zero(&reply);
+
+    sp = krb5_storage_from_mem(buf, sizeof(buf));
+    if (sp == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    ret = krb5_store_int32(sp, kadm_get_princs);
+    if (ret)
+	goto out;
+    ret = krb5_store_int32(sp, expression != NULL);
+    if (ret)
+	goto out;
+    if (expression) {
+	ret = krb5_store_string(sp, expression);
+	if (ret)
+	    goto out;
+    }
+    ret = _kadm5_client_send(context, sp);
+    if (ret)
+	goto out_keep_error;
     ret = _kadm5_client_recv(context, &reply);
-    if(ret)
+    if (ret)
-	return ret;
+	goto out_keep_error;
+    krb5_storage_free(sp);
     sp = krb5_storage_from_data (&reply);
     if (sp == NULL) {
-	krb5_data_free (&reply);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_ret_int32(sp, &tmp);
+    ret = krb5_ret_int32(sp, &tmp);
-    ret = tmp;
+    if (ret == 0)
-    if(ret == 0) {
+	ret = tmp;
-	int i;
+
-	krb5_ret_int32(sp, &tmp);
+    if (ret)
-	*princs = calloc(tmp + 1, sizeof(**princs));
+	goto out;
-	if (*princs == NULL) {
+
-	    ret = ENOMEM;
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret)
+	goto out;
+
+    *princs = calloc(tmp + 1, sizeof(**princs));
+    if (*princs == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    for (i = 0; i < tmp; i++) {
+	ret = krb5_ret_string(sp, &(*princs)[i]);
+	if (ret)
 	    goto out;
-	}
-	for(i = 0; i < tmp; i++)
-	    krb5_ret_string(sp, &(*princs)[i]);
-	*count = tmp;
     }
-out:
+    *count = tmp;
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    krb5_data_free(&reply);
     return ret;
 }

lib/kadm5/modify_c.c

@@ -48,34 +48,46 @@ kadm5_c_modify_principal(void *server_handle,
     krb5_data reply;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_modify);
+    ret = krb5_store_int32(sp, kadm_modify);
-    kadm5_store_principal_ent(sp, princ);
+    if (ret)
-    krb5_store_int32(sp, mask);
+	goto out;
+    ret = kadm5_store_principal_ent(sp, princ);
+    if (ret)
+	goto out;
+    ret = krb5_store_int32(sp, mask);
+    if (ret)
+	goto out;
     ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
+    if (ret)
-    if(ret)
+	goto out_keep_error;
-	return ret;
     ret = _kadm5_client_recv(context, &reply);
-    if(ret)
+    if (ret)
-	return ret;
+	goto out_keep_error;
-    sp = krb5_storage_from_data (&reply);
-    if (sp == NULL) {
-	krb5_clear_error_message(context->context);
-	krb5_data_free (&reply);
-	return ENOMEM;
-    }
-    krb5_ret_int32(sp, &tmp);
-    krb5_clear_error_message(context->context);
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    sp = krb5_storage_from_data(&reply);
-    return tmp;
+    if (sp == NULL) {
-}
+	ret = ENOMEM;
+	goto out;
+    }
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
 
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
+    krb5_storage_free(sp);
+    krb5_data_free(&reply);
+    return ret;
+}

lib/kadm5/privs_c.c

@@ -48,34 +48,41 @@ kadm5_c_get_privs(void *server_handle, uint32_t *privs)
     *privs = 0;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_store_int32(sp, kadm_get_privs);
+    ret = krb5_store_int32(sp, kadm_get_privs);
+    if (ret)
+	goto out;
     ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
+    if (ret)
-    if(ret)
+	goto out_keep_error;
-	return ret;
     ret = _kadm5_client_recv(context, &reply);
     if (ret)
-	return ret;
+	goto out_keep_error;
+    krb5_storage_free(sp);
     sp = krb5_storage_from_data(&reply);
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	krb5_data_free (&reply);
+	goto out;
-	return ENOMEM;
     }
-    krb5_ret_int32(sp, &tmp);
+    ret = krb5_ret_int32(sp, &tmp);
-    krb5_clear_error_message(context->context);
+    if (ret == 0)
-    ret = tmp;
+	ret = tmp;
-    if(ret == 0){
+    if (ret == 0)
 	krb5_ret_uint32(sp, privs);
-    }
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
     krb5_storage_free(sp);
     krb5_data_free (&reply);
     return ret;

lib/kadm5/randkey_c.c

@@ -51,15 +51,18 @@ kadm5_c_randkey_principal(void *server_handle,
     int32_t tmp;
     size_t i;
     krb5_data reply;
+    krb5_keyblock *k;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
+    if (ret)
 	return ret;
 
+    krb5_data_zero(&reply);
+
     sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
 
     /*
@@ -92,56 +95,60 @@ kadm5_c_randkey_principal(void *server_handle,
         if (ret == 0)
             krb5_store_int32(sp, ks_tuple[i].ks_salttype);
     }
-    if (ret)
-        return ret;
     /* Future extensions go here */
+    if (ret)
+	goto out;
 
     ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
-	return ret;
+	goto out_keep_error;
     ret = _kadm5_client_recv(context, &reply);
-    if(ret)
+    if (ret)
-	return ret;
+	goto out_keep_error;
+    krb5_storage_free(sp);
     sp = krb5_storage_from_data(&reply);
     if (sp == NULL) {
-	krb5_clear_error_message(context->context);
+	ret = ENOMEM;
-	krb5_data_free (&reply);
+	goto out;
-	return ENOMEM;
     }
-    krb5_clear_error_message(context->context);
     ret = krb5_ret_int32(sp, &tmp);
     if (ret == 0)
         ret = tmp;
-    if (ret == 0){
+    if (ret)
-	krb5_keyblock *k;
+	goto out;
 
-	ret = krb5_ret_int32(sp, &tmp);
+    ret = krb5_ret_int32(sp, &tmp);
-        if (ret)
+    if (ret)
-            goto out;
+	goto out;
-	if (tmp < 0) {
+    if (tmp < 0) {
-	    ret = EOVERFLOW;
+	ret = EOVERFLOW;
-	    goto out;
+	goto out;
-	}
-	k = calloc(tmp, sizeof(*k));
-	if (k == NULL) {
-	    ret = ENOMEM;
-	    goto out;
-	}
-	for(i = 0; ret == 0 && i < tmp; i++)
-	    ret = krb5_ret_keyblock(sp, &k[i]);
-	if (ret == 0 && n_keys && new_keys) {
-	    *n_keys = tmp;
-	    *new_keys = k;
-	} else {
-            krb5_free_keyblock_contents(context->context, &k[i]);
-            for (; i > 0; i--)
-                krb5_free_keyblock_contents(context->context, &k[i - 1]);
-            free(k);
-        }
     }
-out:
+    k = calloc(tmp, sizeof(*k));
+    if (k == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    for (i = 0; ret == 0 && i < tmp; i++) {
+	ret = krb5_ret_keyblock(sp, &k[i]);
+	if (ret)
+	    goto out;
+    }
+    if (ret == 0 && n_keys && new_keys) {
+	*n_keys = tmp;
+	*new_keys = k;
+    } else {
+	krb5_free_keyblock_contents(context->context, &k[i]);
+	for (; i > 0; i--)
+	    krb5_free_keyblock_contents(context->context, &k[i - 1]);
+	free(k);
+    }
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    krb5_data_free(&reply);
     return ret;
 }

lib/kadm5/rename_c.c

@@ -48,30 +48,47 @@ kadm5_c_rename_principal(void *server_handle,
     krb5_data reply;
 
     ret = _kadm5_connect(server_handle);
-    if(ret)
-	return ret;
-
-    sp = krb5_storage_from_mem(buf, sizeof(buf));
-    if (sp == NULL)
-	return ENOMEM;
-    krb5_store_int32(sp, kadm_rename);
-    krb5_store_principal(sp, source);
-    krb5_store_principal(sp, target);
-    ret = _kadm5_client_send(context, sp);
-    krb5_storage_free(sp);
     if (ret)
 	return ret;
-    ret = _kadm5_client_recv(context, &reply);
+
-    if(ret)
+    krb5_data_zero(&reply);
-	return ret;
+
-    sp = krb5_storage_from_data (&reply);
+    sp = krb5_storage_from_mem(buf, sizeof(buf));
     if (sp == NULL) {
-	krb5_data_free (&reply);
+	ret = ENOMEM;
-	return ENOMEM;
+	goto out;
     }
-    krb5_ret_int32(sp, &tmp);
+
-    ret = tmp;
+    ret = krb5_store_int32(sp, kadm_rename);
+    if (ret)
+	goto out;
+    ret = krb5_store_principal(sp, source);
+    if (ret)
+	goto out;
+    ret = krb5_store_principal(sp, target);
+    if (ret)
+	goto out;
+    ret = _kadm5_client_send(context, sp);
+    if (ret)
+	goto out_keep_error;
+    ret = _kadm5_client_recv(context, &reply);
+    if (ret)
+	goto out_keep_error;
     krb5_storage_free(sp);
-    krb5_data_free (&reply);
+    sp = krb5_storage_from_data(&reply);
+    if (sp == NULL) {
+	ret = ENOMEM;
+	goto out;
+    }
+    ret = krb5_ret_int32(sp, &tmp);
+    if (ret == 0)
+	ret = tmp;
+
+  out:
+    krb5_clear_error_message(context->context);
+
+  out_keep_error:
+    krb5_storage_free(sp);
+    krb5_data_free(&reply);
     return ret;
 }