[PATCH] Add posix_whoami command to smbclient.
Jeremy Allison
jra at samba.org
Wed May 25 19:37:30 UTC 2016
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.
More information about the samba-technical
mailing list