aboutsummaryrefslogtreecommitdiff
path: root/package/libpam-tacplus/0005-Check-for-failure-of-OpenSSL-RAND_pseudo_bytes.patch
blob: ba87ca5a191a656e6541dabee8415bbdd8814a66 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
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 */