s4:kdc: Have smb_krb5_principal_get_comp_string() properly indicate an error

The existing implementation did not differentiate between the case where
the relevant component was not present, and that where talloc_strndup()
failed. To correct this situation, put the result into an out parameter
on success and return an error on failure.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15482

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c
index 062e05ead34a6a48ee0b6e982148f5d7e9037f40..80c9f747e1d22eca2a1cce3db600a1966015db5e 100644 (file)
--- a/lib/krb5_wrap/krb5_samba.c
+++ b/lib/krb5_wrap/krb5_samba.c
@@ -1047,39 +1047,50 @@ done:
  * @param[in] context          The krb5_context
  * @param[in] principal                The principal
  * @param[in] component                The component
- * @return string component
+ * @param[out] out                     The output string
+ * @return krb5_error_code
  *
  * Caller must talloc_free if the return value is not NULL.
  *
  */
-char *smb_krb5_principal_get_comp_string(TALLOC_CTX *mem_ctx,
-                                        krb5_context context,
-                                        krb5_const_principal principal,
-                                        unsigned int component)
+krb5_error_code smb_krb5_principal_get_comp_string(TALLOC_CTX *mem_ctx,
+                                                  krb5_context context,
+                                                  krb5_const_principal principal,
+                                                  unsigned int component,
+                                                  char **out)
 {
+       char *out_str = NULL;
 #if defined(HAVE_KRB5_PRINCIPAL_GET_COMP_STRING)
        const char *str = NULL;
 
        str = krb5_principal_get_comp_string(context, principal, component);
        if (str == NULL) {
-               return NULL;
+               return ENOENT;
        }
 
-       return talloc_strdup(mem_ctx, str);
+       out_str = talloc_strdup(mem_ctx, str);
+       if (out_str == NULL) {
+               return ENOMEM;
+       }
 #else
        krb5_data *data;
 
        if (component >= krb5_princ_size(context, principal)) {
-               return NULL;
+               return ENOENT;
        }
 
        data = krb5_princ_component(context, principal, component);
        if (data == NULL) {
-               return NULL;
+               return ENOENT;
        }
 
-       return talloc_strndup(mem_ctx, data->data, data->length);
+       out_str = talloc_strndup(mem_ctx, data->data, data->length);
+       if (out_str == NULL) {
+               return ENOMEM;
+       }
 #endif
+       *out = out_str;
+       return 0;
 }
 
 /**
@@ -3441,9 +3452,12 @@ int smb_krb5_principal_is_tgs(krb5_context context,
 {
        char *p = NULL;
        int eq = 1;
+       krb5_error_code ret = 0;
 
-       p = smb_krb5_principal_get_comp_string(NULL, context, principal, 0);
-       if (p == NULL) {
+       ret = smb_krb5_principal_get_comp_string(NULL, context, principal, 0, &p);
+       if (ret == ENOENT) {
+               return 0;
+       } else if (ret) {
                return -1;
        }
 

diff --git a/lib/krb5_wrap/krb5_samba.h b/lib/krb5_wrap/krb5_samba.h
index 7b9d8fd145c9cffa1d27315f26becdc9f752accb..e158a404dea18e2c2ba419d741c62b99a5ca7bce 100644 (file)
--- a/lib/krb5_wrap/krb5_samba.h
+++ b/lib/krb5_wrap/krb5_samba.h
@@ -400,10 +400,11 @@ int smb_krb5_create_key_from_string(krb5_context context,
 #endif
 #endif
 
-char *smb_krb5_principal_get_comp_string(TALLOC_CTX *mem_ctx,
-                                        krb5_context context,
-                                        krb5_const_principal principal,
-                                        unsigned int component);
+krb5_error_code smb_krb5_principal_get_comp_string(TALLOC_CTX *mem_ctx,
+                                                  krb5_context context,
+                                                  krb5_const_principal principal,
+                                                  unsigned int component,
+                                                  char **out);
 
 krb5_error_code smb_krb5_copy_data_contents(krb5_data *p,
                                            const void *data,

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 69709af4cfe9748c83888328f48608d6d1fd5882..b8ee21b2b9270ef83d5bae00971ee7cc2c7be38d 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -456,7 +456,7 @@ static int acl_validate_spn_value(TALLOC_CTX *mem_ctx,
                                  const char *netbios_name,
                                  const char *ntds_guid)
 {
-       int ret, princ_size;
+       krb5_error_code ret, princ_size;
        krb5_context krb_ctx;
        krb5_error_code kerr;
        krb5_principal principal;
@@ -509,13 +509,22 @@ static int acl_validate_spn_value(TALLOC_CTX *mem_ctx,
                goto fail;
        }
 
-       instanceName = smb_krb5_principal_get_comp_string(mem_ctx, krb_ctx,
-                                                         principal, 1);
-       serviceType = smb_krb5_principal_get_comp_string(mem_ctx, krb_ctx,
-                                                        principal, 0);
+       ret = smb_krb5_principal_get_comp_string(mem_ctx, krb_ctx,
+                                                         principal, 1, &instanceName);
+       if (ret) {
+               goto fail;
+       }
+       ret = smb_krb5_principal_get_comp_string(mem_ctx, krb_ctx,
+                                                principal, 0, &serviceType);
+       if (ret) {
+               goto fail;
+       }
        if (krb5_princ_size(krb_ctx, principal) == 3) {
-               serviceName = smb_krb5_principal_get_comp_string(mem_ctx, krb_ctx,
-                                                                principal, 2);
+               ret = smb_krb5_principal_get_comp_string(mem_ctx, krb_ctx,
+                                                        principal, 2, &serviceName);
+               if (ret) {
+                       goto fail;
+               }
        }
 
        if (serviceName) {

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 38fac0d5ee88fc9aa23962bb4fb872cca391db07..91449c258d6d331fbf3e68fe6cf8a970b0f9f92a 100644 (file)
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -1233,12 +1233,12 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
                bool is_our_realm;
                bool is_dc;
 
-               third_part = smb_krb5_principal_get_comp_string(tmp_ctx,
-                                                               context,
-                                                               principal,
-                                                               2);
-               if (third_part == NULL) {
-                       ret = ENOMEM;
+               ret = smb_krb5_principal_get_comp_string(tmp_ctx,
+                                                        context,
+                                                        principal,
+                                                        2,
+                                                        &third_part);
+               if (ret) {
                        krb5_set_error_message(context, ret, "smb_krb5_principal_get_comp_string: out of memory");
                        goto out;
                }
@@ -2265,10 +2265,12 @@ static krb5_error_code samba_kdc_lookup_client(krb5_context context,
        char *principal_string = NULL;
 
        if (smb_krb5_principal_get_type(context, principal) == KRB5_NT_ENTERPRISE_PRINCIPAL) {
-               principal_string = smb_krb5_principal_get_comp_string(mem_ctx, context,
-                                                                     principal, 0);
-               if (principal_string == NULL) {
-                       return ENOMEM;
+               krb5_error_code ret = 0;
+
+               ret = smb_krb5_principal_get_comp_string(mem_ctx, context,
+                                                        principal, 0, &principal_string);
+               if (ret) {
+                       return ret;
                }
        } else {
                char *principal_string_m = NULL;
@@ -2314,12 +2316,12 @@ static krb5_error_code samba_kdc_lookup_client(krb5_context context,
                if (num_comp == 1) {
                        size_t len;
 
-                       fallback_account = smb_krb5_principal_get_comp_string(mem_ctx,
-                                               context, fallback_principal, 0);
-                       if (fallback_account == NULL) {
+                       ret = smb_krb5_principal_get_comp_string(mem_ctx,
+                                                                context, fallback_principal, 0, &fallback_account);
+                       if (ret) {
                                krb5_free_principal(context, fallback_principal);
                                TALLOC_FREE(fallback_realm);
-                               return ENOMEM;
+                               return ret;
                        }
 
                        len = strlen(fallback_account);
@@ -2452,7 +2454,12 @@ static krb5_error_code samba_kdc_fetch_krbtgt(krb5_context context,
 
        /* krbtgt case.  Either us or a trusted realm */
 
-       realm_princ_comp = smb_krb5_principal_get_comp_string(tmp_ctx, context, principal, 1);
+       ret = smb_krb5_principal_get_comp_string(tmp_ctx, context, principal, 1, &realm_princ_comp);
+       if (ret == ENOENT) {
+               /* OK. */
+       } else if (ret) {
+               goto out;
+       }
 
        if (lpcfg_is_my_domain_or_realm(lp_ctx, realm_from_princ)
            && lpcfg_is_my_domain_or_realm(lp_ctx, realm_princ_comp)) {
@@ -2659,8 +2666,8 @@ static krb5_error_code samba_kdc_lookup_server(krb5_context context,
                                                       krb5_princ_size(context, principal));
                                return ret;
                        }
-                       str = smb_krb5_principal_get_comp_string(mem_ctx, context, principal, 0);
-                       if (str == NULL) {
+                       ret = smb_krb5_principal_get_comp_string(mem_ctx, context, principal, 0, &str);
+                       if (ret) {
                                return KRB5_PARSE_MALFORMED;
                        }
                        ret = krb5_parse_name(context, str,
@@ -2850,11 +2857,11 @@ static krb5_error_code samba_kdc_lookup_realm(krb5_context context,
                        return SDB_ERR_NOENTRY;
                }
 
-               principal_string = smb_krb5_principal_get_comp_string(frame, context,
-                                                                     principal, 0);
-               if (principal_string == NULL) {
+               ret = smb_krb5_principal_get_comp_string(frame, context,
+                                                        principal, 0, &principal_string);
+               if (ret) {
                        TALLOC_FREE(frame);
-                       return ENOMEM;
+                       return ret;
                }
 
                ret = krb5_parse_name(context, principal_string,
@@ -2874,8 +2881,6 @@ static krb5_error_code samba_kdc_lookup_realm(krb5_context context,
        }
 
        if (flags & SDB_F_GET_SERVER) {
-               char *service_realm = NULL;
-
                ret = principal_comp_strcmp(context, principal, 0, KRB5_TGS_NAME);
                if (ret == 0) {
                        /*
@@ -2897,14 +2902,19 @@ static krb5_error_code samba_kdc_lookup_realm(krb5_context context,
                 */
 
                if (num_comp == 2 || num_comp == 3) {
-                       service_realm = smb_krb5_principal_get_comp_string(frame,
-                                                                          context,
-                                                                          principal,
-                                                                          num_comp - 1);
-               }
+                       char *service_realm = NULL;
 
-               if (service_realm != NULL) {
-                       realm = service_realm;
+                       ret = smb_krb5_principal_get_comp_string(frame,
+                                                                context,
+                                                                principal,
+                                                                num_comp - 1,
+                                                                &service_realm);
+                       if (ret) {
+                               TALLOC_FREE(frame);
+                               return ret;
+                       } else {
+                               realm = service_realm;
+                       }
                }
        }