From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Andrew Hyatt Newsgroups: gmane.emacs.bugs Subject: bug#8427: [SECURITY] sql.el -- comint process passwords are leaked to ps(1) listing Date: Sun, 20 Oct 2019 20:56:32 -0400 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="16149"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (darwin) Cc: Michael Mauger , 8427@debbugs.gnu.org To: Stefan Kangas Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Oct 21 02:57:11 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1iMM0V-00043V-Av for geb-bug-gnu-emacs@m.gmane.org; Mon, 21 Oct 2019 02:57:11 +0200 Original-Received: from localhost ([::1]:54226 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iMM0U-0004kn-3o for geb-bug-gnu-emacs@m.gmane.org; Sun, 20 Oct 2019 20:57:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:44141) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iMM0O-0004kX-64 for bug-gnu-emacs@gnu.org; Sun, 20 Oct 2019 20:57:05 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iMM0M-0003fz-Eg for bug-gnu-emacs@gnu.org; Sun, 20 Oct 2019 20:57:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:46678) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iMM0M-0003ft-9K for bug-gnu-emacs@gnu.org; Sun, 20 Oct 2019 20:57:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1iMM0M-000086-6r for bug-gnu-emacs@gnu.org; Sun, 20 Oct 2019 20:57:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Andrew Hyatt Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 21 Oct 2019 00:57:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 8427 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: security Original-Received: via spool by 8427-submit@debbugs.gnu.org id=B8427.1571619410477 (code B ref 8427); Mon, 21 Oct 2019 00:57:02 +0000 Original-Received: (at 8427) by debbugs.gnu.org; 21 Oct 2019 00:56:50 +0000 Original-Received: from localhost ([127.0.0.1]:55499 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iMM08-00007b-EL for submit@debbugs.gnu.org; Sun, 20 Oct 2019 20:56:50 -0400 Original-Received: from mail-qt1-f174.google.com ([209.85.160.174]:33857) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iMM06-00007M-FE for 8427@debbugs.gnu.org; Sun, 20 Oct 2019 20:56:46 -0400 Original-Received: by mail-qt1-f174.google.com with SMTP id 3so18412847qta.1 for <8427@debbugs.gnu.org>; Sun, 20 Oct 2019 17:56:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=WNX/WVq8cA+HormgP9pRPbnsxXtcFbZBjP8q/zN6NoY=; b=Hep5ieDTwWkqnVgB5KZMuJJ4nodlP3Mj5JwRrHpAFbHSFiqH50/Dc8GyO4XDBXrL6s HY2MJrPw7/ZD1hv1reXl88znYOUn6DoGoTX0goCKc3r4ZZu2pSw2ywNSXSYF+3lpJFTF GA0/1UGzSU+btamQcpkj/yjjS0EKiP3gqvTMeA4EONncVeP2p/GmQO9uPOk05stO3ma/ OuomZXOML9Y0oI7eTyJIqvhYteT4bvAau3Mm/3CB9r4wFiqN8t+MEnw2XkAcQlBE0xJ9 cKGAugP66w5M2tT6oY9VrqZIltzThLpRUnZNUpUngDrG8wUvrTgVPnziKv4A+4DUYygE BlmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=WNX/WVq8cA+HormgP9pRPbnsxXtcFbZBjP8q/zN6NoY=; b=oBPoqwLMjd7pE4s2m4DN1nw8ZOy2nBtobhbLrMABxPMLXa1vZG1KiVTG3cqqujlLfA FzIJNhrp1uMAvxTU9yNY0wlNpCd/uQUKPCzrvD6wlve3lw73710xmwrIt2IxFxIVSMbT nTldBjfq4gjFfDaq8qLdtKnv2pRqFKSfVUiW+gJojv5JXtUuKDI1Mtkn+Ly5hkn8Ibe/ erLHp8Zmx5/nQrz5ad1Rx7B4/YXnlGE3krUR+yW8hhxt6iNONgpQ13gVcb/isCl5sP2S v3kTDHezCH63TILNcNpwSU3nQJVXmtlhPtXsthvxk4trhYCsmTlm0oemij5w9FZ4BEuW 56QQ== X-Gm-Message-State: APjAAAUC7RWrQHDQ0obweIoOCew61PMi39sxTODDAqqChueAL4mhKr/W rAg4lsBZIluUdKv5myY384Bv6G4z7Ks= X-Google-Smtp-Source: APXvYqyldFOtcE8taE9csQT1IpdQCGuZrQw1hA0RUFgfjc3vjx0jP70s4/pAXtry+npoyoeBSxqV6A== X-Received: by 2002:ac8:3408:: with SMTP id u8mr22222992qtb.380.1571619400593; Sun, 20 Oct 2019 17:56:40 -0700 (PDT) Original-Received: from ahyatt-macbookpro6.roam.corp.google.com (ec2-52-55-121-20.compute-1.amazonaws.com. [52.55.121.20]) by smtp.gmail.com with ESMTPSA id l185sm7678293qkd.20.2019.10.20.17.56.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 20 Oct 2019 17:56:39 -0700 (PDT) In-Reply-To: (Stefan Kangas's message of "Sun, 20 Oct 2019 18:02:40 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:169877 Archived-At: --=-=-= Content-Type: text/plain 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. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Enable-password-less-connections-for-sql-where-possi.patch Content-Description: Draft 2 of mysql patch >From 610d4d8c9bb5f04a86afc8a63b671bd035d24e36 Mon Sep 17 00:00:00 2001 From: Andrew Hyatt Date: Fri, 18 Oct 2019 21:56:52 -0400 Subject: [PATCH] Enable password-less connections for sql where possible. * lisp/progmodes/sql.el (sql-comint-mysql): When a blank password is provided (not entered by the user), send an argument to signal to the SQL process to read the password inside the process. This removes the slight chance that someone can spy on the password from ps or via other methods. We also watch for the password inside the SQL process and automatically fill it with `sql-password' (if it exists). --- lisp/progmodes/sql.el | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el index b17364b08f..c453de382d 100644 --- a/lisp/progmodes/sql.el +++ b/lisp/progmodes/sql.el @@ -160,13 +160,16 @@ ;; "Connect ti XyzDB in a comint buffer." ;; ;; ;; Do something with `sql-user', `sql-password', -;; ;; `sql-database', and `sql-server'. +;; ;; `sql-database', and `sql-server'. `sql-password' will +;; ;; be sent automatically if not sent in the command-line. +;; ;; It is recommended to avoid sending in the command-line +;; ;; if possible, since this can briefly expose passwords. ;; (let ((params ;; (append ;; (if (not (string= "" sql-user)) ;; (list "-U" sql-user)) ;; (if (not (string= "" sql-password)) -;; (list "-P" sql-password)) +;; (list "-P")) ;; (if (not (string= "" sql-database)) ;; (list "-D" sql-database)) ;; (if (not (string= "" sql-server)) @@ -4664,8 +4667,8 @@ the call to \\[sql-product-interactive] with (sql-database (default-value 'sql-database)) (sql-port (default-value 'sql-port)) (default-directory - (or sql-default-directory - default-directory))) + (or sql-default-directory + default-directory))) ;; The password wallet returns a function which supplies the password. (when (functionp sql-password) @@ -4681,9 +4684,9 @@ the call to \\[sql-product-interactive] with (sql-generate-unique-sqli-buffer-name product nil)) ((consp new-name) (sql-generate-unique-sqli-buffer-name product - (read-string - "Buffer name (\"*SQL: XXX*\"; enter `XXX'): " - (sql-make-alternate-buffer-name product)))) + (read-string + "Buffer name (\"*SQL: XXX*\"; enter `XXX'): " + (sql-make-alternate-buffer-name product)))) ((stringp new-name) (if (or (string-prefix-p " " new-name) (string-match-p "\\`[*].*[*]\\'" new-name)) @@ -4733,12 +4736,27 @@ the call to \\[sql-product-interactive] with (get-buffer new-sqli-buffer))))) (user-error "No default SQL product defined: set `sql-product'"))) +(define-advice comint-watch-for-password-prompt + (:around (inner-func string) sql-password-autopopulate) + "Intercept password prompts when we know the password. This +must also do the job of detecting password prompts. STRING is +the potential password prompt. INNER-FUNC is the previous +definition of comint-watch-for-password-prompt, which is called +only when there is no prefilled password." + (if (and + (eq major-mode 'sql-interactive-mode) + (not (string= "" sql-password)) + (let ((case-fold-search t)) + (string-match comint-password-prompt-regexp string))) + (funcall comint-input-sender (get-buffer-process (current-buffer)) sql-password) + (funcall inner-func string))) + (defun sql-comint (product params &optional buf-name) "Set up a comint buffer to run the SQL processor. -PRODUCT is the SQL product. PARAMS is a list of strings which are -passed as command line arguments. BUF-NAME is the name of the new -buffer. If nil, a name is chosen for it." +PRODUCT is the SQL product. PARAMS is a list of strings which +are passed as command line arguments. BUF-NAME is the name of +the new buffer. If nil, a name is chosen for it." (let ((program (sql-get-product-feature product :sqli-program))) ;; Make sure we can find the program. `executable-find' does not @@ -5188,7 +5206,9 @@ The default comes from `process-coding-system-alist' and (if (not (string= "" sql-user)) (list (concat "--user=" sql-user))) (if (not (string= "" sql-password)) - (list (concat "--password=" sql-password))) + ;; Sending --password will make MySQL prompt for the + ;; password. + (list "--password")) (if (not (= 0 sql-port)) (list (concat "--port=" (number-to-string sql-port)))) (if (not (string= "" sql-server)) -- 2.19.0.605.g01d371f741-goog --=-=-= Content-Type: text/plain Stefan Kangas writes: > (Please keep the bug address in Cc.) > > Andrew Hyatt 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=" 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 --=-=-=--