Package: emacs;
Reported by: Björn Bidar <bjorn.bidar <at> thaodan.de>
Date: Sat, 3 Aug 2024 11:14:01 UTC
Severity: normal
Found in version 31.0.50
Message #8 received at 72441 <at> debbugs.gnu.org (full text, mbox):
From: "J.P." <jp <at> neverwas.me> To: 72441 <at> debbugs.gnu.org Cc: Björn Bidar <bjorn.bidar <at> thaodan.de> Subject: Re: bug#72441: 31.0.50; Auth-source-pass doesn't match password attributes or hosts without user when extra-query-keywords is true Date: Fri, 09 Aug 2024 11:02:25 -0700
[Message part 1 (text/plain, inline)]
Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> 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].
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.
> 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.
> 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").
In any case, I'm happy to review patches, but I think someone who
actually uses this back end should implement the feature.
[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.
[0001-POC-Match-attrs-with-auth-source-pass-extra-query-ke.patch (text/x-patch, attachment)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.