[PATCH 1/2] s3fs-popt: Add function to burn the commandline password.

[Andreas Schneider]

On Friday 09 November 2012 15:17:13 Jelmer Vernooij wrote:
> On Fri, 2012-11-09 at 10:06 +0100, Andreas Schneider wrote:
> > On Thursday 08 November 2012 12:55:32 you wrote:
> > > On Thu, Nov 08, 2012 at 09:43:51PM +0100, Jelmer Vernooij wrote:
> > > > Hi Andreas, Jeremy,
> > > > 
> > > > On Tue, 2012-11-06 at 09:27 +0100, Andreas Schneider wrote:
> > > > > Signed-off-by: Andreas Schneider <asn at samba.org>
> > > > > ---
> > > > > 
> > > > >  source3/include/popt_common.h |  1 +
> > > > >  source3/lib/popt_common.c     | 47
> > > > >  +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48
> > > > >  insertions(+)
> > > > > 
> > > > > diff --git a/source3/include/popt_common.h
> > > > > b/source3/include/popt_common.h
> > > > > index 2125ed6..5266f36 100644
> > > > > --- a/source3/include/popt_common.h
> > > > > +++ b/source3/include/popt_common.h
> > > > > @@ -49,5 +49,6 @@ extern const struct poptOption
> > > > > popt_common_dynconfig[];
> > > > > 
> > > > >  #define POPT_COMMON_OPTION { NULL, 0, POPT_ARG_INCLUDE_TABLE,
> > > > >  popt_common_option, 0, "Common samba commandline config:", NULL },
> > > > >  
> > > > >  void popt_common_set_auth_info(struct user_auth_info *auth_info);
> > > > > 
> > > > > +void popt_burn_cmdline_password(int argc, char *argv[]);
> > > > 
> > > > Looking at this code again, I'm not sure I understand what this does
> > > > exactly.
> > > > 
> > > > Right after parsing the username with included password, we already
> > > > take
> > > > care of erasing the password in memory in argv (see
> > > > source3/lib/popt_common.c:555). What this do exactly that isn't
> > > > covered
> > > > by that code? Am I misunderstanding the current code?
> > > 
> > > I wasn't aware of that code in source3/lib/popt_common.c:555.
> > 
> > The code works on puser which is a duplicated memory:
> > 
> > 545 »·······»·······»·······char *puser = SMB_STRDUP(arg);
> > 
> > > It does look like it's doing something similar.
> > 
> > It burns the password in memory before freeing puser.
> 
> I suspect freeing the contents of argv was its original purpose though.
> Wouldn't this also be a more appropriate place to free the argv memory?
> 
> > > Andreas, can you check if this has the same effect (I just
> > > checked that the password part of the argv was erased by
> > > the patch, I didn't check if it was already being done
> > > somewhere else :-).
> > 
> > It is something different.
> 
> I think it was meant to do the same thing.

I don't think so. You recognized that 'arg' is a 'const char *'? It always has 
been a copy of 'arg'.

See lib/popt/popt.h line 215.


	-- andreas

-- 
Andreas Schneider                   GPG-ID: F33E3FC6
Samba Team                             asn at samba.org
www.samba.org