--- /dev/null
+From c9bed7496e81e550ee22746f23bbb11be2e046ed Mon Sep 17 00:00:00 2001
+From: Duncan Eastoe <duncan.eastoe@att.com>
+Date: Thu, 22 Oct 2020 19:18:27 +0100
+Subject: [PATCH] magic.c: check for failure of RAND_[pseudo_]bytes
+
+When magic() is implemented via libcrypto's RAND_bytes or
+RAND_pseudo_bytes we should check for a failure and abort to
+ensure we don't use a predictable session_id.
+
+This prevents (further) weakening* of the TACACS+ protocol
+"encryption" since session_id is an input to the algorithm.
+
+*by modern standards TACACS+ is deemed "obfuscated" - RFC 8907.
+
+[Retrieved from:
+https://github.com/kravietz/pam_tacplus/commit/468cc9d484364ecdc8bb245805f5c1fcb415fec9]
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+---
+ libtac/lib/magic.c | 26 +++++++++++++++++++++-----
+ pam_tacplus.c | 6 ++++++
+ 2 files changed, 27 insertions(+), 5 deletions(-)
+
+diff --git a/libtac/lib/magic.c b/libtac/lib/magic.c
+index e13a483..ae6b44f 100644
+--- a/libtac/lib/magic.c
++++ b/libtac/lib/magic.c
+@@ -81,26 +81,42 @@ magic()
+
+ #elif defined(HAVE_OPENSSL_RAND_H) && defined(HAVE_LIBCRYPTO)
+
++#include <openssl/err.h>
+ #include <openssl/rand.h>
+
+ /* RAND_bytes is OpenSSL's classic function to obtain cryptographic strength pseudo-random bytes
+- however, since the magic() function is used to generate TACACS+ session id rather than crypto keys
+- we can use RAND_pseudo_bytes() which doesn't deplete the system's entropy pool
++ however, we can use RAND_pseudo_bytes() which doesn't deplete the system's entropy pool, so long
++ as it returns a "cryptographically strong" result - since session_id is an input to the TACACS+
++ "encryption" ("obfuscation" by modern standards - RFC 8907) algorithm.
+ */
+
+ u_int32_t
+ magic()
+ {
+ u_int32_t num;
++ int ret;
+
+ #ifdef HAVE_RAND_BYTES
+- RAND_bytes((unsigned char *)&num, sizeof(num));
++ ret = RAND_bytes((unsigned char *)&num, sizeof(num));
+ #elif HAVE_RAND_PSEUDO_BYTES
+- RAND_pseudo_bytes((unsigned char *)&num, sizeof(num));
++ ret = RAND_pseudo_bytes((unsigned char *)&num, sizeof(num));
+ #else
+ #error Neither RAND_bytes nor RAND_pseudo_bytes seems to be available
+ #endif
+- return num;
++
++ /* RAND_bytes success / RAND_pseudo_bytes "cryptographically strong" result */
++ if (ret == 1)
++ return num;
++
++ TACSYSLOG(LOG_CRIT,"%s: "
++#ifdef HAVE_RAND_BYTES
++ "RAND_bytes "
++#else
++ "RAND_pseudo_bytes "
++#endif
++ "failed; ret: %d err: %ld", __FUNCTION__, ret, ERR_get_error());
++
++ exit(1);
+ }
+
+ #else
+diff --git a/pam_tacplus.c b/pam_tacplus.c
+index a0cb83d..4999ca9 100644
+--- a/pam_tacplus.c
++++ b/pam_tacplus.c
+@@ -718,6 +718,12 @@ int pam_sm_acct_mgmt(pam_handle_t *pamh, int UNUSED(flags), int argc,
+ PAM_EXTERN
+ int pam_sm_open_session(pam_handle_t *pamh, int UNUSED(flags), int argc,
+ const char **argv) {
++
++/* Task ID has no need to be cryptographically strong so we don't
++ * check for failures of the RAND functions. If they fail then we are
++ * as well sending the accounting request regardless of whether any value
++ * was written to task_id.
++ */
+ #if defined(HAVE_OPENSSL_RAND_H) && defined(HAVE_LIBCRYPTO)
+ # if defined(HAVE_RAND_BYTES)
+ RAND_bytes((unsigned char *) &task_id, sizeof(task_id));
+
+From bceaab0cd51a09b88f40f19da799ac7390264bf8 Mon Sep 17 00:00:00 2001
+From: Duncan Eastoe <duncan.eastoe@att.com>
+Date: Fri, 23 Oct 2020 11:23:07 +0100
+Subject: [PATCH 2/2] pam_tacplus.c: Fallback to using PID as task ID
+
+If there is a failure obtaining a random task ID for the session
+accounting request then fallback to using the PID, as this is unique
+for the lifetime of the PAM application and therefore session.
+---
+ pam_tacplus.c | 16 ++++++++++++----
+ 1 file changed, 12 insertions(+), 4 deletions(-)
+
+diff --git a/pam_tacplus.c b/pam_tacplus.c
+index 4999ca9..b69e3d0 100644
+--- a/pam_tacplus.c
++++ b/pam_tacplus.c
+@@ -100,8 +100,13 @@ int _pam_send_account(int tac_fd, int type, const char *user, char *tty,
+ } else if (type == TAC_PLUS_ACCT_FLAG_STOP) {
+ tac_add_attrib(&attr, "stop_time", buf);
+ }
+- sprintf(buf, "%hu", task_id);
++
++ if (task_id == 0)
++ snprintf(buf, sizeof(buf), "%d", getpid());
++ else
++ snprintf(buf, sizeof(buf), "%hu", task_id);
+ tac_add_attrib(&attr, "task_id", buf);
++
+ tac_add_attrib(&attr, "service", tac_service);
+ if (tac_protocol[0] != '\0')
+ tac_add_attrib(&attr, "protocol", tac_protocol);
+@@ -720,9 +725,8 @@ int pam_sm_open_session(pam_handle_t *pamh, int UNUSED(flags), int argc,
+ const char **argv) {
+
+ /* Task ID has no need to be cryptographically strong so we don't
+- * check for failures of the RAND functions. If they fail then we are
+- * as well sending the accounting request regardless of whether any value
+- * was written to task_id.
++ * check for failures of the RAND functions. If we fail to get an ID we
++ * fallback to using our PID (in _pam_send_account).
+ */
+ #if defined(HAVE_OPENSSL_RAND_H) && defined(HAVE_LIBCRYPTO)
+ # if defined(HAVE_RAND_BYTES)
+@@ -734,6 +738,10 @@ int pam_sm_open_session(pam_handle_t *pamh, int UNUSED(flags), int argc,
+ task_id=(short int) magic();
+ #endif
+
++ if (task_id == 0)
++ syslog(LOG_INFO, "%s: failed to generate random task ID, "
++ "falling back to PID", __FUNCTION__);
++
+ return _pam_account(pamh, argc, argv, TAC_PLUS_ACCT_FLAG_START, NULL);
+ } /* pam_sm_open_session */
+
projects / buildroot.git / commitdiff