From: Martin Hauser Subject: ypldap patch - log reduction, better overflow truncation for groups To: Date: Wed, 15 Jan 2025 11:32:55 +0100 Hello tech@, Observing ypldaps behaviour on my machine, I noticed it logging a lot of weird unknown users, those being upon closer examination issues coming from either groups containing users the full length format (cn=charlie,ou=admin,dc=org,dc=tld) or containing otherwise garbled characters. Below patch properly truncates groups that have more length then ypldap can process and also verifies usernames in groups to be correct. Feedback very welcome. Regards Martin See below: diff --git a/ldapclient.c b/ldapclient.c index 578653f..e457611 100644 --- a/ldapclient.c +++ b/ldapclient.c @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -49,9 +50,9 @@ void client_shutdown(void); void client_configure(struct env *); void client_periodic_update(int, short, void *); int client_build_req(struct idm *, struct idm_req *, struct aldap_message *, - int, int); + int, int, int (*)(char *)); int client_search_idm(struct env *, struct idm *, struct aldap *, - char **, char *, int, int, enum imsg_type); + char **, char *, int, int, enum imsg_type, int (*fun)(char *)); int client_try_idm(struct env *, struct idm *, struct ypldap_addr *); void client_addr_init(struct idm *); int client_addr_free(struct idm *); @@ -416,13 +417,37 @@ ldapclient(int pipe_main2client[2]) } +/* XXX copied from usr.sbin/user/user.c */ +static int +valid_user(char *login_name) +{ + unsigned char *cp; + + /* The first character cannot be a hyphen */ + if (*login_name == '-') + return 0; + + for (cp = login_name ; *cp ; cp++) { + /* We allow '$' as the last character for samba */ + if (!isalnum((unsigned char)*cp) && *cp != '.' && + *cp != '_' && *cp != '-' && + !(*cp == '$' && *(cp + 1) == '\0')) { + return 0; + } + } + if ((char *)cp - login_name > _PW_NAME_LEN) + return 0; + return 1; +} + int client_build_req(struct idm *idm, struct idm_req *ir, struct aldap_message *m, - int min_attr, int max_attr) + int min_attr, int max_attr, int (*validate_fun)(char *)) { struct aldap_stringset *ldap_attrs; int i; - size_t k; + size_t k, ir_len, la_len; + char validate_str[LINE_WIDTH]; memset(ir, 0, sizeof(*ir)); for (i = min_attr; i < max_attr; i++) { @@ -446,7 +471,31 @@ client_build_req(struct idm *idm, struct idm_req *ir, struct aldap_message *m, } else if (idm->idm_list & F_LIST(i)) { aldap_match_attr(m, idm->idm_attrs[i], &ldap_attrs); for (k = 0; k >= 0 && ldap_attrs && k < ldap_attrs->len; k++) { - /* XXX: Fail when attributes have illegal characters e.g. ',' */ + la_len = strlen(ldap_attrs->str[k].ostr_val); + strlcpy(validate_str, ldap_attrs->str[k].ostr_val, la_len); + + if (la_len >= LINE_WIDTH) { + log_debug("ignoring oversized ldap_attr '%s'", validate_str); + continue; + } + + if (validate_fun && !validate_fun(validate_str)) { + log_debug("ignoring invalid ldap_attr '%s'", validate_str); + continue; + } + + ir_len = strlen(ir->ir_line); + if (ir_len + la_len + 1 > LINE_WIDTH) { + if (k == 0) { + log_warn("ignoring overly long ldap_attr: '%s'", validate_str); + continue; + } else { + ir->ir_line[ir_len - 1] = '\0'; + log_debug("truncating because line would be overly long: %s,%s", ir->ir_line, validate_str); + break; + } + } + if (strlcat(ir->ir_line, ldap_attrs->str[k].ostr_val, sizeof(ir->ir_line)) >= sizeof(ir->ir_line)) @@ -492,7 +541,7 @@ client_build_req(struct idm *idm, struct idm_req *ir, struct aldap_message *m, int client_search_idm(struct env *env, struct idm *idm, struct aldap *al, char **attrs, char *filter, int min_attr, int max_attr, - enum imsg_type type) + enum imsg_type type, int (*validate_fun)(char *)) { struct idm_req ir; struct aldap_message *m; @@ -536,7 +585,8 @@ client_search_idm(struct env *env, struct idm *idm, struct aldap *al, goto fail; } - if (client_build_req(idm, &ir, m, min_attr, max_attr) == 0) + if (client_build_req(idm, &ir, m, min_attr, max_attr, + validate_fun) == 0) imsg_compose_event(env->sc_iev, type, 0, 0, -1, &ir, sizeof(ir.ir_key) + strlen(ir.ir_line) + 1); @@ -666,7 +716,8 @@ client_try_idm(struct env *env, struct idm *idm, struct ypldap_addr *addr) where = "search"; log_debug("searching password entries"); if (client_search_idm(env, idm, al, attrs, - idm->idm_filters[FILTER_USER], 0, ATTR_MAX, IMSG_PW_ENTRY) == -1) + idm->idm_filters[FILTER_USER], 0, ATTR_MAX, IMSG_PW_ENTRY, + NULL) == -1) goto bad; memset(attrs, 0, sizeof(attrs)); @@ -684,7 +735,7 @@ client_try_idm(struct env *env, struct idm *idm, struct ypldap_addr *addr) log_debug("searching group entries"); if (client_search_idm(env, idm, al, attrs, idm->idm_filters[FILTER_GROUP], ATTR_GR_MIN, ATTR_GR_MAX, - IMSG_GRP_ENTRY) == -1) + IMSG_GRP_ENTRY, valid_user) == -1) goto bad; aldap_close(al); -- Martin Hauser Software-Engineer genucenter genua GmbH Domagkstrasse 7, 85551 Kirchheim bei Muenchen tel +49 89 991950-0, fax -999, www.genua.de Geschaeftsfuehrer: Matthias Ochs, Marc Tesch Amtsgericht Muenchen HRB 98238 genua ist ein Unternehmen der Bundesdruckerei-Gruppe.