[PATCH] smbstatus output for sessions with authentication still in progress

Ralph Böhme slow at samba.org
Wed Nov 22 10:01:36 UTC 2017


On Tue, Nov 21, 2017 at 03:07:26PM -0800, Jeremy Allison wrote:
> On Tue, Nov 21, 2017 at 05:53:48PM +0100, Ralph Böhme via samba-technical wrote:
> > Hi!
> > 
> > Currently smbstatus displays -1 for user and group info when it hits a session
> > where authentication is still in progress.
> > 
> > The attached patch changes this to display
> > 
> >     PID  Username   Group    Machine                          Protocol Version ....
> >     6604 (auth in progress)  127.0.0.1 (ipv4:127.0.0.1:47930) SMB3_11 ....
> > 
> > instead, which should help users make sense of the output.
> > 
> > Please review & push if happy. Thanks!
> 
> Very slight NAK on this. The reason is if someone has
> set numeric_only=true then it changes the output for
> an auth in progress from "-1 -1" to "(auth in progress)"
> which may break scripts that are expecting only numbers
> (even invalid ones :-) in this field.
> 
> Can you add an if statement that re-adds the "-1 -1" output
> in the "The session is not fully authenticated yet."
> clause ?

sure. Updated patch attached.

> Sorry for being nit-picky :-).

No prob.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
-------------- next part --------------
From 43f44641629469a757bce9e03dc4595e12a46aa1 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 22 Nov 2017 10:43:19 +0100
Subject: [PATCH 1/2] s3/smbstatus: add a NULL check

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/utils/status.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/source3/utils/status.c b/source3/utils/status.c
index abc0d26df53..dd196b64a47 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -386,7 +386,12 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
 		if (numeric_only) {
 			fstr_sprintf(uid_str, "%u", (unsigned int)session->uid);
 		} else {
-			fstrcpy(uid_str, uidtoname(session->uid));
+			const char *uid_name = uidtoname(session->uid);
+
+			if (uid_name == NULL) {
+				return -1;
+			}
+			fstrcpy(uid_str, uid_name);
 		}
 	}
 
@@ -396,6 +401,11 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
 		if (numeric_only) {
 			fstr_sprintf(gid_str, "%u", (unsigned int)session->gid);
 		} else {
+			const char *gid_name = gidtoname(session->gid);
+
+			if (gid_name == NULL) {
+				return -1;
+			}
 			fstrcpy(gid_str, gidtoname(session->gid));
 		}
 	}
-- 
2.13.6


From 7579145be144ac3451ad0bc71464d43d7ca5488e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 4 Jul 2017 12:22:00 +0200
Subject: [PATCH 2/2] smbstatus: correctly denote not fully authenticated
 sessions

Currently for sessions where authentication is still in progress we
print uid and gid as -1.

With this change we nicely list them like this:

PID  Username   Group    Machine                          Protocol Version ....
6604 (auth in progress)  127.0.0.1 (ipv4:127.0.0.1:47930) SMB3_11 ....

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/utils/status.c | 61 ++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/source3/utils/status.c b/source3/utils/status.c
index dd196b64a47..dfb1d921a42 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -365,7 +365,7 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
 			      void *private_data)
 {
 	TALLOC_CTX *mem_ctx = (TALLOC_CTX *)private_data;
-	fstring uid_str, gid_str;
+	fstring uid_gid_str;
 	struct server_id_buf tmp;
 	char *machine_hostname = NULL;
 	int result = 0;
@@ -380,33 +380,40 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
 
 	Ucrit_addPid(session->pid);
 
-	fstrcpy(uid_str, "-1");
-
-	if (session->uid != -1) {
-		if (numeric_only) {
-			fstr_sprintf(uid_str, "%u", (unsigned int)session->uid);
+	if (numeric_only) {
+		fstr_sprintf(uid_gid_str, "%-12u %-12u",
+			     (unsigned int)session->uid,
+			     (unsigned int)session->gid);
+	} else {
+		if (session->uid == -1 && session->gid == -1) {
+			/*
+			 * The session is not fully authenticated yet.
+			 */
+			fstrcpy(uid_gid_str, "(auth in progress)");
 		} else {
-			const char *uid_name = uidtoname(session->uid);
-
-			if (uid_name == NULL) {
-				return -1;
+			/*
+			 * In theory it should not happen that one of
+			 * session->uid and session->gid is valid (ie != -1)
+			 * while the other is not (ie = -1), so we a check for
+			 * that case that bails out would be reasonable.
+			 */
+			const char *uid_name = "-1";
+			const char *gid_name = "-1";
+
+			if (session->uid != -1) {
+				uid_name = uidtoname(session->uid);
+				if (uid_name == NULL) {
+					return -1;
+				}
 			}
-			fstrcpy(uid_str, uid_name);
-		}
-	}
-
-	fstrcpy(gid_str, "-1");
-
-	if (session->gid != -1) {
-		if (numeric_only) {
-			fstr_sprintf(gid_str, "%u", (unsigned int)session->gid);
-		} else {
-			const char *gid_name = gidtoname(session->gid);
-
-			if (gid_name == NULL) {
-				return -1;
+			if (session->gid != -1) {
+				gid_name = gidtoname(session->gid);
+				if (gid_name == NULL) {
+					return -1;
+				}
 			}
-			fstrcpy(gid_str, gidtoname(session->gid));
+			fstr_sprintf(uid_gid_str, "%-12s %-12s",
+				     uid_name, gid_name);
 		}
 	}
 
@@ -467,9 +474,9 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
 	}
 
 
-	d_printf("%-7s %-12s %-12s %-41s %-17s %-20s %-21s\n",
+	d_printf("%-7s %-25s %-41s %-17s %-20s %-21s\n",
 		 server_id_str_buf(session->pid, &tmp),
-		 uid_str, gid_str,
+		 uid_gid_str,
 		 machine_hostname,
 		 session_dialect_str(session->connection_dialect),
 		 encryption,
-- 
2.13.6



More information about the samba-technical mailing list