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

[Jeremy Allison]

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 ?

Sorry for being nit-picky :-).

Jeremy.

> -- 
> Ralph Boehme, Samba Team       https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

> From 964b5f9a2d6bb9d300b0bb4b794ff1d5a58e97a1 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] 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 | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/source3/utils/status.c b/source3/utils/status.c
> index abc0d26df53..f69326e888a 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,24 +380,22 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
>  
>  	Ucrit_addPid(session->pid);
>  
> -	fstrcpy(uid_str, "-1");
> +	if (session->uid == -1 && session->gid == -1) {
> +		/*
> +		 * The session is not fully authenticated yet.
> +		 */
> +		fstrcpy(uid_gid_str, "(auth in progress)");
> +	} else {
> +		fstring uid_str, gid_str;
>  
> -	if (session->uid != -1) {
>  		if (numeric_only) {
>  			fstr_sprintf(uid_str, "%u", (unsigned int)session->uid);
> -		} else {
> -			fstrcpy(uid_str, uidtoname(session->uid));
> -		}
> -	}
> -
> -	fstrcpy(gid_str, "-1");
> -
> -	if (session->gid != -1) {
> -		if (numeric_only) {
>  			fstr_sprintf(gid_str, "%u", (unsigned int)session->gid);
>  		} else {
> +			fstrcpy(uid_str, uidtoname(session->uid));
>  			fstrcpy(gid_str, gidtoname(session->gid));
>  		}
> +		fstr_sprintf(uid_gid_str, "%s %s", uid_str, gid_str);
>  	}
>  
>  	machine_hostname = talloc_asprintf(mem_ctx, "%s (%s)",
> @@ -457,9 +455,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
>