bug#72441: 31.0.50; Auth-source-pass doesn't match password attributes or hosts without user when extra-query-keywords is true

["Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

"J.P." <jp@neverwas.me> writes:

> Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs@gnu.org> writes:
>
>> I noticed that auth-source-pass doesn't match agains password file
>> attributes such as those containing :user and only matches password
>> files which contain a host and a user when
>> auth-source-pass-extra-query-keywords is true.
>
> I don't use pass myself, nor have I ever, but I suppose I did add the
> `auth-source-pass-extra-query-keywords' option (though mainly in a bid
> to make auth-source-pass behave more like other back ends so it's usable
> with ERC). Anyway, I do actually recall being somewhat aware of the
> existence of the file attributes you mention. It seems I even left a
> comment about the current lack of support [1].

I started using the option since my girlfriends password store is
organized for many passwords as hostname.tld/user@exampl.com, when she
created a password that is <Topic e.g. SSO>/example.com/user where
without the option the wrong password files were used.
Her mail address is user@example.com and the login server is also
example.com with her name as the username (we selfhost).

> Looking into this a bit, it seems the password store's web page
> considers everything after the initial (password) line to be an opaque
> text blob:
>
>   The password store does not impose any particular schema or type of
>   organization of your data, as it is simply a flat text file, which can
>   contain arbitrary data. Though the most common case is storing a
>   single password per entry, some power users find they would like to
>   store more than just their password inside the password store, and
>   additionally store answers to secret questions, website URLs, and
>   other sensitive information or metadata. Since the password store does
>   not impose a scheme of it's own, you can choose your own organization.
>   There are many possibilities.
>
> However, I do realize that the auth-source-pass back end without the
> extra-keywords option already dips into a file's contents looking for an
> attributes list like the one shown on the web page. (Whether that's wise
> is pretty much moot after all these years.) Anyway, for that reason, I
> suppose we _should_ attempt to at least explore doing the same when the
> extra-keywords option is enabled. For me, the most important thing
> remains mimicking the behavior of the other built-in back ends, which at
> times is admittedly unintuitive but nevertheless consistent and thus
> predictable from a mechanical POV.

I agree fully with the comment. Other's that use pass as source for
passwords also use file contents to match or retrieve variables from.
E.g. most browser plugins derive the parameter for the login or user
name from either of these names.

>> Steps to reproduce:
>> 1. Setup pass with the following structure
>>    WorkingTest/example.com/foo
>>    FailingTest/example2.com
>>    FailingTest/example3.com with user: foo in the password file
>> 2. (auth-source-pass-enable)
>> 3. (setq auth-source-pass-extra-query-keywords t)
>> 4. (auth-source-search :host "example" :user "foo") -> works
>> 5. (auth-source-search :host "example2.com") -> fails
>> 6. (auth-source-search :host "example3.com" :user "foo")
>>
>> Auth-source-pass should be able to query the password file for
>> additional attributes if one of the previous attributes such as :host
>> match to it. Quering the file attributes is quite important in use
>> cases where it doesn't make sense to the user to have a
>> host-folder/user-file structure in cases where there's only one
>> password for said host.
>
> Currently, if you have a file in the root of your ~/.password-store
> named something like "top-level-host.com", and it's contents feature a
> "user: foo" attribute, both
>
>   (auth-source-search :host "top-level-host.com")
>
> and
>
>   (auth-source-search :host "top-level-host.com" :user "foo")
>
> return
>
>   ((:host "top-level-host.com" :secret ...)).
>
> If you're saying you want to see (:user "foo") in the results as well, I
> guess we can do that (see attached patch as well as [2], below).
> However, this still won't work on any of your examples, which all have
> intervening path components between the root directory and the .gpg
> files. The reason for this restriction is explained below.
>
> If we do end up going with something like the attached patch, we'll need
> to profile it. I can create a bunch of fake trees of varying shapes and
> sizes, but I'd rather someone with real data and a sizable store assess
> how much slower it is to visit (and thus decrypt) potentially every file
> in the tree, which is what any attr-reading implementation must do. On
> my machine, it takes roughly 0.18 seconds to decrypt a single two-line
> file via `auth-source-pass--read-entry'. (This seems prohibitively
> expensive to do en masse, no?) FWIW, most of this time is spent in
> `epg-wait-for-status', which blocks until the subprocess exits.

That is why I was arguing that we should attempt to not try decrypt the
password file unless a previous attribute such as :host or :user matched
before.
If we could do the search in parallel or at least without blocking Emacs
that would be a different story of course.

>> Same it should maybe also match against :host
>> if no user was provided, I don't know how other sources do this thou.
>
> While the reference implementation indeed succeeds with a plain :host
> input (see test `auth-source-pass-extra-query-keywords--netrc-host'), I
> believe the actual problem you perceive has more to do with the content
> of the file paths, specifically, leading directory components. Still,
> I'm inclined to agree that this would be nice to have. However, I do
> seem to recall this being discussed on at least one occasion, with the
> conclusion being that it's too complicated, if not impossible, to
> disambiguate between a trailing "hostname/user" and "folder/hostname".
>
> Nevertheless, we could add an option to do it anyway based on one or
> more heuristics-based strategy (resolving hosts for real is surely a no
> go). For example, one such strategy could ignore a penultimate file-path
> component that's not an FQDN, even if it's, say, LDH-only and resolvable
> as a hostname, so long as the leaf component _is_ an FQDN. However, such
> an option would have to be disabled by default to prevent existing
> entries like "localhost/test.user" from being parsed as (:host
> "test.user").
>

What do you mean by resolving hosts for real? I think another option
would be for the user to specific the hierarchy of their password store
to auth-source-pass e.g. word/%host%/%user or word/(or %host word)/%user
where word is any word that isn't used for matching but just for the
user to organize the hierarchy.

> In any case, I'm happy to review patches, but I think someone who
> actually uses this back end should implement the feature.

I'm not a good lisp programmar but I could give it a go with some help
such as your patch as a start point.

>
> [1] https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source-pass.el?id=423c86cb#n300
>
> [2] The following describes details of the attached patch's logic for
>     the inner (dolist (e entries) ...) loop of the primary matching
>     function `auth-source-pass--find-match-many'. Hopefully it's
>     somewhat sound with regard to deferring decryption for as long as
>     possible.
>
>     1. Parse the file path of each entry first and cache its results in
>        a plist, the "cached entry metadata," which is filed under the
>        entry's file-path in the `seen' hash table. If it doesn't match
>        the basic filename format, it must not be a passwordstore file,
>        so reject the entry.
>     2. Check the :host field before reading the file. Unless it matches,
>        reject the entry.
>     3. Engage in a series of probing conditional checks for a :port
>        field to match against a provided "port" query parameter, all
>        while attempting to defer decryption until absolutely necessary.
>        (A path-encoded :port always takes precedence over a :port in the
>        file.)
>        - If a `port' query parameter is not given for matching against,
>          continue to the next steps for the current entry.
>        - Otherwise, if a :port parsed from the file path is present and
>          it doesn't match, reject the entry, meaning go to the beginning
>          of the current loop, considering the next entry.
>        - If a path-derived :port is absent, ensure the cached entry
>          metadata contains an additional :attrs field (an alist). If the
>          metadata lacks an :attrs field, the file has not yet been
>          decrypted. Decrypt it now using `auth-source-pass-parse-entry',
>          then add its secret and its attrs alist to the cached metadata,
>          under :attrs.
>        - Look in the cached entry metadata's :attrs alist for a "port"
>          attr. If a "port" attr is indeed present and doesn't match the
>          port query parameter, reject the entry.
>        - If no such "port" attr exists and is required (meaning :port
>          appears in the `require' query parameter), reject the entry.
>     4. Repeat step 3 for :user. The same precedence rules apply, meaning
>        any non-null path-derived :user field is immediately accepted,
>        and the file is not decrypted.
>     5. If we haven't yet decrypted the file, do so now and populate the
>        :attrs item in the cached entry metadata. If it's already been
>        decrypted at some point, :attrs will present (though possibly
>        empty). In any case, add the items we care about if non-null
>        (:user, :port, and :secret) to the matched results for this entry.
>        However, only do so if a secret was either not required or is
>        present; otherwise, reject the entry.