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.

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
97 changes: 63 additions & 34 deletions src/tacacs/pam/0006-Add-support-for-source-ip-address.patch
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
From 9c26e734cf9e5cec950dc8b8f474f89d87833bcd Mon Sep 17 00:00:00 2001
From 49526a27e90647ed4e48c1d1d88e0c75a1ce221b Mon Sep 17 00:00:00 2001
From: Venkatesan Mahalingam <venkatesan_mahalinga@dell.com>
Date: Wed, 1 Jul 2020 18:57:28 -0700
Subject: [PATCH] Add support to specify source address for TACACS+
Date: Thu, 2 Jul 2020 09:57:28 +0800
Subject: [PATCH 1/4] Add support to specify source address for TACACS+

---
pam_tacplus.c | 8 ++++----
support.c | 31 +++++++++++++++++++++++++++++++
support.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++--
support.h | 1 +
3 files changed, 36 insertions(+), 4 deletions(-)
3 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/pam_tacplus.c b/pam_tacplus.c
index 38e2a70..ec8ea27 100644
index 7544b2e..9fc6be7 100644
--- a/pam_tacplus.c
+++ b/pam_tacplus.c
@@ -177,7 +177,7 @@ int _pam_account(pam_handle_t *pamh, int argc, const char **argv,
Expand Down Expand Up @@ -50,7 +50,7 @@ index 38e2a70..ec8ea27 100644
_pam_log(LOG_ERR, "connection failed srv %d: %m", srv_i);
continue;
diff --git a/support.c b/support.c
index 7c00618..3e55e2f 100644
index 8f42a0c..164df62 100644
--- a/support.c
+++ b/support.c
@@ -37,6 +37,8 @@ char tac_service[64];
Expand All @@ -62,60 +62,89 @@ index 7c00618..3e55e2f 100644

void _pam_log(int err, const char *format,...) {
char msg[256];
@@ -183,6 +185,12 @@ int _pam_parse (int argc, const char **argv) {
@@ -171,6 +173,44 @@ 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) {
+ /*
+ addrinfo created by getaddrinfo must be released with freeaddrinfo.
+ so source ip address will be stored in following static variables.
+ */
+ static struct addrinfo tac_source_address;
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use 4 spaces instead of 1 tab #Closed

+ static struct sockaddr tac_source_sock_addr;
+ static struct sockaddr_in6 tac_source_sock6_addr;
+
+ 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 {
+ tac_source_addr = &tac_source_address;
+ 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;
@@ -183,6 +223,12 @@ int _pam_parse (int argc, const char **argv) {
tac_protocol[0] = 0;
tac_prompt[0] = 0;
tac_login[0] = 0;
+ tac_source_ip[0] = 0;
+
+ if (tac_source_addr != NULL) {
+ freeaddrinfo(tac_source_addr);
+ /* reset source address */
+ tac_source_addr = NULL;
+ }

for (ctrl = 0; argc-- > 0; ++argv) {
if (!strcmp (*argv, "debug")) { /* all */
@@ -274,6 +282,10 @@ int _pam_parse (int argc, const char **argv) {
@@ -274,6 +320,10 @@ int _pam_parse (int argc, const char **argv) {
}
} else if(!strncmp(*argv, "vrf=", 4)) {
__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);
+ set_source_ip(tac_source_ip);
} else {
_pam_log (LOG_WARNING, "unrecognized option: %s", *argv);
}
@@ -292,8 +304,27 @@ int _pam_parse (int argc, const char **argv) {
@@ -292,8 +342,8 @@ int _pam_parse (int argc, const char **argv) {
_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);
+ _pam_log(LOG_DEBUG, "tac_source_ip='%s'", tac_source_ip);
}

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");
+ }
+}
-} /* _pam_parse */
-
+} /* _pam_parse */
\ No newline at end of file
diff --git a/support.h b/support.h
index 9cbd040..09b8a85 100644
index 9cbd040..b1faf43 100644
--- a/support.h
+++ b/support.h
@@ -37,6 +37,7 @@ extern int tac_srv_no;
Expand All @@ -127,5 +156,5 @@ index 9cbd040..09b8a85 100644
int _pam_parse (int, const char **);
unsigned long _resolve_name (char *);
--
2.7.4
2.17.1.windows.2

124 changes: 124 additions & 0 deletions src/tacacs/pam/0007-Fix-memory-leak-when-parse-configuration.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
From 99eeeccd14c905b7ad77210343bb07334eb0e8d1 Mon Sep 17 00:00:00 2001
From: liuh-80 <58683130+liuh-80@users.noreply.github.com>
Date: Tue, 12 Oct 2021 10:05:28 +0800
Subject: [PATCH 2/4] Fix memory leak when parse configuration.
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
The fix code in this patch are copy from upstream project: https://github.com/kravietz/pam_tacplus/blob/master/support.c

---
pam_tacplus.c | 6 ++++--
support.c | 37 +++++++++++++++++++++++++++++++++----
support.h | 2 +-
3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/pam_tacplus.c b/pam_tacplus.c
index 9fc6be7..d062359 100644
--- a/pam_tacplus.c
+++ b/pam_tacplus.c
@@ -323,7 +323,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);
@@ -820,7 +821,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 164df62..e22fa31 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];
@@ -173,6 +178,26 @@ int tacacs_get_password (pam_handle_t * pamh, int flags
return PAM_SUCCESS;
}

+/*
+ * Set tacacs server addrinfo.
+ */
+void set_tacacs_server_addr(int tac_srv_no, struct addrinfo* server) {
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
+ tac_srv[tac_srv_no].addr = &(tac_srv_addr[tac_srv_no]);
+ memcpy(tac_srv[tac_srv_no].addr, server, sizeof(struct addrinfo));
+
+ if (server->ai_family == AF_INET6) {
+ tac_srv[tac_srv_no].addr->ai_addr = (struct sockaddr *)&(tac_sock6_addr[tac_srv_no]);
+ memcpy(tac_srv[tac_srv_no].addr->ai_addr, server->ai_addr, sizeof(struct sockaddr_in6));
+ }
+ else {
+ tac_srv[tac_srv_no].addr->ai_addr = &(tac_sock_addr[tac_srv_no]);
+ memcpy(tac_srv[tac_srv_no].addr->ai_addr, server->ai_addr, sizeof(struct sockaddr));
+ }
+
+ tac_srv[tac_srv_no].addr->ai_canonname = NULL;
+ tac_srv[tac_srv_no].addr->ai_next = NULL;
+}
+
/* set source ip address for the outgoing tacacs packets */
void set_source_ip(const char *tac_source_ip) {
/*
@@ -284,8 +309,11 @@ 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);
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
+
+ /* copy secret to key */
+ snprintf(tac_srv[tac_srv_no].key, sizeof(tac_srv[tac_srv_no].key), "%s", current_secret);
tac_srv_no++;
}
} else {
@@ -304,10 +332,11 @@ int _pam_parse (int argc, const char **argv) {

/* 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)
+ if (tac_srv[i].key[0] != 0)
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 */
diff --git a/support.h b/support.h
index b1faf43..6bcb07f 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];
--
2.17.1.windows.2

Loading