From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Noam Postavsky Newsgroups: gmane.emacs.bugs Subject: bug#36052: 26.2.50; [PATCH] Improve auth-source-pass Date: Thu, 06 Jun 2019 20:43:06 -0400 Message-ID: <87a7eu2pk5.fsf@gmail.com> References: <87o93gjqrw.fsf@cassou.me> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="266756"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) Cc: Magnus Henoch , Nicolas Petton , Iku Iwasa , Keith Amidon , galaunay , 36052@debbugs.gnu.org, Ted Zlatanov To: Damien Cassou Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jun 07 02:44:34 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.47]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hZ2zh-0017Di-K7 for geb-bug-gnu-emacs@m.gmane.org; Fri, 07 Jun 2019 02:44:33 +0200 Original-Received: from localhost ([::1]:45328 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hZ2zg-0004Mb-Ja for geb-bug-gnu-emacs@m.gmane.org; Thu, 06 Jun 2019 20:44:32 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58116) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hZ2zH-0004Hb-Ex for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2019 20:44:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hZ2zF-0002GM-I0 for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2019 20:44:07 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:37379) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hZ2zF-0002Fh-70 for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2019 20:44:05 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hZ2zF-0008OE-4H for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2019 20:44:05 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Noam Postavsky Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 07 Jun 2019 00:44:05 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36052 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 36052-submit@debbugs.gnu.org id=B36052.155986819732149 (code B ref 36052); Fri, 07 Jun 2019 00:44:05 +0000 Original-Received: (at 36052) by debbugs.gnu.org; 7 Jun 2019 00:43:17 +0000 Original-Received: from localhost ([127.0.0.1]:50914 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hZ2yS-0008MT-Gu for submit@debbugs.gnu.org; Thu, 06 Jun 2019 20:43:16 -0400 Original-Received: from mail-it1-f178.google.com ([209.85.166.178]:35706) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hZ2yP-0008ME-Ix for 36052@debbugs.gnu.org; Thu, 06 Jun 2019 20:43:15 -0400 Original-Received: by mail-it1-f178.google.com with SMTP id n189so268234itd.0 for <36052@debbugs.gnu.org>; Thu, 06 Jun 2019 17:43:13 -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=syEFVNJl4IuZgDNfNev99a7aMkGz8o7ncoaP9TmbgCg=; b=Kgs2BxbXN3PJrCSkz700fdGmqU+DMN1Taka6Pjv8plHHr7MRDzKvsXV1DoQyumz88A ML6N+wxZlhpkEtbz44xXaP9Fm/B5OTybDJJvd2VJP3RDM8fyXn196NWz9UDULE3ZBIWG I1EUpnROZHSneqZ0taRyaVBA132qAkyXtEQnuFHQnKpqc0tHcX13idyCWGgweT9yXUhq nuDKAP31prVJOTDcJupidaR2eEEkfzq5W0OonnnGbGtFl+M7XbRN1pDmRtVuP8Msmhln +3szdsjwENo7b41FDWSsmxEsQHmLWOUbd1Au6yZEZ6y5ocpUg7k0qFANv98mw/RJ9slD TmuQ== 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=syEFVNJl4IuZgDNfNev99a7aMkGz8o7ncoaP9TmbgCg=; b=rf73MhkB2gn2fuTQm6PEMObIVjbCsq+9/HWkOhJztE4s6L+20EAH4Emi+02L+ZiqMz gRFKudjvCthA2BeMQr9oDarLeCl9cT1iZVRqweQ3xBw9xM/TQTPX6GUJSbrI1Yt2iMYF kIAgls+dgaQk0aClnjr78+bNiXlYz3COJa9tOyjbp74Sq887YS5xoN/bdd6wt75K1Nhv MXBWg3TzRrT9PEwfHn62KyvCm+lNEt/QHIiDXOIurSWFGh9KNPqoVP9z9wTlvCNPwymX VI6NHRCTRvDKzqu2yKG875a1273lY3Wiut9sg7OPXaKeXQ47MC23rUTvSOqHS4KpIJP/ /9kw== X-Gm-Message-State: APjAAAXRgQF6N3bSgHxEj54+rjRGb4VguORK5+ZTg/UfV1cB9GcUqr66 7nlMxgHOwf44+aNrL12G550= X-Google-Smtp-Source: APXvYqxD8Zln4Uo02I7axHeogX4ptjf9FA6NKgnTU+XVdM3HSZCQ1RmGh2D3R3OeF8wa3G2Au6ybWQ== X-Received: by 2002:a02:ca19:: with SMTP id i25mr23666709jak.6.1559868187726; Thu, 06 Jun 2019 17:43:07 -0700 (PDT) Original-Received: from minid (cbl-45-2-119-34.yyz.frontiernetworks.ca. [45.2.119.34]) by smtp.gmail.com with ESMTPSA id o206sm197283ita.19.2019.06.06.17.43.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 06 Jun 2019 17:43:07 -0700 (PDT) In-Reply-To: <87o93gjqrw.fsf@cassou.me> (Damien Cassou's message of "Sun, 02 Jun 2019 11:11:15 +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:160203 Archived-At: Damien Cassou writes: > Magnus Henoch hasn't signed but his contribution is rather small (4 > lines of a new unit-test and 2 lines of code). See patch 0001. His patch needs the line Copyright-paperwork-exempt: yes > Subject: [PATCH 04/12] Add auth-source-pass-path option I think auth-source-pass-filename would be the correct name to conform with GNU naming conventions. > Subject: [PATCH 07/12] Split out the attribute retrieval form > auth-source-pass-get > > Eliminate the need to repeatedly retrieve and parse the data for the > entry. This is generally a good thing since it eliminates repetitions > of the same crypto and parsing operations. It is especially valuable > when protecting an entry with a yubikey with touch required for crypto > operations as it eliminates the need to touch the yubikey sensor for > each attribute retrieved. Missing double spacing at end of sentence here. > +(defun auth-source-pass--get-attr (key entry-data) > + "Return the value associated to KEY in data from an already parsed entry. > + > +ENTRY-DATA is the data from a parsed password-store entry. > +The key used to retrieve the password is the symbol `secret'. > + > +The convention used as the format for a password-store file is > +the following (see http://www.passwordstore.org/#organization): > + > +secret > +key1: value1 > +key2: value2" I think the end of the docstring could be replaced with See `auth-source-pass-get'. It seems a little silly to duplicate this info so close by. > Subject: [PATCH 08/12] Minimize entry parsing in auth-source-pass > > Prior to this commit, while searching for the most applicable entry > password-store entries were decrypted and parsed to ensure they were > valid. The entries were parsed in the order they were found on the > filesystem and all applicable entries would be decrypted and parsed, > which varied based on the contents of the password-store and the entry > to be found. > > This is fine when the GPG key is cached and each entry can be > decrypted without user interaction. However, for security some people > have their GPG on a hardware token like a Yubikey setup so that they > have to touch a sensor on the toke for every cryptographic operation, > in which case it becomes inconvenient as each attempt to find an entry > requires a variable number of touches of the hardware token. > > The implementation already assumes that names which contain more of > the information in the search key should be preferred so there is an > ordering of preference of applicable entries. If the decrypt and > parsing is removed from the initial identification of applicable > entries in the store then in most cases a single decrypt and parse of > the most preferred entry will suffice, improving the experience for > hardware token users that require interaction with the token. > > This commit implements that strategy. It is in spirit a refactor of > the existing code. The core of the change is the function > auth-source-pass--applicable-entries, which generates an ordered list > of regular expression matchers for all possible names that could be in > the password-store for the entry to be found and then makes a pass > over the password-store entry names accumulating the matching entries > in a list after the regexp that matched. This implementation ensures > the password-store entry list still only has to be scanned once. > > The existing auth-source-pass--find-match-unambiguous was modified to > use this new function to obtain candidate entries and then parse them > one by one until an entry containing the desired information is > located. When complete it now returns the parsed data of the entry > instead of the entry name so that the information can be used directly > to construct the auth-source response. > > * lisp/auth-source-pass.el: Private functions were refactored to > reduce the number of decryption operations. Double spacing, and this ChangeLog entry is a little sparse. It looks like the last two prose paragraphs could be easily made into ChangeLog entries, since they're already talking about specific functions. > + (when (> (length name-components) 0) > + (cons (mapconcat 'identity name-components ".") > + (auth-source-pass--domains (cdr name-components))))) I suggest instead: (cl-maplist (lambda (components) (mapconcat #'identity components ".")) name-components) > Subject: [PATCH 10/12] Refactoring of auth-source-pass > > * lisp/auth-source-pass.el: Refactoring. This one's a little empty too. > Subject: [PATCH 12/12] * etc/NEWS: Describe changes to auth-source-pass > > --- > etc/NEWS | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/etc/NEWS b/etc/NEWS > index 975fab495a..5dcdac2668 100644 > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -1485,6 +1485,25 @@ the new variable 'buffer-auto-revert-by-notification' to a non-nil > value. Auto Revert mode can use this information to avoid polling the > buffer periodically when 'auto-revert-avoid-polling' is non-nil. > > +** auth-source-pass > + > +*** New customizable variable 'auth-source-pass-path' for the path to > +the password-store. This defaults to ~/.password-store. It's better to have NEWS entries have the first sentence in one line. Something like *** New customizable variable 'auth-source-pass-path'. Allows setting the path to the password-store, defaults to ~/.password-store. > +*** New customizable variable 'auth-source-pass-port-separator' to > +specify separator between host and port. This defaults to colon > +":". *** New customizable variable 'auth-source-pass-port-separator'. Specifies separator between host and port, defaults to colon ":". > +*** auth-source-pass.el and auth-source-pass-tests.el have been > +massively rewritten to minimize parsing of password-store entries. > +This makes the package usable with physical tokens requiring touching > +a sensor for every decryption. This one puts too much emphasis on the rewrite which is an implementation detail. *** Minimize the number of decryptions during password lookup. This makes the package usable with physical tokens requiring touching a sensor for every decryption. > +*** 'auth-source-pass-get' has an autoload cookie now. Maybe just say "is now autoloaded". > +*** 'auth-source-pass-search' now correctly returns nil if no entry > +found. We don't put bug fixes in NEWS, so this one can be left out.