From: Greg Hudson Date: Tue, 29 Jan 2013 01:15:01 +0000 (-0500) Subject: Refactor LDAP DB option parsing code X-Git-Tag: krb5-1.12-alpha1~323 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0b1dc2f93da4c860dd27f1ac997617b712dff383;p=thirdparty%2Fkrb5.git Refactor LDAP DB option parsing code krb5_ldap_open and krb5_ldap_create contain two large, almost identical blocks of DB option processing code. Factor it out into a new function krb5_ldap_parse_db_params in ldap_misc.c, and simplify the factored-out code. Create a helper function to add server entries and use it to simplify krb5_ldap_read_server_params as well as DB option parsing. Since the new DB option helper uses isspace instead of isblank, we no longer require portability goop for isblank. --- diff --git a/src/configure.in b/src/configure.in index c5c599b57b..600cc8d905 100644 --- a/src/configure.in +++ b/src/configure.in @@ -120,7 +120,6 @@ KRB5_NEED_PROTO([#include /* Solaris 8 declares swab in stdlib.h. */ #include ],swab,1) -KRB5_NEED_PROTO([#include ],isblank,1) AC_PROG_AWK KRB5_AC_INET6 @@ -263,7 +262,7 @@ AC_CONFIG_HEADERS(include/autoconf.h, [echo timestamp > include/autoconf.stamp]) AC_PROG_LEX AC_C_CONST AC_HEADER_DIRENT -AC_CHECK_FUNCS(strdup setvbuf seteuid setresuid setreuid setegid setresgid setregid setsid flock fchmod chmod strftime strptime geteuid setenv unsetenv getenv gmtime_r localtime_r bswap16 bswap64 mkstemp getusershell access getcwd srand48 srand srandom stat strchr strerror strerror_r timegm isblank) +AC_CHECK_FUNCS(strdup setvbuf seteuid setresuid setreuid setegid setresgid setregid setsid flock fchmod chmod strftime strptime geteuid setenv unsetenv getenv gmtime_r localtime_r bswap16 bswap64 mkstemp getusershell access getcwd srand48 srand srandom stat strchr strerror strerror_r timegm) AC_CHECK_FUNC(mkstemp, [MKSTEMP_ST_OBJ= diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c index a29b3326ec..e64d22d1dd 100644 --- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c +++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c @@ -39,55 +39,6 @@ #include #include -#if !defined(isblank) && defined(HAVE_ISBLANK) -#if defined(NEED_ISBLANK_PROTO) -extern int isblank(); -#endif -#else /* isblank missing */ -#if !defined(isblank) -#define isblank isspace -#endif -#endif - -krb5_error_code -krb5_ldap_get_db_opt(char *input, char **opt, char **val) -{ - char *pos = strchr(input, '='); - - *val = NULL; - if (pos == NULL) { - *opt = strdup(input); - if (*opt == NULL) { - return ENOMEM; - } - } else { - int len = pos - input; - *opt = malloc((unsigned) len + 1); - if (!*opt) { - return ENOMEM; - } - memcpy(*opt, input, (unsigned) len); - /* ignore trailing blanks */ - while (isblank((*opt)[len-1])) - --len; - (*opt)[len] = '\0'; - - pos += 1; /* move past '=' */ - while (isblank(*pos)) /* ignore leading blanks */ - pos += 1; - if (*pos != '\0') { - *val = strdup (pos); - if (!*val) { - free (*opt); - return ENOMEM; - } - } - } - return (0); - -} - - /* * ldap get age */ @@ -302,161 +253,27 @@ krb5_ldap_open(krb5_context context, char *conf_section, char **db_args, int mode) { krb5_error_code status = 0; - char **t_ptr = db_args; krb5_ldap_context *ldap_context=NULL; - int srv_cnt = 0; - kdb5_dal_handle *dal_handle=NULL; /* Clear the global error string */ krb5_clear_error_message(context); - ldap_context = calloc(1, sizeof(krb5_ldap_context)); - if (ldap_context == NULL) { - status = ENOMEM; + ldap_context = k5alloc(sizeof(krb5_ldap_context), &status); + if (ldap_context == NULL) goto clean_n_exit; - } - + context->dal_handle->db_context = ldap_context; ldap_context->kcontext = context; - while (t_ptr && *t_ptr) { - char *opt = NULL, *val = NULL; - - if ((status = krb5_ldap_get_db_opt(*t_ptr, &opt, &val)) != 0) { - goto clean_n_exit; - } - if (opt && !strcmp(opt, "binddn")) { - if (ldap_context->bind_dn) { - free (opt); - free (val); - status = EINVAL; - krb5_set_error_message(context, status, _("'binddn' missing")); - goto clean_n_exit; - } - if (val == NULL) { - status = EINVAL; - krb5_set_error_message(context, status, - _("'binddn' value missing")); - free(opt); - goto clean_n_exit; - } - ldap_context->bind_dn = strdup(val); - if (ldap_context->bind_dn == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto clean_n_exit; - } - } else if (opt && !strcmp(opt, "nconns")) { - if (ldap_context->max_server_conns) { - free (opt); - free (val); - status = EINVAL; - krb5_set_error_message(context, status, _("'nconns' missing")); - goto clean_n_exit; - } - if (val == NULL) { - status = EINVAL; - krb5_set_error_message(context, status, - _("'nconns' value missing")); - free(opt); - goto clean_n_exit; - } - ldap_context->max_server_conns = atoi(val) ? atoi(val) : DEFAULT_CONNS_PER_SERVER; - } else if (opt && !strcmp(opt, "bindpwd")) { - if (ldap_context->bind_pwd) { - free (opt); - free (val); - status = EINVAL; - krb5_set_error_message(context, status, - _("'bindpwd' missing")); - goto clean_n_exit; - } - if (val == NULL) { - status = EINVAL; - krb5_set_error_message(context, status, - _("'bindpwd' value missing")); - free(opt); - goto clean_n_exit; - } - ldap_context->bind_pwd = strdup(val); - if (ldap_context->bind_pwd == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto clean_n_exit; - } - } else if (opt && !strcmp(opt, "host")) { - if (val == NULL) { - status = EINVAL; - krb5_set_error_message(context, status, - _("'host' value missing")); - free(opt); - goto clean_n_exit; - } - if (ldap_context->server_info_list == NULL) - ldap_context->server_info_list = (krb5_ldap_server_info **) calloc (SERV_COUNT+1, sizeof (krb5_ldap_server_info *)) ; - - if (ldap_context->server_info_list == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto clean_n_exit; - } - - ldap_context->server_info_list[srv_cnt] = (krb5_ldap_server_info *) calloc (1, sizeof (krb5_ldap_server_info)); - if (ldap_context->server_info_list[srv_cnt] == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto clean_n_exit; - } - - ldap_context->server_info_list[srv_cnt]->server_status = NOTSET; - - ldap_context->server_info_list[srv_cnt]->server_name = strdup(val); - if (ldap_context->server_info_list[srv_cnt]->server_name == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto clean_n_exit; - } - - srv_cnt++; - } else { - /* ignore hash argument. Might have been passed from create */ - status = EINVAL; - if (opt && !strcmp(opt, "temporary")) { - /* - * temporary is passed in when kdb5_util load without -update is done. - * This is unsupported by the LDAP plugin. - */ - krb5_set_error_message(context, status, - _("open of LDAP directory aborted, " - "plugin requires -update argument")); - } else { - krb5_set_error_message (context, status, - _("unknown option \'%s\'"), - opt?opt:val); - } - free(opt); - free(val); - goto clean_n_exit; - } - - free(opt); - free(val); - t_ptr++; + status = krb5_ldap_parse_db_params(context, db_args); + if (status) { + prepend_err_str(context, _("Error processing LDAP DB params:"), + status, status); + goto clean_n_exit; } - dal_handle = context->dal_handle; - dal_handle->db_context = ldap_context; status = krb5_ldap_read_server_params(context, conf_section, mode & 0x0300); if (status) { - if (ldap_context) - krb5_ldap_free_ldap_context(ldap_context); - ldap_context = NULL; - dal_handle->db_context = NULL; - prepend_err_str(context, _("Error reading LDAP server params: "), + prepend_err_str(context, _("Error reading LDAP server params:"), status, status); goto clean_n_exit; } diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h index 30d3a4aef4..918df26773 100644 --- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h +++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h @@ -59,7 +59,6 @@ extern struct timeval timelimit; -#define SERV_COUNT 100 #define DEFAULT_CONNS_PER_SERVER 5 #define REALM_READ_REFRESH_INTERVAL (5 * 60) diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_create.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_create.c index 86282ea2b3..4fcf5a025f 100644 --- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_create.c +++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_create.c @@ -53,162 +53,32 @@ krb5_error_code krb5_ldap_create(krb5_context context, char *conf_section, char **db_args) { krb5_error_code status = 0; - char **t_ptr = db_args; krb5_ldap_realm_params *rparams = NULL; - kdb5_dal_handle *dal_handle = NULL; krb5_ldap_context *ldap_context=NULL; krb5_boolean realm_obj_created = FALSE; krb5_boolean krbcontainer_obj_created = FALSE; - int srv_cnt = 0; int mask = 0; /* Clear the global error string */ krb5_clear_error_message(context); - ldap_context = malloc(sizeof(krb5_ldap_context)); - if (ldap_context == NULL) { - status = ENOMEM; + ldap_context = k5alloc(sizeof(krb5_ldap_context), &status); + if (ldap_context == NULL) goto cleanup; - } - memset(ldap_context, 0, sizeof(*ldap_context)); - + context->dal_handle->db_context = ldap_context; ldap_context->kcontext = context; - /* populate ldap_context with ldap specific options */ - while (t_ptr && *t_ptr) { - char *opt = NULL, *val = NULL; - - if ((status = krb5_ldap_get_db_opt(*t_ptr, &opt, &val)) != 0) { - goto cleanup; - } - if (opt && !strcmp(opt, "binddn")) { - if (ldap_context->bind_dn) { - free (opt); - free (val); - status = EINVAL; - krb5_set_error_message (context, status, "'binddn' missing"); - goto cleanup; - } - if (val == NULL) { - status = EINVAL; - krb5_set_error_message (context, status, "'binddn' value missing"); - free(opt); - goto cleanup; - } - ldap_context->bind_dn = strdup(val); - if (ldap_context->bind_dn == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto cleanup; - } - } else if (opt && !strcmp(opt, "nconns")) { - if (ldap_context->max_server_conns) { - free (opt); - free (val); - status = EINVAL; - krb5_set_error_message (context, status, "'nconns' missing"); - goto cleanup; - } - if (val == NULL) { - status = EINVAL; - krb5_set_error_message (context, status, "'nconns' value missing"); - free(opt); - goto cleanup; - } - ldap_context->max_server_conns = atoi(val) ? atoi(val) : DEFAULT_CONNS_PER_SERVER; - } else if (opt && !strcmp(opt, "bindpwd")) { - if (ldap_context->bind_pwd) { - free (opt); - free (val); - status = EINVAL; - krb5_set_error_message (context, status, "'bindpwd' missing"); - goto cleanup; - } - if (val == NULL) { - status = EINVAL; - krb5_set_error_message (context, status, "'bindpwd' value missing"); - free(opt); - goto cleanup; - } - ldap_context->bind_pwd = strdup(val); - if (ldap_context->bind_pwd == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto cleanup; - } - } else if (opt && !strcmp(opt, "host")) { - if (val == NULL) { - status = EINVAL; - krb5_set_error_message (context, status, "'host' value missing"); - free(opt); - goto cleanup; - } - if (ldap_context->server_info_list == NULL) - ldap_context->server_info_list = - (krb5_ldap_server_info **) calloc(SERV_COUNT+1, sizeof(krb5_ldap_server_info *)); - - if (ldap_context->server_info_list == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto cleanup; - } - - ldap_context->server_info_list[srv_cnt] = - (krb5_ldap_server_info *) calloc(1, sizeof(krb5_ldap_server_info)); - if (ldap_context->server_info_list[srv_cnt] == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto cleanup; - } - - ldap_context->server_info_list[srv_cnt]->server_status = NOTSET; - - ldap_context->server_info_list[srv_cnt]->server_name = strdup(val); - if (ldap_context->server_info_list[srv_cnt]->server_name == NULL) { - free (opt); - free (val); - status = ENOMEM; - goto cleanup; - } - - srv_cnt++; - } else { - /* ignore hash argument. Might have been passed from create */ - status = EINVAL; - if (opt && !strcmp(opt, "temporary")) { - /* - * temporary is passed in when kdb5_util load without -update is done. - * This is unsupported by the LDAP plugin. - */ - krb5_set_error_message(context, status, - _("creation of LDAP entries aborted, " - "plugin requires -update argument")); - } else { - krb5_set_error_message(context, status, - _("unknown option \'%s\'"), - opt?opt:val); - } - free(opt); - free(val); - goto cleanup; - } - - free(opt); - free(val); - t_ptr++; + status = krb5_ldap_parse_db_params(context, db_args); + if (status) { + prepend_err_str(context, _("Error processing LDAP DB params:"), + status, status); + goto cleanup; } - dal_handle = context->dal_handle; - dal_handle->db_context = (kdb5_dal_handle *) ldap_context; - status = krb5_ldap_read_server_params(context, conf_section, KRB5_KDB_SRV_TYPE_ADMIN); if (status) { - dal_handle->db_context = NULL; - prepend_err_str (context, "Error reading LDAP server params: ", status, status); + prepend_err_str(context, _("Error reading LDAP server params:"), + status, status); goto cleanup; } status = krb5_ldap_db_init(context, ldap_context); @@ -253,7 +123,6 @@ krb5_ldap_create(krb5_context context, char *conf_section, char **db_args) goto cleanup; cleanup: - /* If the krbcontainer/realm creation is not complete, do the roll-back here */ if ((krbcontainer_obj_created) && (!realm_obj_created)) { int rc; @@ -267,5 +136,7 @@ cleanup: if (rparams) krb5_ldap_free_realm_params(rparams); + if (status) + krb5_ldap_close(context); return(status); } diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c index aba9e8eb10..9bfd90a950 100644 --- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c +++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c @@ -33,6 +33,7 @@ */ #include #include +#include #include "kdb_ldap.h" #include "ldap_misc.h" #include "ldap_handle.h" @@ -53,7 +54,7 @@ remove_overlapping_subtrees(char **listin, char **listop, int *subtcount, But all the world's not Linux. */ #undef strndup #define strndup my_strndup -#ifdef HAVE_LDAP_STR2DN + static char * my_strndup(const char *input, size_t limit) { @@ -69,7 +70,6 @@ my_strndup(const char *input, size_t limit) } else return strdup(input); } -#endif /* Get integer or string values from the config section, falling back to the default section, then to hard-coded values. */ @@ -164,7 +164,141 @@ prof_get_string_def(krb5_context ctx, const char *conf_section, return 0; } +static krb5_error_code +get_db_opt(const char *input, char **opt_out, char **val_out) +{ + const char *pos; + char *opt, *val = NULL; + size_t len; + + *opt_out = *val_out = NULL; + pos = strchr(input, '='); + if (pos == NULL) { + opt = strdup(input); + if (opt == NULL) + return ENOMEM; + } else { + len = pos - input; + /* Ignore trailing spaces. */ + while (len > 0 && isspace((unsigned char)input[len - 1])) + len--; + opt = strndup(input, len); + + pos++; /* Move past '='. */ + while (isspace(*pos)) /* Ignore leading spaces. */ + pos++; + if (*pos != '\0') { + val = strdup(pos); + if (val == NULL) { + free(opt); + return ENOMEM; + } + } + } + *opt_out = opt; + *val_out = val; + return 0; +} + +static krb5_error_code +add_server_entry(krb5_context context, const char *name) +{ + krb5_ldap_context *lctx = context->dal_handle->db_context; + krb5_ldap_server_info **sp, **list, *server; + size_t count = 0; + + /* Allocate list space for the new entry and null terminator. */ + for (sp = lctx->server_info_list; sp != NULL && *sp != NULL; sp++) + count++; + list = realloc(lctx->server_info_list, (count + 2) * sizeof(*list)); + if (list == NULL) + return ENOMEM; + lctx->server_info_list = list; + + server = calloc(1, sizeof(krb5_ldap_server_info)); + if (server == NULL) + return ENOMEM; + server->server_status = NOTSET; + server->server_name = strdup(name); + if (server->server_name == NULL) { + free(server); + return ENOMEM; + } + list[count] = server; + list[count + 1] = NULL; + return 0; +} + +krb5_error_code +krb5_ldap_parse_db_params(krb5_context context, char **db_args) +{ + char *opt = NULL, *val = NULL; + krb5_error_code status; + krb5_ldap_context *lctx = context->dal_handle->db_context; + + if (db_args == NULL) + return 0; + for (; *db_args != NULL; db_args++) { + status = get_db_opt(*db_args, &opt, &val); + if (status) + goto cleanup; + + /* Check for options which don't require values. */ + if (!strcmp(opt, "temporary")) { + /* "temporary" is passed by kdb5_util load without -update, + * which we don't support. */ + status = EINVAL; + krb5_set_error_message(context, status, + _("KDB module requires -update argument")); + goto cleanup; + } + + if (val == NULL) { + status = EINVAL; + krb5_set_error_message(context, status, _("'%s' value missing"), + opt); + goto cleanup; + } + + /* Check for options which do require arguments. */ + if (!strcmp(opt, "binddn")) { + free(lctx->bind_dn); + lctx->bind_dn = strdup(val); + if (lctx->bind_dn == NULL) { + status = ENOMEM; + goto cleanup; + } + } else if (!strcmp(opt, "nconns")) { + lctx->max_server_conns = atoi(val) ? atoi(val) : + DEFAULT_CONNS_PER_SERVER; + } else if (!strcmp(opt, "bindpwd")) { + free(lctx->bind_pwd); + lctx->bind_pwd = strdup(val); + if (lctx->bind_pwd == NULL) { + status = ENOMEM; + goto cleanup; + } + } else if (!strcmp(opt, "host")) { + status = add_server_entry(context, val); + if (status) + goto cleanup; + } else { + status = EINVAL; + krb5_set_error_message(context, status, _("unknown option '%s'"), + opt); + goto cleanup; + } + + free(opt); + free(val); + opt = val = NULL; + } +cleanup: + free(opt); + free(val); + return status; +} /* * This function reads the parameters from the krb5.conf file. The @@ -175,12 +309,11 @@ krb5_error_code krb5_ldap_read_server_params(krb5_context context, char *conf_section, int srv_type) { - char *tempval=NULL, *save_ptr=NULL; + char *tempval=NULL, *save_ptr=NULL, *item=NULL; const char *delims="\t\n\f\v\r ,"; krb5_error_code st=0; kdb5_dal_handle *dal_handle=NULL; krb5_ldap_context *ldap_context=NULL; - krb5_ldap_server_info ***server_info=NULL; dal_handle = context->dal_handle; ldap_context = (krb5_ldap_context *) dal_handle->db_context; @@ -269,17 +402,6 @@ krb5_ldap_read_server_params(krb5_context context, char *conf_section, */ if (ldap_context->server_info_list == NULL) { - unsigned int ele=0; - - server_info = &(ldap_context->server_info_list); - *server_info = (krb5_ldap_server_info **) calloc (SERV_COUNT+1, - sizeof (krb5_ldap_server_info *)); - - if (*server_info == NULL) { - st = ENOMEM; - goto cleanup; - } - if ((st=profile_get_string(context->profile, KDB_MODULE_SECTION, conf_section, KRB5_CONF_LDAP_SERVERS, NULL, &tempval)) != 0) { krb5_set_error_message(context, st, _("Error reading " @@ -288,36 +410,16 @@ krb5_ldap_read_server_params(krb5_context context, char *conf_section, } if (tempval == NULL) { - - (*server_info)[ele] = (krb5_ldap_server_info *)calloc(1, - sizeof(krb5_ldap_server_info)); - - (*server_info)[ele]->server_name = strdup("ldapi://"); - if ((*server_info)[ele]->server_name == NULL) { - st = ENOMEM; + st = add_server_entry(context, "ldapi://"); + if (st) goto cleanup; - } - (*server_info)[ele]->server_status = NOTSET; } else { - char *item=NULL; - - item = strtok_r(tempval,delims,&save_ptr); - while (item != NULL && eleserver_name = strdup(item); - if ((*server_info)[ele]->server_name == NULL) { - st = ENOMEM; + item = strtok_r(tempval, delims, &save_ptr); + while (item != NULL) { + st = add_server_entry(context, item); + if (st) goto cleanup; - } - - (*server_info)[ele]->server_status = NOTSET; item = strtok_r(NULL,delims,&save_ptr); - ++ele; } profile_release_string(tempval); } diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h index c2d4b0ebed..c3bb6c8907 100644 --- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h +++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h @@ -74,6 +74,9 @@ is_principal_in_realm(krb5_ldap_context *, krb5_const_principal); krb5_error_code krb5_get_subtree_info(krb5_ldap_context *, char ***, unsigned int *); +krb5_error_code +krb5_ldap_parse_db_params(krb5_context, char **); + krb5_error_code krb5_ldap_read_server_params(krb5_context , char *, int); @@ -119,9 +122,6 @@ krb5_ldap_policydn_to_name (krb5_context, char *, char **); krb5_error_code krb5_ldap_name_to_policydn (krb5_context, char *, char **); -krb5_error_code -krb5_ldap_get_db_opt(char *, char **, char **); - krb5_error_code populate_krb5_db_entry(krb5_context context, krb5_ldap_context *ldap_context, diff --git a/src/tests/t_kdb.py b/src/tests/t_kdb.py index 3c664f0ef4..e9769f76ee 100644 --- a/src/tests/t_kdb.py +++ b/src/tests/t_kdb.py @@ -249,7 +249,7 @@ realm.stop() dumpfile = os.path.join(realm.testdir, 'dump') realm.run([kdb5_util, 'dump', dumpfile]) out = realm.run([kdb5_util, 'load', dumpfile], expected_code=1) -if 'plugin requires -update argument' not in out: +if 'KDB module requires -update argument' not in out: fail('Unexpected error from kdb5_util load without -update') realm.run([kdb5_util, 'load', '-update', dumpfile])