From: Joseph Sutton Date: Wed, 20 Sep 2023 22:41:05 +0000 (+1200) Subject: s4:kdc: Have smb_krb5_principal_get_comp_string() properly indicate an error X-Git-Tag: talloc-2.4.2~1009 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9a0c5ee4aefac943ee21e93af643b44e336c3563;p=thirdparty%2Fsamba.git 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 Reviewed-by: Andrew Bartlett --- diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c index 062e05ead34..80c9f747e1d 100644 --- 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 7b9d8fd145c..e158a404dea 100644 --- 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 69709af4cfe..b8ee21b2b92 100644 --- 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 38fac0d5ee8..91449c258d6 100644 --- 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; + } } }