git: 80999dcd5bde - main - heimdal: Add missing kadmind error checks

From: Cy Schubert <cy_at_FreeBSD.org>
Date: Sun, 27 Nov 2022 02:44:05 UTC
The branch main has been updated by cy:

URL: https://cgit.FreeBSD.org/src/commit/?id=80999dcd5bdebd0beb2591a1de3f6db9767c0066

commit 80999dcd5bdebd0beb2591a1de3f6db9767c0066
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-26 17:48:31 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-11-27 02:41:52 +0000

    heimdal: Add missing kadmind error checks
    
    Inspired by:    Heimdal commmit 1b213c1082be4ef5a1c23928d614c762f837dbe7
    MFC after:      3 days
---
 crypto/heimdal/kadmin/server.c | 108 ++++++++++++++++++++++++-----------------
 1 file changed, 63 insertions(+), 45 deletions(-)

diff --git a/crypto/heimdal/kadmin/server.c b/crypto/heimdal/kadmin/server.c
index 2800a2e1fc29..ed6ba5a1f790 100644
--- a/crypto/heimdal/kadmin/server.c
+++ b/crypto/heimdal/kadmin/server.c
@@ -38,7 +38,8 @@ static kadm5_ret_t
 kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 		 krb5_data *in, krb5_data *out)
 {
-    kadm5_ret_t ret;
+    kadm5_ret_t ret = 0;
+    kadm5_ret_t ret_sp = 0;
     int32_t cmd, mask, tmp;
     kadm5_server_context *contextp = kadm_handlep;
     char client[128], name[128], name2[128];
@@ -50,16 +51,23 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
     int n_keys;
     char **princs;
     int n_princs;
-    krb5_storage *sp;
+    krb5_storage *rsp = NULL; /* response goes here */
+    krb5_storage *sp = NULL;
 
-    krb5_unparse_name_fixed(contextp->context, contextp->caller,
+    memset(&ent, 0, sizeof(ent));
+    krb5_data_zero(out);
+    ret = krb5_unparse_name_fixed(contextp->context, contextp->caller,
 			    client, sizeof(client));
 
     sp = krb5_storage_from_data(in);
     if (sp == NULL)
 	krb5_errx(contextp->context, 1, "out of memory");
 
-    krb5_ret_int32(sp, &cmd);
+    ret = krb5_ret_int32(sp, &cmd);
+    if (ret) {
+	krb5_storage_free(sp);
+	goto fail;
+    }
     switch(cmd){
     case kadm_get:{
 	op = "GET";
@@ -72,9 +80,10 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	    goto fail;
 	}
 	mask |= KADM5_PRINCIPAL;
-	krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
+	ret = krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
-	ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_GET, princ);
+	if (ret == 0)
+	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_GET, princ);
 	if(ret){
 	    krb5_free_principal(contextp->context, princ);
 	    goto fail;
@@ -95,9 +104,10 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	ret = krb5_ret_principal(sp, &princ);
 	if(ret)
 	    goto fail;
-	krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
+	ret = krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
-	ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_DELETE, princ);
+	if (ret == 0)
+	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_DELETE, princ);
 	if(ret){
 	    krb5_free_principal(contextp->context, princ);
 	    goto fail;
@@ -124,10 +134,11 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	    kadm5_free_principal_ent(contextp->context, &ent);
 	    goto fail;
 	}
-	krb5_unparse_name_fixed(contextp->context, ent.principal,
+	ret = krb5_unparse_name_fixed(contextp->context, ent.principal,
 				name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
-	ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_ADD,
+	if (ret == 0)
+	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_ADD,
 					  ent.principal);
 	if(ret){
 	    kadm5_free_principal_ent(contextp->context, &ent);
@@ -155,10 +166,11 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	    kadm5_free_principal_ent(contextp, &ent);
 	    goto fail;
 	}
-	krb5_unparse_name_fixed(contextp->context, ent.principal,
+	ret = krb5_unparse_name_fixed(contextp->context, ent.principal,
 				name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
-	ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_MODIFY,
+	if (ret == 0)
+	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_MODIFY,
 					  ent.principal);
 	if(ret){
 	    kadm5_free_principal_ent(contextp, &ent);
@@ -181,11 +193,13 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	    krb5_free_principal(contextp->context, princ);
 	    goto fail;
 	}
-	krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
-	krb5_unparse_name_fixed(contextp->context, princ2, name2, sizeof(name2));
+	ret = krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
+	if (ret == 0)
+	    ret = krb5_unparse_name_fixed(contextp->context, princ2, name2, sizeof(name2));
 	krb5_warnx(contextp->context, "%s: %s %s -> %s",
 		   client, op, name, name2);
-	ret = _kadm5_acl_check_permission(contextp,
+	if (ret == 0)
+	    ret = _kadm5_acl_check_permission(contextp,
 					  KADM5_PRIV_ADD,
 					  princ2)
 	    || _kadm5_acl_check_permission(contextp,
@@ -214,7 +228,7 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	    krb5_free_principal(contextp->context, princ);
 	    goto fail;
 	}
-	krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
+	ret = krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
 
 	/*
@@ -227,26 +241,28 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	 * c) the user is on the CPW ACL.
 	 */
 
-	if (krb5_config_get_bool_default(contextp->context, NULL, TRUE,
-					 "kadmin", "allow_self_change_password", NULL)
-	    && initial
-	    && krb5_principal_compare (contextp->context, contextp->caller,
-				       princ))
-	{
-	    krb5_data pwd_data;
-	    const char *pwd_reason;
-
-	    pwd_data.data = password;
-	    pwd_data.length = strlen(password);
-
-	    pwd_reason = kadm5_check_password_quality (contextp->context,
-						       princ, &pwd_data);
-	    if (pwd_reason != NULL)
-		ret = KADM5_PASS_Q_DICT;
-	    else
-		ret = 0;
-	} else
-	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
+	if (ret == 0) {
+	    if (krb5_config_get_bool_default(contextp->context, NULL, TRUE,
+						 "kadmin", "allow_self_change_password", NULL)
+		&& initial
+		&& krb5_principal_compare (contextp->context, contextp->caller,
+					       princ))
+		{
+		    krb5_data pwd_data;
+		    const char *pwd_reason;
+
+		    pwd_data.data = password;
+		    pwd_data.length = strlen(password);
+
+		    pwd_reason = kadm5_check_password_quality (contextp->context,
+							       princ, &pwd_data);
+		    if (pwd_reason != NULL)
+			ret = KADM5_PASS_Q_DICT;
+		    else
+			ret = 0;
+		} else
+		    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
+	}
 
 	if(ret) {
 	    krb5_free_principal(contextp->context, princ);
@@ -304,7 +320,7 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	    }
 	}
 
-	krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
+	ret = krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
 
 	/*
@@ -312,7 +328,8 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	 * this it to force password quality check on the user.
 	 */
 
-	ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
+	if (ret == 0)
+	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
 	if(ret) {
 	    int16_t dummy = n_key_data;
 
@@ -339,7 +356,7 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	ret = krb5_ret_principal(sp, &princ);
 	if(ret)
 	    goto fail;
-	krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
+	ret = krb5_unparse_name_fixed(contextp->context, princ, name, sizeof(name));
 	krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
 	/*
 	 * The change is allowed if at least one of:
@@ -347,13 +364,14 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
 	 * b) the user is on the CPW ACL.
 	 */
 
-	if (initial
-	    && krb5_principal_compare (contextp->context, contextp->caller,
+	if (ret == 0) {
+	    if (initial
+		&& krb5_principal_compare (contextp->context, contextp->caller,
 				       princ))
-	    ret = 0;
-	else
-	    ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
-
+		ret = 0;
+	    else
+		ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
+	}
 	if(ret) {
 	    krb5_free_principal(contextp->context, princ);
 	    goto fail;