From eb7ae29dd1157f01ed0d57692ec8cd40e9d23554 Mon Sep 17 00:00:00 2001 From: Alexandra Ellwood Date: Tue, 10 Aug 1999 20:16:15 +0000 Subject: (krb5_change_password): Reorganized code so that krb5_change_password actually frees everything it allocated on error. Also fixed some memory leaks which happened even without an error occurring. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@11646 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/os/changepw.c | 532 ++++++++++++++++++++++++--------------------- 1 file changed, 284 insertions(+), 248 deletions(-) (limited to 'src') diff --git a/src/lib/krb5/os/changepw.c b/src/lib/krb5/os/changepw.c index 779fc89..485149e 100644 --- a/src/lib/krb5/os/changepw.c +++ b/src/lib/krb5/os/changepw.c @@ -57,8 +57,8 @@ krb5_change_password(context, creds, newpw, result_code, krb5_address local_kaddr, remote_kaddr; const char *realm_kdc_names[4]; int default_port; - char **hostlist, *host, *port, *cp, *code_string; - krb5_error_code code; + char **hostlist, *host, *tmphost, *port, *cp, *code_string; + krb5_error_code code = 0; int i, j, out, count, addrlen; struct sockaddr *addr_p, local_addr, remote_addr, tmp_addr; struct sockaddr_in *sin_p; @@ -68,17 +68,30 @@ krb5_change_password(context, creds, newpw, result_code, u_short udpport = htons(KRB5_DEFAULT_PORT); #endif int cc, local_result_code, tmp_len; - SOCKET s1, s2; + SOCKET s1 = INVALID_SOCKET, s2 = INVALID_SOCKET; + /* Initialize values so that cleanup call can safely check for NULL */ auth_context = NULL; - + addr_p = NULL; + host = NULL; + hostlist = NULL; + memset(&chpw_req, 0, sizeof(krb5_data)); + memset(&chpw_rep, 0, sizeof(krb5_data)); + memset(&ap_req, 0, sizeof(krb5_data)); + + /* initialize auth_context so that we know we have to free it */ + if ((code = krb5_auth_con_init(context, &auth_context))) + goto cleanup; + if (code = krb5_mk_req_extended(context, &auth_context, AP_OPTS_USE_SUBKEY, NULL, creds, &ap_req)) - return(code); + goto cleanup; - if ((host = malloc(krb5_princ_realm(context, creds->client)->length + 1)) - == NULL) - return ENOMEM; + if ((host = malloc(krb5_princ_realm(context, creds->client)->length + 1)) == NULL) + { + code = ENOMEM; + goto cleanup; + } strncpy(host, krb5_princ_realm(context, creds->client)->data, krb5_princ_realm(context, creds->client)->length); @@ -94,23 +107,27 @@ krb5_change_password(context, creds, newpw, result_code, code = profile_get_values(context->profile, realm_kdc_names, &hostlist); - if (code == PROF_NO_RELATION) { - realm_kdc_names[2] = "admin_server"; - - default_port = 1; - - code = profile_get_values(context->profile, realm_kdc_names, - &hostlist); - } - - krb5_xfree(host); + if (code == PROF_NO_RELATION) + { + realm_kdc_names[2] = "admin_server"; + default_port = 1; + code = profile_get_values(context->profile, realm_kdc_names, &hostlist); + } if (code == PROF_NO_SECTION) - return KRB5_REALM_UNKNOWN; - else if (code == PROF_NO_RELATION) - return KRB5_CONFIG_BADFORMAT; - else if (code) - return code; + { + code = KRB5_REALM_UNKNOWN; + goto cleanup; + } + else + if (code == PROF_NO_RELATION) + { + code = KRB5_CONFIG_BADFORMAT; + goto cleanup; + } + else + if (code) + goto cleanup; #ifdef HAVE_NETINET_IN_H /* XXX should look for "kpasswd" in /etc/services */ @@ -122,28 +139,34 @@ krb5_change_password(context, creds, newpw, result_code, count++; if (count == 0) - /* XXX */ - return(KADM_NO_HOST); + { + /* XXX */ + code = KADM_NO_HOST; + goto cleanup; + } addr_p = (struct sockaddr *) malloc(sizeof(struct sockaddr) * count); if (addr_p == NULL) - return ENOMEM; + { + code = ENOMEM; + goto cleanup; + } - host = hostlist[0]; + tmphost = hostlist[0]; out = 0; /* * Strip off excess whitespace */ - cp = strchr(host, ' '); + cp = strchr(tmphost, ' '); if (cp) - *cp = 0; - cp = strchr(host, '\t'); + *cp = 0; + cp = strchr(tmphost, '\t'); if (cp) - *cp = 0; - port = strchr(host, ':'); + *cp = 0; + port = strchr(tmphost, ':'); if (port) { - *port = 0; + *port = 0; port++; /* if the admin_server line was used, ignore the specified port */ @@ -152,40 +175,46 @@ krb5_change_password(context, creds, newpw, result_code, } hp = gethostbyname(hostlist[0]); - if (hp != 0) { - switch (hp->h_addrtype) { + if (hp != 0) + { + switch (hp->h_addrtype) + { #ifdef HAVE_NETINET_IN_H - case AF_INET: - for (j=0; hp->h_addr_list[j]; j++) { - sin_p = (struct sockaddr_in *) &addr_p[out++]; - memset ((char *)sin_p, 0, sizeof(struct sockaddr)); - sin_p->sin_family = hp->h_addrtype; - sin_p->sin_port = port ? htons(atoi(port)) : udpport; - memcpy((char *)&sin_p->sin_addr, - (char *)hp->h_addr_list[j], - sizeof(struct in_addr)); - if (out+1 >= count) { - count += 5; - addr_p = (struct sockaddr *) - realloc ((char *)addr_p, - sizeof(struct sockaddr) * count); - if (addr_p == NULL) - return ENOMEM; - } - } - break; + case AF_INET: + for (j=0; hp->h_addr_list[j]; j++) + { + sin_p = (struct sockaddr_in *) &addr_p[out++]; + memset ((char *)sin_p, 0, sizeof(struct sockaddr)); + sin_p->sin_family = hp->h_addrtype; + sin_p->sin_port = port ? htons(atoi(port)) : udpport; + memcpy((char *)&sin_p->sin_addr, + (char *)hp->h_addr_list[j], + sizeof(struct in_addr)); + if (out+1 >= count) + { + count += 5; + addr_p = (struct sockaddr *) + realloc ((char *)addr_p, sizeof(struct sockaddr) * count); + if (addr_p == NULL) + { + code = ENOMEM; + goto cleanup; + } + } + } + break; #endif - default: - break; - } - } - - profile_free_list(hostlist); - - if (out == 0) { /* Couldn't resolve any KDC names */ - free (addr_p); - return(KADM_NO_HOST); - } + default: + break; + } + } + + if (out == 0) + { + /* Couldn't resolve any KDC names */ + code = KADM_NO_HOST; + goto cleanup; + } /* this is really obscure. s1 is used for all communications. it is left unconnected in case the server is multihomed and routes @@ -203,187 +232,194 @@ krb5_change_password(context, creds, newpw, result_code, hostname resolution to get the local ip addr) will work and interoperate if the client is single-homed. */ - if ((s1 = socket(AF_INET, SOCK_DGRAM, 0)) == INVALID_SOCKET) { - free(addr_p); - return(SOCKET_ERRNO); - } - - if ((s2 = socket(AF_INET, SOCK_DGRAM, 0)) == INVALID_SOCKET) { - free(addr_p); - return(SOCKET_ERRNO); - } - - for (i=0; isin_addr.s_addr != 0) { - local_kaddr.addrtype = ADDRTYPE_INET; - local_kaddr.length = - sizeof(((struct sockaddr_in *) &local_addr)->sin_addr); - local_kaddr.contents = - (krb5_octet *) &(((struct sockaddr_in *) &local_addr)->sin_addr); - } else { - krb5_address **addrs; - - krb5_os_localaddr(context, &addrs); - local_kaddr.magic = addrs[0]->magic; - local_kaddr.addrtype = addrs[0]->addrtype; - local_kaddr.length = addrs[0]->length; - local_kaddr.contents = malloc(addrs[0]->length); - memcpy(local_kaddr.contents, addrs[0]->contents, addrs[0]->length); - - krb5_free_addresses(context, addrs); - } - - addrlen = sizeof(remote_addr); - if (getpeername(s2, &remote_addr, &addrlen) < 0) { - if ((SOCKET_ERRNO == ECONNREFUSED) || - (SOCKET_ERRNO == EHOSTUNREACH)) - continue; /* try the next addr */ - free(addr_p); - closesocket(s1); - closesocket(s2); - return(SOCKET_ERRNO); - } - - remote_kaddr.addrtype = ADDRTYPE_INET; - remote_kaddr.length = - sizeof(((struct sockaddr_in *) &remote_addr)->sin_addr); - remote_kaddr.contents = - (krb5_octet *) &(((struct sockaddr_in *) &remote_addr)->sin_addr); - - /* mk_priv requires that the local address be set. - getsockname is used for this. rd_priv requires that the - remote address be set. recvfrom is used for this. If - rd_priv is given a local address, and the message has the - recipient addr in it, this will be checked. However, there - is simply no way to know ahead of time what address the - message will be delivered *to*. Therefore, it is important - that either no recipient address is in the messages when - mk_priv is called, or that no local address is passed to - rd_priv. Both is a better idea, and I have done that. In - summary, when mk_priv is called, *only* a local address is - specified. when rd_priv is called, *only* a remote address - is specified. Are we having fun yet? */ - - if (code = krb5_auth_con_setaddrs(context, auth_context, &local_kaddr, - NULL)) { - free(addr_p); - closesocket(s1); - closesocket(s2); - return(code); - } - - if (code = krb5_mk_chpw_req(context, auth_context, &ap_req, - newpw, &chpw_req)) { - free(addr_p); - closesocket(s1); - closesocket(s2); - return(code); - } - - if ((cc = sendto(s1, chpw_req.data, chpw_req.length, 0, - (struct sockaddr *) &addr_p[i], - sizeof(addr_p[i]))) != - chpw_req.length) { - if ((cc < 0) && ((SOCKET_ERRNO == ECONNREFUSED) || - (SOCKET_ERRNO == EHOSTUNREACH))) - continue; /* try the next addr */ - free(addr_p); - closesocket(s1); - closesocket(s2); - return((cc < 0)?SOCKET_ERRNO:ECONNABORTED); - } - - krb5_xfree(chpw_req.data); - - chpw_rep.length = 1500; - chpw_rep.data = (char *) malloc(chpw_rep.length); - - /* XXX need a timeout/retry loop here */ - - /* "recv" would be good enough here... except that Windows/NT - commits the atrocity of returning -1 to indicate failure, - but leaving errno set to 0. - - "recvfrom(...,NULL,NULL)" would seem to be a good enough - alternative, and it works on NT, but it doesn't work on - SunOS 4.1.4 or Irix 5.3. Thus we must actually accept the - value and discard it. */ - tmp_len = sizeof(tmp_addr); - if ((cc = recvfrom(s1, chpw_rep.data, chpw_rep.length, 0, &tmp_addr, &tmp_len)) < 0) { - free(addr_p); - closesocket(s1); - closesocket(s2); - return(SOCKET_ERRNO); - } - - closesocket(s1); - closesocket(s2); - - chpw_rep.length = cc; - - if (code = krb5_auth_con_setaddrs(context, auth_context, NULL, - &remote_kaddr)) { - free(addr_p); - closesocket(s1); - closesocket(s2); - return(code); - } - - code = krb5_rd_chpw_rep(context, auth_context, &chpw_rep, - &local_result_code, result_string); - - free(chpw_rep.data); - free(addr_p); - - if (code) - return(code); - - if (result_code) - *result_code = local_result_code; - - if (result_code_string) { - if (code = krb5_chpw_result_code_string(context, local_result_code, - &code_string)) - return(code); - - result_code_string->length = strlen(code_string); - if ((result_code_string->data = - (char *) malloc(result_code_string->length)) == NULL) - return(ENOMEM); - strncpy(result_code_string->data, code_string, - result_code_string->length); - } - - return(0); - } - - free(addr_p); - closesocket(s1); - closesocket(s2); - return(SOCKET_ERRNO); + if ((s1 = socket(AF_INET, SOCK_DGRAM, 0)) == INVALID_SOCKET) + { + code = SOCKET_ERRNO; + goto cleanup; + } + + if ((s2 = socket(AF_INET, SOCK_DGRAM, 0)) == INVALID_SOCKET) + { + code = SOCKET_ERRNO; + goto cleanup; + } + + for (i=0; isin_addr.s_addr != 0) + { + local_kaddr.addrtype = ADDRTYPE_INET; + local_kaddr.length = sizeof(((struct sockaddr_in *) &local_addr)->sin_addr); + local_kaddr.contents = (krb5_octet *) &(((struct sockaddr_in *) &local_addr)->sin_addr); + } + else + { + krb5_address **addrs; + + krb5_os_localaddr(context, &addrs); + + local_kaddr.magic = addrs[0]->magic; + local_kaddr.addrtype = addrs[0]->addrtype; + local_kaddr.length = addrs[0]->length; + local_kaddr.contents = malloc(addrs[0]->length); + memcpy(local_kaddr.contents, addrs[0]->contents, addrs[0]->length); + + krb5_free_addresses(context, addrs); + } + + addrlen = sizeof(remote_addr); + if (getpeername(s2, &remote_addr, &addrlen) < 0) + { + if ((SOCKET_ERRNO == ECONNREFUSED) || (SOCKET_ERRNO == EHOSTUNREACH)) + continue; /* try the next addr */ + + code = SOCKET_ERRNO; + goto cleanup; + } + + remote_kaddr.addrtype = ADDRTYPE_INET; + remote_kaddr.length = sizeof(((struct sockaddr_in *) &remote_addr)->sin_addr); + remote_kaddr.contents = (krb5_octet *) &(((struct sockaddr_in *) &remote_addr)->sin_addr); + + /* mk_priv requires that the local address be set. + getsockname is used for this. rd_priv requires that the + remote address be set. recvfrom is used for this. If + rd_priv is given a local address, and the message has the + recipient addr in it, this will be checked. However, there + is simply no way to know ahead of time what address the + message will be delivered *to*. Therefore, it is important + that either no recipient address is in the messages when + mk_priv is called, or that no local address is passed to + rd_priv. Both is a better idea, and I have done that. In + summary, when mk_priv is called, *only* a local address is + specified. when rd_priv is called, *only* a remote address + is specified. Are we having fun yet? */ + + if (code = krb5_auth_con_setaddrs(context, auth_context, &local_kaddr, NULL)) + { + code = SOCKET_ERRNO; + goto cleanup; + } + + if (code = krb5_mk_chpw_req(context, auth_context, &ap_req, newpw, &chpw_req)) + { + code = SOCKET_ERRNO; + goto cleanup; + } + + if ((cc = sendto(s1, chpw_req.data, chpw_req.length, 0, + (struct sockaddr *) &addr_p[i], + sizeof(addr_p[i]))) != chpw_req.length) + { + if ((cc < 0) && ((SOCKET_ERRNO == ECONNREFUSED) || + (SOCKET_ERRNO == EHOSTUNREACH))) + continue; /* try the next addr */ + + code = (cc < 0) ? SOCKET_ERRNO : ECONNABORTED; + goto cleanup; + } + + chpw_rep.length = 1500; + chpw_rep.data = (char *) malloc(chpw_rep.length); + + /* XXX need a timeout/retry loop here */ + + /* "recv" would be good enough here... except that Windows/NT + commits the atrocity of returning -1 to indicate failure, + but leaving errno set to 0. + + "recvfrom(...,NULL,NULL)" would seem to be a good enough + alternative, and it works on NT, but it doesn't work on + SunOS 4.1.4 or Irix 5.3. Thus we must actually accept the + value and discard it. */ + tmp_len = sizeof(tmp_addr); + if ((cc = recvfrom(s1, chpw_rep.data, chpw_rep.length, 0, &tmp_addr, &tmp_len)) < 0) + { + code = SOCKET_ERRNO; + goto cleanup; + } + + closesocket(s1); + s1 = INVALID_SOCKET; + closesocket(s2); + s2 = INVALID_SOCKET; + + chpw_rep.length = cc; + + if (code = krb5_auth_con_setaddrs(context, auth_context, NULL, &remote_kaddr)) + goto cleanup; + + if(code = krb5_rd_chpw_rep(context, auth_context, &chpw_rep, + &local_result_code, result_string)) + goto cleanup; + + if (result_code) + *result_code = local_result_code; + + if (result_code_string) + { + if (code = krb5_chpw_result_code_string(context, local_result_code, + &code_string)) + goto cleanup; + + result_code_string->length = strlen(code_string); + if ((result_code_string->data = + (char *) malloc(result_code_string->length)) == NULL) + return(ENOMEM); + strncpy(result_code_string->data, code_string, result_code_string->length); + } + + code = 0; + goto cleanup; + } + + code = SOCKET_ERRNO; + +cleanup: + if(auth_context != NULL) + krb5_auth_con_free(context, auth_context); + + if(host != NULL) + krb5_xfree(host); + + if(addr_p != NULL) + krb5_xfree(addr_p); + + if(hostlist != NULL) + profile_free_list(hostlist); + + if(s1 != INVALID_SOCKET) + closesocket(s1); + + if(s2 != INVALID_SOCKET) + closesocket(s2); + + krb5_free_data_contents(context, &chpw_req); + krb5_free_data_contents(context, &chpw_rep); + krb5_free_data_contents(context, &ap_req); + + return(code); } -- cgit v1.1