[PATCH] Add posix_whoami command to smbclient.

[Jeremy Allison]

On Wed, May 25, 2016 at 10:27:48PM +0300, Uri Simchoni wrote:
> On 05/25/2016 07:31 PM, Jeremy Allison wrote:
> > Does what it says on the tin :-).
> > 
> > Adds async cli_posix_whoami interfaces to
> > the cliXXX library, and adds a command in
> > smbclient to execute it (plus documentation).
> > 
> > I'm doing this so it will be easier to write
> > tests that explore what the server process
> > token looks like under various different
> > setups.
> > 
> > It's also pretty handy for debugging
> > authorization problems for a logged-on
> > user.
> > 
> > Please review and push if happy !
> > 
> > Jeremy.
> > 
> > 
> 
> RB+ me with few small comments.
> 
> First patch:
> > +
> > +	status = cli_trans_recv(subreq,
> > +				state,
> > +				NULL,
> > +				NULL,
> > +				0,
> > +				NULL,
> > +                                NULL,
> > +				0,
> > +				NULL,
> > +				&rdata,
> > +				40,
> > +				&num_rdata);
> > +	TALLOC_FREE(subreq);
> > +        if (tevent_req_nterror(req, status)) {
> > +                return;
> > +        }
> spaces -> tabs on last three lines

Already fixed :-).

> > +	if (num_rdata < 40 || rdata + num_rdata < rdata) {
> > +		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
> > +		return;
> > +	}
> Do we need this test? Doesn't cli_trans_recv guarantee we have at least
> 40 bytes, plus guarantee rdata is a bona-fide array, and so num_rdata is
> not overflowing? Your call.

Yes, this test isn't needed. I deliberately
added it to remind me that checking for overflows
is mandatory and should never be forgotten. Plus
it makes it obvious to third-parties that yes,
we care and check for this stuff :-).

> > +
> > +	for (i = 0; i < state->num_gids; i++) {
> > +		if (p + 8 > rdata + num_rdata) {
> > +			tevent_req_nterror(req,
> > +				NT_STATUS_INVALID_NETWORK_RESPONSE);
> > +			return;
> > +		}
> > +		state->gids[i] = IVAL(p, 0);
> Are we losing the high 32 bits here? The server side sets all 64 bits

Good catch ! I'll fix immediately.

> > +		p += 8;
> > +	}
> > +
> 
> Second patch:
> > +static int cmd_posix_whoami(void)
> > +{
> > +	TALLOC_CTX *ctx = talloc_tos();
> > +	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
> > +	uint64_t uid = 0;
> > +	uint64_t gid= 0;
> 
> Missing space after '='

Thanks !

Jeremy.