[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#36052: 26.2.50; [PATCH] Improve auth-source-pass
From: |
Noam Postavsky |
Subject: |
bug#36052: 26.2.50; [PATCH] Improve auth-source-pass |
Date: |
Thu, 06 Jun 2019 20:43:06 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Damien Cassou <damien@cassou.me> 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.
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Damien Cassou, 2019/06/02
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass,
Noam Postavsky <=
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Damien Cassou, 2019/06/08
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Eli Zaretskii, 2019/06/08
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Noam Postavsky, 2019/06/08
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Damien Cassou, 2019/06/13
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Noam Postavsky, 2019/06/13
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Damien Cassou, 2019/06/14
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Eli Zaretskii, 2019/06/14
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Damien Cassou, 2019/06/14
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Eli Zaretskii, 2019/06/22
- bug#36052: 26.2.50; [PATCH] Improve auth-source-pass, Damien Cassou, 2019/06/24