bug#8427: [SECURITY] sql.el -- comint process passwords are leaked to ps(1) listing

[Michael Mauger]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, October 20, 2019 8:56 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:

>
>
> Thanks for the insightful comments - yes, everything you say makes
> sense. I've implemented what you describe. However, I'm a little unsure
> of this one - I had to advise a comint primitive and even re-implement
> part of an existing comint function. It feels like comint should perhaps
> have a way to do this sort of thing within itself, but I couldn't find
> any.
>
> I've attached the latest revision.
>
> Stefan Kangas stefan@marxist.se writes:
>
> > (Please keep the bug address in Cc.)
> > Andrew Hyatt ahyatt@gmail.com writes:
> >
> > > I'm attaching the fix. The fix for MySQL was fairly straightforward. I
> > > tried it out, and it works.
> >
> > I'm not sure this is the right fix. How is the user to know that the
> > correct thing is to provide an empty password when prompted for it?
> > Why do we even prompt for the password then?
> > Also, what if a user wants to login to an account that has no
> > password? Should we really pass the "--password" parameter in that
> > case? Does that work?
> > I think something like this would be better:
> >
> > 1.  Keep the password prompt.
> > 2.  Use the naked "--password" parameter only when the user has
> >     entered a password, and use nothing when the user entered nothing.
> >
> > 3.  Never use the "--password=<foo>" parameter.
> > 4.  When mysql prompts for the password, send it to the process
> >     automatically, without user interaction.
> >
> >
> > > I looked through sql.el for similar issues,
> > > and was able to fix Vertica as well, although I've never heard of
> > > Vertica before and couldn't test it out. Parameters were set according
> > > to the docs at
> > > https://www.vertica.com/docs/9.2.x/HTML/Content/Authoring/ConnectingToVertica/vsql/CommandLineOptions.htm,
> > > which does match the existing code.
> >
> > Unless someone can test it, perhaps we should leave out the Vertica part?
> > Thanks for working on this.
> > Best regards,
> > Stefan Kangas

I have tried a couple of different versions of this in the past but have found 
a lot of corner cases that made me back off.

Some thoughts:

* The login-params function will set `sql-password' to nil if it isn't a 
parameter being prompted for and is not set otherwise. If it is prompted for 
and empty the variable will be an empty string not nil. We need some test cases 
written to confirm that the behavior is as we expect. Small shell scripts can 
be created to simulate the SQL processor for the general flow. The test scripts 
should be included in the commit for this feature.

* Only supply the password using the comint password filter if support for 
passing the password on stdin is supported and expected in this instance. This 
would probably be a flag set in the `sql-PRODUCT-comint-func' (based on the 
command line logic) and set as a buffer local in the buffer. The comint filter 
would then check the flag before trying to stuff the password into the stream. 
That avoids sending a database password to a prompt that is for other purposes. 
Also does this have to be advice to the comint filter or just another filter 
installed on the comint hook? (Policy is that standard Emacs packages do not 
use advice and the hook is present and used by the existing hook.)

* Only send the password to the first time a password is asked for. Some 
interactive sql processes allow changing the connection mid-session and the 
password for the original username may not be appropriate for the new 
connection they made. This is especially true in enterprise environments where 
password failures can be set to disable the database user.

* Please do not alter indenting of existing code not involved in the change; 
the indentation is deliberate in some cases and the spurious changes just 
generate noise in the diffs. Thanks.

* Are we adding a flag to the `sql-product-alist' to indicate that passwords 
may be passed via stdin? I would recommend that we do so because that way it 
can be globally disabled if the environment calls for it. For example, a user 
may want to supply it on the command line because it is not a concen in their 
environment. Some database products support passing the password this way, but 
also alter the command line parameters to mask out the actual password so that 
the `ps' exposure is fairly small.

Thanks working on this but I'm still concerned that we could break existing use 
of sql-interactive-mode unintentionally.

--
MICHAEL@MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer

0001-Enable-password-less-connections-for-sql-where-possi.patch
Description: Text Data