[PATCH] smbstatus output for sessions with authentication still in progress
Jeremy Allison
jra at samba.org
Mon Nov 27 19:37:28 UTC 2017
On Wed, Nov 22, 2017 at 11:01:36AM +0100, Ralph Böhme wrote:
> 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.
LGTM. Pushed ! Thanks Ralph.
> --
> Ralph Boehme, Samba Team https://samba.org/
> Samba Developer, SerNet GmbH https://sernet.de/en/samba/
> 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