From a2ee53a0868a5bd5612c4d6529ae460b3e7dd4ab Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Thu, 28 Mar 2013 21:49:33 +0100 Subject: more stuff belongs in headers --- libtac/include/libtac.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'libtac/include') diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index 6ede892..bab3d26 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -151,6 +151,9 @@ extern void tac_add_attrib_pair(struct tac_attrib **attr, char *name, char sep, char *value); extern int tac_read_wait(int fd, int timeout, int size, int *time_left); +/* magic.c */ +u_int32_t magic(void); + #ifdef __cplusplus } #endif -- cgit v1.2.3 From 8a305d6b868e418e5849375e1904e79438fd0561 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Thu, 28 Mar 2013 21:54:15 +0100 Subject: just them prototypes --- libtac/include/libtac.h | 53 +++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 26 deletions(-) (limited to 'libtac/include') diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index bab3d26..36dc9b0 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -124,32 +124,33 @@ extern int tac_readtimeout_enable; /* connect.c */ extern int tac_timeout; -extern int tac_connect(struct addrinfo **server, char **key, int servers); -extern int tac_connect_single(struct addrinfo *server, char *key); -extern char *tac_ntop(const struct sockaddr *sa, size_t ai_addrlen); - -extern int tac_authen_send(int fd, const char *user, char *pass, char *tty, - char *r_addr); -extern int tac_authen_read(int fd); -extern int tac_cont_send(int fd, char *pass); -extern HDR *_tac_req_header(u_char type, int cont_session); -extern void _tac_crypt(u_char *buf, HDR *th, int length); -extern u_char *_tac_md5_pad(int len, HDR *hdr); -extern void tac_add_attrib(struct tac_attrib **attr, char *name, char *value); -extern void tac_free_attrib(struct tac_attrib **attr); -extern char *tac_acct_flag2str(int flag); -extern int tac_acct_send(int fd, int type, const char *user, char *tty, char *r_addr, - struct tac_attrib *attr); -extern int tac_acct_read(int fd, struct areply *arep); -extern void *xcalloc(size_t nmemb, size_t size); -extern void *xrealloc(void *ptr, size_t size); -extern char *_tac_check_header(HDR *th, int type); -extern int tac_author_send(int fd, const char *user, char *tty, char *r_addr, - struct tac_attrib *attr); -extern int tac_author_read(int fd, struct areply *arep); -extern void tac_add_attrib_pair(struct tac_attrib **attr, char *name, char sep, - char *value); -extern int tac_read_wait(int fd, int timeout, int size, int *time_left); + +int tac_connect(struct addrinfo **, char **, int); +int tac_connect_single(struct addrinfo *, char *); +char *tac_ntop(const struct sockaddr *, size_t); + +int tac_authen_send(int, const char *, char *, char *, + char *); +int tac_authen_read(int); +int tac_cont_send(int, char *); +HDR *_tac_req_header(u_char, int); +void _tac_crypt(u_char *, HDR *, int); +u_char *_tac_md5_pad(int, HDR *); +void tac_add_attrib(struct tac_attrib **, char *, char *); +void tac_free_attrib(struct tac_attrib **); +char *tac_acct_flag2str(int); +int tac_acct_send(int, int, const char *, char *, char *, + struct tac_attrib *); +int tac_acct_read(int, struct areply *); +void *xcalloc(size_t, size_t); +void *xrealloc(void *, size_t); +char *_tac_check_header(HDR *, int); +int tac_author_send(int, const char *, char *, char *, + struct tac_attrib *); +int tac_author_read(int, struct areply *); +void tac_add_attrib_pair(struct tac_attrib **, char *, char, + char *); +int tac_read_wait(int, int, int, int *); /* magic.c */ u_int32_t magic(void); -- cgit v1.2.3 From 1119da92c3c47a2cb2a48d494f98ff10a6c32619 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Thu, 28 Mar 2013 23:10:43 +0100 Subject: server is a struct { address, key } --- libtac/include/libtac.h | 4 ++-- libtac/lib/header.c | 2 +- pam_tacplus.c | 57 ++++++++++++++++++++++++++++++++----------------- support.c | 46 +++++++++++++++++++++------------------ support.h | 9 ++++++-- 5 files changed, 72 insertions(+), 46 deletions(-) (limited to 'libtac/include') diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index 36dc9b0..bcc5880 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -113,7 +113,7 @@ extern int tac_ver_patch; /* header.c */ extern int session_id; extern int tac_encryption; -extern char *tac_secret; +extern const char *tac_secret; extern char *tac_login; extern int tac_priv_lvl; extern int tac_authen_method; @@ -126,7 +126,7 @@ extern int tac_readtimeout_enable; extern int tac_timeout; int tac_connect(struct addrinfo **, char **, int); -int tac_connect_single(struct addrinfo *, char *); +int tac_connect_single(struct addrinfo *, const char *); char *tac_ntop(const struct sockaddr *, size_t); int tac_authen_send(int, const char *, char *, char *, diff --git a/libtac/lib/header.c b/libtac/lib/header.c index f361225..dd04c92 100644 --- a/libtac/lib/header.c +++ b/libtac/lib/header.c @@ -33,7 +33,7 @@ int session_id; int tac_encryption = 0; /* Pointer to TACACS+ shared secret string. */ -char *tac_secret = NULL; +const char *tac_secret = NULL; /* Pointer to TACACS+ shared login string. */ char *tac_login = NULL; /* default is PAP */ diff --git a/pam_tacplus.c b/pam_tacplus.c index 42d5f54..ea2478c 100644 --- a/pam_tacplus.c +++ b/pam_tacplus.c @@ -42,8 +42,8 @@ #endif /* address of server discovered by pam_sm_authenticate */ -static struct addrinfo *active_server = NULL; -static char *active_key = NULL; +static tacplus_server_t *active_server = NULL; + /* accounting task identifier */ static short int task_id = 0; @@ -169,7 +169,7 @@ int _pam_account(pam_handle_t *pamh, int argc, const char **argv, while ((status == PAM_SESSION_ERR) && (srv_i < tac_srv_no)) { int tac_fd; - tac_fd = tac_connect_single(tac_srv[srv_i], tac_srv_key[srv_i]); + tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key); if(tac_fd < 0) { _pam_log(LOG_WARNING, "%s: error sending %s (fd)", __FUNCTION__, typemsg); @@ -204,7 +204,7 @@ int _pam_account(pam_handle_t *pamh, int argc, const char **argv, for(srv_i = 0; srv_i < tac_srv_no; srv_i++) { int tac_fd; - tac_fd = tac_connect_single(tac_srv[srv_i], tac_srv_key[srv_i]); + tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key); if(tac_fd < 0) { _pam_log(LOG_WARNING, "%s: error sending %s (fd)", __FUNCTION__, typemsg); @@ -260,6 +260,7 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags, int status = PAM_AUTH_ERR; user = pass = tty = r_addr = NULL; + active_server = NULL; ctrl = _pam_parse (argc, argv); @@ -305,7 +306,7 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags, if (ctrl & PAM_TAC_DEBUG) syslog (LOG_DEBUG, "%s: trying srv %d", __FUNCTION__, srv_i ); - tac_fd = tac_connect_single(tac_srv[srv_i], tac_srv_key[srv_i]); + tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key); if (tac_fd < 0) { _pam_log (LOG_ERR, "connection failed srv %d: %m", srv_i); if (srv_i == tac_srv_no-1) { @@ -335,9 +336,12 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags, /* OK, we got authenticated; save the server that accepted us for pam_sm_acct_mgmt and exit the loop */ status = PAM_SUCCESS; - active_server = tac_srv[srv_i]; - active_key = tac_srv_key[srv_i]; + active_server = &tac_srv[srv_i]; close(tac_fd); + + if (ctrl & PAM_TAC_DEBUG) + syslog (LOG_DEBUG, "%s: active srv %d", __FUNCTION__, srv_i ); + break; } } @@ -348,9 +352,12 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags, /* OK, we got authenticated; save the server that accepted us for pam_sm_acct_mgmt and exit the loop */ status = PAM_SUCCESS; - active_server = tac_srv[srv_i]; - active_key = tac_srv_key[srv_i]; + active_server = &tac_srv[srv_i]; close(tac_fd); + + if (ctrl & PAM_TAC_DEBUG) + syslog (LOG_DEBUG, "%s: active srv %d", __FUNCTION__, srv_i ); + break; } } @@ -417,7 +424,7 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, if (ctrl & PAM_TAC_DEBUG) syslog(LOG_DEBUG, "%s: username obtained [%s]", __FUNCTION__, user); - + tty = _pam_get_terminal(pamh); if(!strncmp(tty, "/dev/", 5)) tty += 5; @@ -432,21 +439,21 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, by TACACS+; we cannot solely authorize user if it hasn't been authenticated or has been authenticated by method other than TACACS+ */ - if(!active_server) { + if(active_server == NULL) { _pam_log (LOG_ERR, "user not authenticated by TACACS+"); return PAM_AUTH_ERR; } if (ctrl & PAM_TAC_DEBUG) syslog (LOG_DEBUG, "%s: active server is [%s]", __FUNCTION__, - tac_ntop(active_server->ai_addr, active_server->ai_addrlen)); + tac_ntop(active_server->addr->ai_addr, active_server->addr->ai_addrlen)); /* checks for specific data required by TACACS+, which should be supplied in command line */ - if(tac_service == NULL || *tac_service == '\0') { + if(tac_service == NULL || !*tac_service) { _pam_log (LOG_ERR, "TACACS+ service type not configured"); return PAM_AUTH_ERR; } - if(tac_protocol == NULL || *tac_protocol == '\0') { + if(tac_protocol == NULL || !*tac_protocol) { _pam_log (LOG_ERR, "TACACS+ protocol type not configured"); return PAM_AUTH_ERR; } @@ -454,10 +461,12 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, tac_add_attrib(&attr, "service", tac_service); tac_add_attrib(&attr, "protocol", tac_protocol); - tac_fd = tac_connect_single(active_server, active_key); + tac_fd = tac_connect_single(active_server->addr, active_server->key); if(tac_fd < 0) { _pam_log (LOG_ERR, "TACACS+ server unavailable"); - if(arep.msg != NULL) free (arep.msg); + if(arep.msg != NULL) + free (arep.msg); + close(tac_fd); return PAM_AUTH_ERR; } @@ -468,7 +477,9 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, if(retval < 0) { _pam_log (LOG_ERR, "error getting authorization"); - if(arep.msg != NULL) free (arep.msg); + if(arep.msg != NULL) + free (arep.msg); + close(tac_fd); return PAM_AUTH_ERR; } @@ -482,7 +493,9 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, arep.status != AUTHOR_STATUS_PASS_REPL) { _pam_log (LOG_ERR, "TACACS+ authorisation failed for [%s]", user); - if(arep.msg != NULL) free (arep.msg); + if(arep.msg != NULL) + free (arep.msg); + close(tac_fd); return PAM_PERM_DENIED; } @@ -528,8 +541,12 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, } /* free returned attributes */ - if(arep.attr != NULL) tac_free_attrib(&arep.attr); - if(arep.msg != NULL) free (arep.msg); + if(arep.attr != NULL) + tac_free_attrib(&arep.attr); + + if(arep.msg != NULL) + free (arep.msg); + close(tac_fd); return status; diff --git a/support.c b/support.c index df09aba..101a8e0 100644 --- a/support.c +++ b/support.c @@ -27,10 +27,9 @@ #include "support.h" #include "pam_tacplus.h" -struct addrinfo *tac_srv[TAC_PLUS_MAXSERVERS]; +tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS]; int tac_srv_no = 0; -char *tac_srv_key[TAC_PLUS_MAXSERVERS]; -int tac_srv_key_no = 0; + char *tac_service = NULL; char *tac_protocol = NULL; char *tac_prompt = NULL; @@ -186,9 +185,11 @@ int tacacs_get_password (pam_handle_t * pamh, int flags int _pam_parse (int argc, const char **argv) { int ctrl = 0; + const char *current_secret = NULL; /* otherwise the list will grow with each call */ - tac_srv_no = tac_srv_key_no = 0; + memset(tac_srv, 0, sizeof(tacplus_server_t) * TAC_PLUS_MAXSERVERS); + tac_srv_no = 0; for (ctrl = 0; argc-- > 0; ++argv) { if (!strcmp (*argv, "debug")) { /* all */ @@ -238,7 +239,8 @@ int _pam_parse (int argc, const char **argv) { } if ((rv = getaddrinfo(server_buf, (port == NULL) ? "49" : port, &hints, &servers)) == 0) { for(server = servers; server != NULL && tac_srv_no < TAC_PLUS_MAXSERVERS; server = server->ai_next) { - tac_srv[tac_srv_no] = server; + tac_srv[tac_srv_no].addr = server; + tac_srv[tac_srv_no].key = current_secret; tac_srv_no++; } } else { @@ -251,13 +253,16 @@ int _pam_parse (int argc, const char **argv) { TAC_PLUS_MAXSERVERS); } } else if (!strncmp (*argv, "secret=", 7)) { - if(tac_srv_key_no < TAC_PLUS_MAXSERVERS) { - tac_srv_key[tac_srv_key_no] = (char *) _xcalloc (strlen (*argv + 7) + 1); - strcpy (tac_srv_key[tac_srv_key_no], *argv + 7); - tac_srv_key_no++; - } else { - _pam_log(LOG_ERR, "maximum number of secrets (%d) exceeded, skipping", - TAC_PLUS_MAXSERVERS); + int i; + + current_secret = *argv + 7; /* points right into argv (which is const) */ + + /* if 'secret=' was given after a 'server=' parameter, fill in the current secret */ + for(i = tac_srv_no-1; i >= 0; i--) { + if (tac_srv[i].key != NULL) + break; + + tac_srv[i].key = current_secret; } } else if (!strncmp (*argv, "timeout=", 8)) { tac_timeout = atoi(*argv + 8); @@ -269,15 +274,14 @@ int _pam_parse (int argc, const char **argv) { } } - if (tac_srv_key_no == 0) { - /* FIXME this should really be NULL - but watch out with breaking other code - */ - tac_srv_key[0] = ""; - tac_srv_key_no++; - } - for (;tac_srv_key_no < tac_srv_no;tac_srv_key_no++) { - tac_srv_key[tac_srv_key_no] = tac_srv_key[0]; + if (ctrl & PAM_TAC_DEBUG) { + int n; + + _pam_log(LOG_DEBUG, "%d servers defined", tac_srv_no); + + for(n = 0; n < tac_srv_no; n++) { + _pam_log(LOG_DEBUG, "server[%d] { addr=%s, key='%s' }", n, tac_ntop(tac_srv[n].addr->ai_addr, 0), tac_srv[n].key); + } } return ctrl; diff --git a/support.h b/support.h index 9662961..260dea6 100644 --- a/support.h +++ b/support.h @@ -26,9 +26,14 @@ #include -extern struct addrinfo *tac_srv[TAC_PLUS_MAXSERVERS]; -extern char *tac_srv_key[TAC_PLUS_MAXSERVERS]; +typedef struct { + struct addrinfo *addr; + const char *key; +} tacplus_server_t; + +extern tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS]; extern int tac_srv_no; + extern char *tac_service; extern char *tac_protocol; -- cgit v1.2.3 From ca77c0cfd6f62e0ac7780b5161bb6c4c49065d9b Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Thu, 28 Mar 2013 23:21:43 +0100 Subject: fixes various memory leaks, really --- libtac/include/libtac.h | 2 +- libtac/lib/connect.c | 36 ++++++++++++++++++------------------ pam_tacplus.c | 2 +- support.c | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-) (limited to 'libtac/include') diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index bcc5880..8a7381d 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -127,7 +127,7 @@ extern int tac_timeout; int tac_connect(struct addrinfo **, char **, int); int tac_connect_single(struct addrinfo *, const char *); -char *tac_ntop(const struct sockaddr *, size_t); +char *tac_ntop(const struct sockaddr *); int tac_authen_send(int, const char *, char *, char *, char *); diff --git a/libtac/lib/connect.c b/libtac/lib/connect.c index 1ec4c5f..1226797 100644 --- a/libtac/lib/connect.c +++ b/libtac/lib/connect.c @@ -75,7 +75,7 @@ int tac_connect_single(struct addrinfo *server, const char *key) { struct timeval tv; socklen_t len; struct sockaddr_storage addr; - char *ip = NULL; + char *ip; if(server == NULL) { TACSYSLOG((LOG_ERR, "%s: no TACACS+ server defined", __FUNCTION__)) @@ -83,8 +83,7 @@ int tac_connect_single(struct addrinfo *server, const char *key) { } /* format server address into a string for use in messages */ - /* FIXME this leaks memory, ip is not free()d */ - ip = tac_ntop(server->ai_addr, 0); + ip = tac_ntop(server->ai_addr); if((fd=socket(server->ai_family, server->ai_socktype, server->ai_protocol)) < 0) { TACSYSLOG((LOG_ERR,"%s: socket creation error", __FUNCTION__)) @@ -160,8 +159,6 @@ int tac_connect_single(struct addrinfo *server, const char *key) { tac_secret = key; } - free(ip); - /* if valid fd, but error experienced after open, close fd */ if ( fd >= 0 && retval < 0 ) { close(fd); @@ -175,29 +172,32 @@ int tac_connect_single(struct addrinfo *server, const char *key) { /* return value: * ptr to char* with format IP address - * must be freed by caller + * warning: returns a static buffer + * (which some ppl don't like, but it's robust and at last no more memory leaks) */ -char *tac_ntop(const struct sockaddr *sa, size_t unused) { - char portstr[7]; - char *str = (char *) xcalloc(1, INET6_ADDRSTRLEN+sizeof(portstr)); +char *tac_ntop(const struct sockaddr *sa) { + static char server_address[INET6_ADDRSTRLEN+16]; switch(sa->sa_family) { case AF_INET: inet_ntop(AF_INET, &(((struct sockaddr_in *)sa)->sin_addr), - str, INET_ADDRSTRLEN); - snprintf(portstr, sizeof(portstr), ":%hu", - htons(((struct sockaddr_in *)sa)->sin_port)); - strcat(str, portstr); + server_address, INET_ADDRSTRLEN); + + snprintf(server_address + strlen(server_address), 14, ":%hu", + htons(((struct sockaddr_in *)sa)->sin_port)); break; + case AF_INET6: inet_ntop(AF_INET6, &(((struct sockaddr_in6 *)sa)->sin6_addr), - str, INET6_ADDRSTRLEN); - snprintf(portstr, sizeof(portstr), ":%hu", + server_address, INET6_ADDRSTRLEN); + + snprintf(server_address + strlen(server_address), 14, ":%hu", htons(((struct sockaddr_in6 *)sa)->sin6_port)); - strcat(str, portstr); break; + default: - strncpy(str, "Unknown AF", INET6_ADDRSTRLEN); + strcpy(server_address, "Unknown AF"); } - return str; + return server_address; } /* tac_ntop */ + diff --git a/pam_tacplus.c b/pam_tacplus.c index ea2478c..be0c826 100644 --- a/pam_tacplus.c +++ b/pam_tacplus.c @@ -445,7 +445,7 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags, } if (ctrl & PAM_TAC_DEBUG) syslog (LOG_DEBUG, "%s: active server is [%s]", __FUNCTION__, - tac_ntop(active_server->addr->ai_addr, active_server->addr->ai_addrlen)); + tac_ntop(active_server->addr->ai_addr)); /* checks for specific data required by TACACS+, which should be supplied in command line */ diff --git a/support.c b/support.c index 101a8e0..3181036 100644 --- a/support.c +++ b/support.c @@ -280,7 +280,7 @@ int _pam_parse (int argc, const char **argv) { _pam_log(LOG_DEBUG, "%d servers defined", tac_srv_no); for(n = 0; n < tac_srv_no; n++) { - _pam_log(LOG_DEBUG, "server[%d] { addr=%s, key='%s' }", n, tac_ntop(tac_srv[n].addr->ai_addr, 0), tac_srv[n].key); + _pam_log(LOG_DEBUG, "server[%d] { addr=%s, key='%s' }", n, tac_ntop(tac_srv[n].addr->ai_addr), tac_srv[n].key); } } -- cgit v1.2.3 From f663d6e0e8b5aa16009610b429499671bf8f4cc9 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Fri, 29 Mar 2013 00:28:10 +0100 Subject: removed double xcalloc() function; do not leak memory for these small buffers; added safe xstrcpy() --- libtac/include/libtac.h | 4 +-- libtac/lib/acct_s.c | 2 +- libtac/lib/authen_s.c | 10 +++---- libtac/lib/header.c | 5 ++-- libtac/lib/xalloc.c | 4 +-- pam_tacplus.c | 11 +++++--- support.c | 70 ++++++++++++++++++++++++++++++++----------------- support.h | 5 ++-- 8 files changed, 70 insertions(+), 41 deletions(-) (limited to 'libtac/include') diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index 8a7381d..7b7518f 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -79,7 +79,7 @@ struct areply { }; #ifndef TAC_PLUS_MAXSERVERS -#define TAC_PLUS_MAXSERVERS 4 +#define TAC_PLUS_MAXSERVERS 8 #endif #ifndef TAC_PLUS_PORT @@ -114,7 +114,7 @@ extern int tac_ver_patch; extern int session_id; extern int tac_encryption; extern const char *tac_secret; -extern char *tac_login; +extern char tac_login[64]; extern int tac_priv_lvl; extern int tac_authen_method; extern int tac_authen_service; diff --git a/libtac/lib/acct_s.c b/libtac/lib/acct_s.c index 200dd62..929378a 100644 --- a/libtac/lib/acct_s.c +++ b/libtac/lib/acct_s.c @@ -78,7 +78,7 @@ int tac_acct_send(int fd, int type, const char *user, char *tty, tb.flags=(u_char) type; tb.authen_method=tac_authen_method; tb.priv_lvl=tac_priv_lvl; - if (tac_login == NULL) { + if (tac_login == NULL || !*tac_login) { /* default to PAP */ tb.authen_type = TAC_PLUS_AUTHEN_TYPE_PAP; } else { diff --git a/libtac/lib/authen_s.c b/libtac/lib/authen_s.c index 8cb7cb9..87dcb74 100644 --- a/libtac/lib/authen_s.c +++ b/libtac/lib/authen_s.c @@ -51,7 +51,7 @@ int tac_authen_send(int fd, const char *user, char *pass, char *tty, th=_tac_req_header(TAC_PLUS_AUTHEN, 0); /* set some header options */ - if ((tac_login != NULL) && (strcmp(tac_login,"login") == 0)) { + if (tac_login != NULL && !strcmp(tac_login,"login")) { th->version = TAC_PLUS_VER_0; } else { th->version = TAC_PLUS_VER_1; @@ -62,7 +62,7 @@ int tac_authen_send(int fd, const char *user, char *pass, char *tty, __FUNCTION__, user, tty, r_addr, \ (tac_encryption) ? "yes" : "no")) - if ((tac_login != NULL) && (strcmp(tac_login,"chap") == 0)) { + if (tac_login != NULL && !strcmp(tac_login,"chap")) { chal_len = strlen(chal); mdp_len = sizeof(u_char) + strlen(pass) + chal_len; mdp = (u_char *) xcalloc(1, mdp_len); @@ -90,13 +90,13 @@ int tac_authen_send(int fd, const char *user, char *pass, char *tty, /* fill the body of message */ tb.action = TAC_PLUS_AUTHEN_LOGIN; tb.priv_lvl = tac_priv_lvl; - if (tac_login == NULL) { + if (tac_login == NULL || !*tac_login) { /* default to PAP */ tb.authen_type = TAC_PLUS_AUTHEN_TYPE_PAP; } else { - if (strcmp(tac_login,"chap") == 0) { + if (!strcmp(tac_login,"chap")) { tb.authen_type = TAC_PLUS_AUTHEN_TYPE_CHAP; - } else if (strcmp(tac_login,"login") == 0) { + } else if (!strcmp(tac_login,"login")) { tb.authen_type = TAC_PLUS_AUTHEN_TYPE_ASCII; } else { tb.authen_type = TAC_PLUS_AUTHEN_TYPE_PAP; diff --git a/libtac/lib/header.c b/libtac/lib/header.c index dd04c92..73c4f13 100644 --- a/libtac/lib/header.c +++ b/libtac/lib/header.c @@ -33,10 +33,11 @@ int session_id; int tac_encryption = 0; /* Pointer to TACACS+ shared secret string. */ +/* note: tac_secret will point to tacplus_server[i].key */ const char *tac_secret = NULL; -/* Pointer to TACACS+ shared login string. */ -char *tac_login = NULL; /* default is PAP */ +/* TACACS+ shared login string. */ +char tac_login[64]; /* default is PAP */ /* priv_lvl */ int tac_priv_lvl = TAC_PLUS_PRIV_LVL_MIN; diff --git a/libtac/lib/xalloc.c b/libtac/lib/xalloc.c index ce34c44..d749b52 100644 --- a/libtac/lib/xalloc.c +++ b/libtac/lib/xalloc.c @@ -23,7 +23,7 @@ #include "xalloc.h" void *xcalloc(size_t nmemb, size_t size) { - register void *val = calloc(nmemb, size); + void *val = calloc(nmemb, size); if(val == 0) { TACSYSLOG((LOG_ERR, "%s: calloc(%u,%u) failed", __FUNCTION__,\ (unsigned) nmemb, (unsigned) size)) @@ -33,7 +33,7 @@ void *xcalloc(size_t nmemb, size_t size) { } void *xrealloc(void *ptr, size_t size) { - register void *val = realloc(ptr, size); + void *val = realloc(ptr, size); if(val == 0) { TACSYSLOG((LOG_ERR, "%s: realloc(%u) failed", __FUNCTION__, (unsigned) size)) exit(1); diff --git a/pam_tacplus.c b/pam_tacplus.c index be0c826..635c11d 100644 --- a/pam_tacplus.c +++ b/pam_tacplus.c @@ -56,7 +56,7 @@ int _pam_send_account(int tac_fd, int type, const char *user, char *tty, struct tac_attrib *attr; int retval; - attr=(struct tac_attrib *)_xcalloc(sizeof(struct tac_attrib)); + attr=(struct tac_attrib *)xcalloc(1, sizeof(struct tac_attrib)); sprintf(buf, "%lu", (unsigned long)time(NULL)); @@ -93,12 +93,17 @@ int _pam_send_account(int tac_fd, int type, const char *user, char *tty, __FUNCTION__, tac_acct_flag2str(type), task_id); - if(re.msg != NULL) free(re.msg); + + if(re.msg != NULL) + free(re.msg); + close(tac_fd); return -1; } - if(re.msg != NULL) free(re.msg); + if(re.msg != NULL) + free(re.msg); + close(tac_fd); return 0; } diff --git a/support.c b/support.c index 3181036..7ee2dad 100644 --- a/support.c +++ b/support.c @@ -27,28 +27,40 @@ #include "support.h" #include "pam_tacplus.h" +#include +#include + tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS]; int tac_srv_no = 0; -char *tac_service = NULL; -char *tac_protocol = NULL; -char *tac_prompt = NULL; +char tac_service[64]; +char tac_protocol[64]; +char tac_prompt[64]; /* - FIXME using xcalloc() leaks memory for long-running programs that authenticate multiple times + safe string copy, like strlcpy() really */ -#ifndef xcalloc -void *_xcalloc (size_t size) { - register void *val = calloc (1, size); - if (val == 0) { - syslog (LOG_ERR, "xcalloc: calloc(1,%u) failed", (unsigned) size); - abort(); - } - return val; +size_t xstrcpy(char *dst, const char *src, size_t dst_size) { + if (dst == NULL) + _pam_log(LOG_ERR, "xstrcpy(): dst == NULL"); + + if (src == NULL) + _pam_log(LOG_ERR, "xstrcpy(): src == NULL"); + + if (!dst_size) + return 0; + + size_t s_len = strlen(src); + + size_t n = s_len; + if (n >= dst_size) + n = dst_size - 1; + + strncpy(dst, src, n); + dst[n] = 0; + + return n; } -#else -#define _xcalloc xcalloc -#endif void _pam_log(int err, const char *format,...) { char msg[256]; @@ -191,6 +203,11 @@ int _pam_parse (int argc, const char **argv) { memset(tac_srv, 0, sizeof(tacplus_server_t) * TAC_PLUS_MAXSERVERS); tac_srv_no = 0; + tac_service[0] = 0; + tac_protocol[0] = 0; + tac_prompt[0] = 0; + tac_login[0] = 0; + for (ctrl = 0; argc-- > 0; ++argv) { if (!strcmp (*argv, "debug")) { /* all */ ctrl |= PAM_TAC_DEBUG; @@ -199,14 +216,11 @@ int _pam_parse (int argc, const char **argv) { } else if (!strcmp (*argv, "try_first_pass")) { ctrl |= PAM_TAC_TRY_FIRST_PASS; } else if (!strncmp (*argv, "service=", 8)) { /* author & acct */ - tac_service = (char *) _xcalloc (strlen (*argv + 8) + 1); - strcpy (tac_service, *argv + 8); + xstrcpy (tac_service, *argv + 8, sizeof(tac_service)); } else if (!strncmp (*argv, "protocol=", 9)) { /* author & acct */ - tac_protocol = (char *) _xcalloc (strlen (*argv + 9) + 1); - strcpy (tac_protocol, *argv + 9); + xstrcpy (tac_protocol, *argv + 9, sizeof(tac_protocol)); } else if (!strncmp (*argv, "prompt=", 7)) { /* authentication */ - tac_prompt = (char *) _xcalloc (strlen (*argv + 7) + 1); - strcpy (tac_prompt, *argv + 7); + xstrcpy (tac_prompt, *argv + 7, sizeof(tac_prompt)); /* Replace _ with space */ int chr; for (chr = 0; chr < strlen(tac_prompt); chr++) { @@ -214,6 +228,8 @@ int _pam_parse (int argc, const char **argv) { tac_prompt[chr] = ' '; } } + } else if (!strncmp (*argv, "login=", 6)) { + xstrcpy (tac_login, *argv + 6, sizeof(tac_login)); } else if (!strcmp (*argv, "acct_all")) { ctrl |= PAM_TAC_ACCT; } else if (!strncmp (*argv, "server=", 7)) { /* authen & acct */ @@ -265,10 +281,11 @@ int _pam_parse (int argc, const char **argv) { tac_srv[i].key = current_secret; } } else if (!strncmp (*argv, "timeout=", 8)) { + /* FIXME atoi() doesn't handle invalid numeric strings well */ tac_timeout = atoi(*argv + 8); - } else if (!strncmp (*argv, "login=", 6)) { - tac_login = (char *) _xcalloc (strlen (*argv + 6) + 1); - strcpy (tac_login, *argv + 6); + + if (tac_timeout < 0) + tac_timeout = 0; } else { _pam_log (LOG_WARNING, "unrecognized option: %s", *argv); } @@ -282,6 +299,11 @@ int _pam_parse (int argc, const char **argv) { for(n = 0; n < tac_srv_no; n++) { _pam_log(LOG_DEBUG, "server[%d] { addr=%s, key='%s' }", n, tac_ntop(tac_srv[n].addr->ai_addr), tac_srv[n].key); } + + _pam_log(LOG_DEBUG, "tac_service='%s'", tac_service); + _pam_log(LOG_DEBUG, "tac_protocol='%s'", tac_protocol); + _pam_log(LOG_DEBUG, "tac_prompt='%s'", tac_prompt); + _pam_log(LOG_DEBUG, "tac_login='%s'", tac_login); } return ctrl; diff --git a/support.h b/support.h index 260dea6..9cbd040 100644 --- a/support.h +++ b/support.h @@ -34,8 +34,9 @@ typedef struct { extern tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS]; extern int tac_srv_no; -extern char *tac_service; -extern char *tac_protocol; +extern char tac_service[64]; +extern char tac_protocol[64]; +extern char tac_prompt[64]; int _pam_parse (int, const char **); unsigned long _resolve_name (char *); -- cgit v1.2.3 From d3c323cc390659dfd3da50a015e18285b5b5b6ff Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Fri, 29 Mar 2013 10:03:40 +0100 Subject: xstrcpy() belongs in libtac --- libtac/include/libtac.h | 1 + libtac/lib/xalloc.c | 25 ++++++++++++++++++++++++- libtac/lib/xalloc.h | 2 +- support.c | 21 --------------------- 4 files changed, 26 insertions(+), 23 deletions(-) (limited to 'libtac/include') diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index 7b7518f..aad4cbf 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -144,6 +144,7 @@ int tac_acct_send(int, int, const char *, char *, char *, int tac_acct_read(int, struct areply *); void *xcalloc(size_t, size_t); void *xrealloc(void *, size_t); +char *xstrcpy(char *, const char *, size_t); char *_tac_check_header(HDR *, int); int tac_author_send(int, const char *, char *, char *, struct tac_attrib *); diff --git a/libtac/lib/xalloc.c b/libtac/lib/xalloc.c index d749b52..8fcce26 100644 --- a/libtac/lib/xalloc.c +++ b/libtac/lib/xalloc.c @@ -41,7 +41,7 @@ void *xrealloc(void *ptr, size_t size) { return val; } -char *xstrdup(char *s) { +char *xstrdup(const char *s) { char *p; if (s == NULL) return NULL; @@ -51,3 +51,26 @@ char *xstrdup(char *s) { } return p; } + + +/* + safe string copy that aborts when destination buffer is too small +*/ +char *xstrcpy(char *dst, const char *src, size_t dst_size) { + if (dst == NULL) { + TACSYSLOG((LOG_ERR, "xstrcpy(): dst == NULL")); + } + if (src == NULL) { + TACSYSLOG((LOG_ERR, "xstrcpy(): src == NULL")); + } + if (!dst_size) + return NULL; + + if (strlen(src) >= dst_size) { + TACSYSLOG((LOG_ERR, "xstrcpy(): argument too long, aborting")); + abort(); + } + + return strcpy(dst, src); +} + diff --git a/libtac/lib/xalloc.h b/libtac/lib/xalloc.h index 70bc666..196cc9f 100644 --- a/libtac/lib/xalloc.h +++ b/libtac/lib/xalloc.h @@ -27,7 +27,7 @@ __BEGIN_DECLS extern void *xcalloc(size_t nmemb, size_t size); extern void *xrealloc(void *ptr, size_t size); -extern char *xstrdup(char *s); +extern char *xstrdup(const char *s); __END_DECLS #endif diff --git a/support.c b/support.c index fe084cf..3210a37 100644 --- a/support.c +++ b/support.c @@ -49,27 +49,6 @@ void _pam_log(int err, const char *format,...) { closelog(); } -/* - safe string copy that aborts when destination buffer is too small -*/ -char *xstrcpy(char *dst, const char *src, size_t dst_size) { - if (dst == NULL) - _pam_log(LOG_ERR, "xstrcpy(): dst == NULL"); - - if (src == NULL) - _pam_log(LOG_ERR, "xstrcpy(): src == NULL"); - - if (!dst_size) - return NULL; - - if (strlen(src) >= dst_size) { - _pam_log(LOG_ERR, "xstrcpy(): argument too long, aborting"); - abort(); - } - - return strcpy(dst, src); -} - char *_pam_get_user(pam_handle_t *pamh) { int retval; char *user; -- cgit v1.2.3