Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TACACS+]: Extract tacacs support functions into library and fix memory leak issue. #8659

Merged
merged 13 commits into from
Oct 14, 2021
Merged

[TACACS+]: Extract tacacs support functions into library and fix memory leak issue. #8659

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
00e9867
[TACACS+]: Add TACACS support library and fix memory leak issue.
liuh-80 Aug 31, 2021
2f636c5
Improve PR.
liuh-80 Sep 10, 2021
2648dcd
Merge remote-tracking branch 'origin/master' into dev/liuh/tac_suppor…
liuh-80 Sep 17, 2021
bb9fec3
Add setting flag for authorization and accounting.
liuh-80 Sep 29, 2021
7022bb3
Fix source address code issue..
liuh-80 Oct 1, 2021
19f27fa
Fix trailing whitespace.
liuh-80 Oct 1, 2021
8e3d88f
Fix apply patch issue.
liuh-80 Oct 1, 2021
b30ace5
Fix secret key issue.
liuh-80 Oct 2, 2021
761c1cf
Improve patch file.
liuh-80 Oct 3, 2021
14ea9aa
Fix PR comments.
liuh-80 Oct 8, 2021
140e4d4
Improve patches.
liuh-80 Oct 11, 2021
15168a1
Fix PR comments.
liuh-80 Oct 12, 2021
a0911fa
Fix tab issue in patch file.
liuh-80 Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
314 changes: 314 additions & 0 deletions src/tacacs/pam/0007-Fix-memory-leak-when-parse-configuration.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,314 @@
From 95f09ea41d7179bbc321fad476cfe47cbdabdcab Mon Sep 17 00:00:00 2001
From: liuh-80 <58683130+liuh-80@users.noreply.github.com>
Date: Fri, 10 Sep 2021 17:27:05 +0800
Subject: [PATCH 1/2] Fix memory leak when parse configuration.

---
pam_tacplus.c | 34 +++++++++++++--------
support.c | 81 ++++++++++++++++++++++++++++++++++-----------------
support.h | 4 +--
3 files changed, 78 insertions(+), 41 deletions(-)

diff --git a/pam_tacplus.c b/pam_tacplus.c
index 9fc6be7..8a93c9b 100644
--- a/pam_tacplus.c
+++ b/pam_tacplus.c
@@ -138,8 +138,10 @@ int _pam_account(pam_handle_t *pamh, int argc, const char **argv,
syslog(LOG_DEBUG, "%s: tac_srv_no=%d", __FUNCTION__, tac_srv_no);
}

- if ((user = _pam_get_user(pamh)) == NULL)
+ if ((user = _pam_get_user(pamh)) == NULL) {
return PAM_USER_UNKNOWN;
+ }
+

if (ctrl & PAM_TAC_DEBUG)
syslog(LOG_DEBUG, "%s: username [%s] obtained", __FUNCTION__, user);
@@ -177,7 +179,7 @@ int _pam_account(pam_handle_t *pamh, int argc, const char **argv,

status = PAM_SESSION_ERR;
for(srv_i = 0; srv_i < tac_srv_no; srv_i++) {
- tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key, tac_source_addr, tac_timeout, __vrfname);
+ tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key, &tac_source_addr, tac_timeout, __vrfname);
if (tac_fd < 0) {
_pam_log(LOG_WARNING, "%s: error sending %s (fd)",
__FUNCTION__, typemsg);
@@ -208,6 +210,7 @@ int _pam_account(pam_handle_t *pamh, int argc, const char **argv,
signal(SIGCHLD, SIG_DFL);
signal(SIGHUP, SIG_DFL);
}
+
return status;
}

@@ -238,8 +241,9 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags,
syslog(LOG_DEBUG, "%s: called (pam_tacplus v%u.%u.%u)",
__FUNCTION__, PAM_TAC_VMAJ, PAM_TAC_VMIN, PAM_TAC_VPAT);

- if ((user = _pam_get_user(pamh)) == NULL)
+ if ((user = _pam_get_user(pamh)) == NULL) {
return PAM_USER_UNKNOWN;
+ }

if (ctrl & PAM_TAC_DEBUG)
syslog(LOG_DEBUG, "%s: user [%s] obtained", __FUNCTION__, user);
@@ -276,7 +280,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].addr, tac_srv[srv_i].key, tac_source_addr, tac_timeout, __vrfname);
+ tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key, &tac_source_addr, tac_timeout, __vrfname);
if (tac_fd < 0) {
_pam_log(LOG_ERR, "%s: connection to srv %d failed", __FUNCTION__, srv_i);
continue;
@@ -323,7 +327,8 @@ int pam_sm_authenticate (pam_handle_t * pamh, int flags,
status = PAM_SUCCESS;
communicating = 0;
active_server.addr = tac_srv[srv_i].addr;
- active_server.key = tac_srv[srv_i].key;
+ /* copy secret to key */
+ snprintf(active_server.key, sizeof(active_server.key), "%s", tac_srv[srv_i].key);

if (ctrl & PAM_TAC_DEBUG)
syslog(LOG_DEBUG, "%s: active srv %d", __FUNCTION__, srv_i);
@@ -537,8 +542,9 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags,
syslog (LOG_DEBUG, "%s: called (pam_tacplus v%u.%u.%u)"
, __FUNCTION__, PAM_TAC_VMAJ, PAM_TAC_VMIN, PAM_TAC_VPAT);

- if ((user = _pam_get_user(pamh)) == NULL)
+ if ((user = _pam_get_user(pamh)) == NULL) {
return PAM_USER_UNKNOWN;
+ }

if (ctrl & PAM_TAC_DEBUG)
syslog(LOG_DEBUG, "%s: username obtained [%s]", __FUNCTION__, user);
@@ -579,11 +585,13 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags,
if(tac_protocol[0] != '\0')
tac_add_attrib(&attr, "protocol", tac_protocol);

- tac_fd = tac_connect_single(active_server.addr, active_server.key, tac_source_addr, tac_timeout, __vrfname);
+ tac_fd = tac_connect_single(active_server.addr, active_server.key, &tac_source_addr, tac_timeout, __vrfname);
if(tac_fd < 0) {
_pam_log (LOG_ERR, "TACACS+ server unavailable");
- if(arep.msg != NULL)
+ if(arep.msg != NULL) {
free (arep.msg);
+ }
+
return PAM_AUTH_ERR;
}

@@ -664,7 +672,6 @@ int pam_sm_acct_mgmt (pam_handle_t * pamh, int flags,
free (arep.msg);

close(tac_fd);
-
return status;
} /* pam_sm_acct_mgmt */

@@ -726,8 +733,9 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,

if ( (pam_get_item(pamh, PAM_OLDAUTHTOK, &pam_pass) == PAM_SUCCESS)
&& (pam_pass != NULL) ) {
- if ((pass = strdup(pam_pass)) == NULL)
+ if ((pass = strdup(pam_pass)) == NULL) {
return PAM_BUF_ERR;
+ }
} else {
pass = strdup("");
}
@@ -736,6 +744,7 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
if(pass) {
free(pass);
}
+
return PAM_USER_UNKNOWN;
}

@@ -762,7 +771,7 @@ int pam_sm_chauthtok(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].addr, tac_srv[srv_i].key, tac_source_addr, tac_timeout, __vrfname);
+ tac_fd = tac_connect_single(tac_srv[srv_i].addr, tac_srv[srv_i].key, &tac_source_addr, tac_timeout, __vrfname);
if (tac_fd < 0) {
_pam_log(LOG_ERR, "connection failed srv %d: %m", srv_i);
continue;
@@ -820,7 +829,8 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
communicating = 0;

active_server.addr = tac_srv[srv_i].addr;
- active_server.key = tac_srv[srv_i].key;
+ /* copy secret to key */
+ snprintf(active_server.key, sizeof(active_server.key), "%s", tac_srv[srv_i].key);

if (ctrl & PAM_TAC_DEBUG)
syslog(LOG_DEBUG, "%s: active srv %d", __FUNCTION__, srv_i);
diff --git a/support.c b/support.c
index be1f21b..3d38f24 100644
--- a/support.c
+++ b/support.c
@@ -30,7 +30,12 @@
#include <stdlib.h>
#include <string.h>

+/* tacacs server information */
tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS];
+struct addrinfo tac_srv_addr[TAC_PLUS_MAXSERVERS];
+struct sockaddr tac_sock_addr[TAC_PLUS_MAXSERVERS];
+struct sockaddr_in6 tac_sock6_addr[TAC_PLUS_MAXSERVERS];
+
int tac_srv_no = 0;

char tac_service[64];
@@ -38,7 +43,11 @@ char tac_protocol[64];
char tac_prompt[64];
char *__vrfname=NULL;
char tac_source_ip[64];
-struct addrinfo *tac_source_addr = NULL;
+
+/* source address */
+struct addrinfo tac_source_addr;
+struct sockaddr tac_source_sock_addr;
+struct sockaddr_in6 tac_source_sock6_addr;

void _pam_log(int err, const char *format,...) {
char msg[256];
@@ -173,9 +182,38 @@ int tacacs_get_password (pam_handle_t * pamh, int flags
return PAM_SUCCESS;
}

+}
+/* set source ip address for the outgoing tacacs packets */
+void set_source_ip(const char *tac_source_ip) {
+ struct addrinfo hints, *source_address;
+ int rv;
+ /* set the source ip address for the tacacs packets */
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = AF_UNSPEC;
+ hints.ai_socktype = SOCK_STREAM;
+ if ((rv = getaddrinfo(tac_source_ip, NULL, &hints,
+ &source_address)) != 0) {
+ _pam_log(LOG_ERR, "error setting the source ip information");
+ } else {
+ memcpy(&tac_source_addr, source_address, sizeof(struct addrinfo));
+ if (source_address->ai_family == AF_INET6) {
+ tac_source_addr.ai_addr = (struct sockaddr *)&(tac_source_sock6_addr);
+ memcpy(tac_source_addr.ai_addr, source_address->ai_addr, sizeof(struct sockaddr_in6));
+ }
+ else {
+ tac_source_addr.ai_addr = &(tac_source_sock_addr);
+ memcpy(tac_source_addr.ai_addr, source_address->ai_addr, sizeof(struct sockaddr));
+ }
+
+
+ freeaddrinfo(source_address);
+ _pam_log(LOG_DEBUG, "source ip is set");
+ }
+}
int _pam_parse (int argc, const char **argv) {
int ctrl = 0;
- const char *current_secret = NULL;
+ char current_secret[256];
+ memset(current_secret, 0, sizeof(current_secret));

/* otherwise the list will grow with each call */
memset(tac_srv, 0, sizeof(tacplus_server_t) * TAC_PLUS_MAXSERVERS);
@@ -246,10 +284,16 @@ int _pam_parse (int argc, const char **argv) {
}
if ((rv = getaddrinfo(server_name, (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].addr = server;
- tac_srv[tac_srv_no].key = current_secret;
+ /* set server address with allocate memory */
+ set_tacacs_server_addr(tac_srv_no, server);
+
+ /* copy secret to key */
+ snprintf(tac_srv[tac_srv_no].key, sizeof(tac_srv[tac_srv_no].key), "%s", current_secret);
tac_srv_no++;
}
+
+ /* release servers memory */
+ freeaddrinfo(servers);
} else {
_pam_log (LOG_ERR,
"skip invalid server: %s (getaddrinfo: %s)",
@@ -262,14 +306,16 @@ int _pam_parse (int argc, const char **argv) {
} else if (!strncmp (*argv, "secret=", 7)) {
int i;

- current_secret = *argv + 7; /* points right into argv (which is const) */
+ /* points right into arg (which is const) */
+ snprintf(current_secret, sizeof(current_secret), "%s", arg + 7);

/* 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;
+ /* copy secret to key */
+ snprintf(tac_srv[i].key, sizeof(tac_srv[i].key), "%s", current_secret);
}
} else if (!strncmp (*argv, "timeout=", 8)) {
/* FIXME atoi() doesn't handle invalid numeric strings well */
@@ -284,8 +330,8 @@ int _pam_parse (int argc, const char **argv) {
__vrfname = strdup(*argv + 4);
} else if (!strncmp (*argv, "source_ip=", strlen("source_ip="))) {
/* source ip for the packets */
- strncpy (tac_source_ip, *argv + strlen("source_ip="), sizeof(tac_source_ip));
- set_source_ip (tac_source_ip, &tac_source_addr);
+ strncpy (tac_source_ip, *argv + strlen("source_ip="), sizeof(tac_source_ip));
+ set_source_ip (tac_source_ip);
} else {
_pam_log (LOG_WARNING, "unrecognized option: %s", *argv);
}
@@ -309,22 +355,3 @@ int _pam_parse (int argc, const char **argv) {

return ctrl;
} /* _pam_parse */
-
-/* set source ip address for the outgoing tacacs packets */
-void set_source_ip(const char *tac_source_ip,
- struct addrinfo **source_address) {
-
- struct addrinfo hints;
- int rv;
-
- /* set the source ip address for the tacacs packets */
- memset(&hints, 0, sizeof(hints));
- hints.ai_family = AF_UNSPEC;
- hints.ai_socktype = SOCK_STREAM;
- if ((rv = getaddrinfo(tac_source_ip, NULL, &hints,
- source_address)) != 0) {
- _pam_log(LOG_ERR, "error setting the source ip information");
- } else {
- _pam_log(LOG_DEBUG, "source ip is set");
- }
-}
diff --git a/support.h b/support.h
index b1faf43..feadbf5 100644
--- a/support.h
+++ b/support.h
@@ -28,7 +28,7 @@

typedef struct {
struct addrinfo *addr;
- const char *key;
+ char key[256];
} tacplus_server_t;

extern tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS];
@@ -37,7 +37,7 @@ extern int tac_srv_no;
extern char tac_service[64];
extern char tac_protocol[64];
extern char tac_prompt[64];
-extern struct addrinfo *tac_source_addr;
+extern struct addrinfo tac_source_addr;

int _pam_parse (int, const char **);
unsigned long _resolve_name (char *);
--
2.17.1.windows.2

Loading