From: "J.P." <jp@neverwas.me>
To: "Arsen Arsenović" <arsen@aarsen.me>
Cc: Damien Cassou <damien@cassou.me>, Eli Zaretskii <eliz@gnu.org>,
67937@debbugs.gnu.org
Subject: bug#67937: 30.0.50; auth-source-pass relies on epa-file being enabled
Date: Fri, 22 Dec 2023 06:27:45 -0800 [thread overview]
Message-ID: <87ttoas466.fsf@neverwas.me> (raw)
In-Reply-To: <87jzp6is0s.fsf@aarsen.me> ("Arsen Arsenović"'s message of "Fri, 22 Dec 2023 08:33:48 +0100")
Hi Arsen,
Arsen Arsenović <arsen@aarsen.me> writes:
> "J.P." <jp@neverwas.me> writes:
> [...snip...]
>>
>> I think what `epa-hook' does beyond `epg' is offer the option of
>> opting out of the latter by way of the `file-name-handler-alist'
>> interface. If that's unwise or unnecessary for some reason, we should
>> probably explain why, if only for posterity.
>
> I believe it is, because a pass entry is precisely a single OpenPGP
> encrypted file. All other pass-compatible tools and implementations
> already rely on that. If we want to allow the user to change that, we
> should do so by relying on the pass CLI tool, because that way other
> parts of their pass workflow allow for change.
>
> But, I don't think even that is needed, at least for now.
I see. If there's essentially only one way to go about accessing and
decrypting files in these pass trees, then perhaps this is more of a bug
fix than a feature?
>> And in doing so, we should maybe also account for behavior like that
>> of `epa-file-insert-file-contents', which lies in the default
>> `f-n-h-a' code path and appears to go out of its way to provide a
>> different user experience when it comes to error handling. If such
>> accommodations are unnecessary, perhaps we ought to be somewhat
>> explicit as to why.
>
> Indeed, stuff like this was what I was referring to. Thanks for naming
> the function that implements this, I went ahead and read it.
>
> I believe the entire file-exists-p check is unnecessary, as the file
> being read is "guaranteed" to exist, bar race conditions (which ought to
> be fixed via a slightly larger refactor, by having a-s-p functions
> accept either a buffer or an open file or something, then having its
> user open a file literally).
Hm, I guess `expand-file-name' doesn't actually check to see if the file
name it returns exists, so I think the subprocess will ultimately be fed
... --decrypt -- non-existent-file.gpg
as trailing args. But I suppose that's not a concern so long as the user
can readily deduce that some kind of easily fixable pilot error has
occurred.
> That leaves us with just:
>
> (if (setq entry (assoc file epa-file-passphrase-alist))
> (setcdr entry nil))
> ;; If the decryption program can't be found,
> ;; signal that as a non-file error
> ;; so that find-file-noselect-1 won't handle it.
> ;; Borrowed from jka-compr.el.
> (if (and (memq 'file-error (get (car error) 'error-conditions))
> (equal (cadr error) "Searching for program"))
> (error "Decryption program `%s' not found"
> (nth 3 error)))
>
> I believe the passphrase handling is also unnecessary, or at least not
> very likely to matter, as 'pass' files aren't intended to be
> symmetrically encrypted.
Makes sense. And I guess pass doesn't sign these files either, so
there's no risk of a "wrong password" failure from the agent or pinentry
ending up in that condition-case handler.
> That leaves us with handling the lack of a decryption program. Perhaps
> we should extract this into some common code (I'm surprised other users
> of EPG don't need it). Perhaps the status quo is good enough as it is?
> I have not tested that yet (need to run - sorry).
Again, I'd say as long as there's some way for these rare pilot errors
to reach the user, that's probably enough.
> Overall, I don't think involving the f-n-h-a mechanism is desirable to
> get one error path that could be obtained via refactoring when it ends
> up also dragging in all the possible complexity of f-n-h-a where it is
> not desirable (as pass entries are strictly defined).
Simplicity is a worthy goal, I think we can all agree.
>>> I'm not sure what you mean by 'hard-coding' here. These files are
>>> always gpg files (that's how pass works), and they are always OpenPGP
>>> encrypted. The usage of epg-decrypt-file is proposed by the help of
>>> epa-decrypt-region IIRC.
>>
>> I meant "hard-coding" in the sense that the original design seems to
>> allow external code to potentially influence the decryption process,
>> as mentioned above.
>
> I believe this is accidental. auth-source-pass authors would have to
> weigh in, though.
>
>> But from what you're saying, it sounds like there's no legitimate use
>> case for doing so. I wasn't privy to the original design discussions,
>> but it would be nice to know if there was a good reason for going this
>> route beyond adhering to the age-old best practice of relying on
>> interfaces rather than implementations.
>
> AFAIK, epg-decrypt-file is a public interface. epa-decrypt-region (not
> epa-decrypt-file, sorry, I misrecalled in my earlier message) even
> suggests using it:
>
> Be careful about using this command in Lisp programs!
[...]
Sorry, by "relying on interfaces", I basically meant adhering to the
"dependency inversion principle," at least insofar as `epa-hook' and
`a-s-p' both rely on `f-n-h-a'. But if there's only one way to skin this
particular cat, then perhaps that's all just unwanted complexity, as you
say.
>> Perhaps it's worth rifling through the archives for mention of the
>> authors' original motivations WRT `f-n-h-a', just for good measure?
>
> Probably, but my intuition tells me it was accidental. I'll try to find
> relevant messages (thankfully, there wasn't too much discussion on this
> topic, so that shouldn't be too hard to search :-) ).
>
>>>> 2. How likely is it that someone actually depends on the perceived
>>>> undesirable behavior currently on HEAD? Like, for example, could
>>>> someone out there conceivably have a cron-like script that runs
>>>> `epa-file-disable' before copying the encrypted secrets from the
>>>> result of an `auth-source-search' to Nextcloud or something? If these
>>>> weren't passwords, perhaps we could just shrug off such
>>>> hypotheticals, but... (just saying).
>
> I missed the latter part of this before, apologies.
>
> If such a user exists, and they use any auth-sources value besides
> '(password-source), their setup will already break, as this hack only
> works for password-source. In addition, due to auth-source caching,
> they'd need to flush the cache twice per such a backup (once to throw
> out the unencrypted password, and once to recover it). There are
> certainly more elegant solutions to getting the contents of those
> encrypted files.
I guess I was originally envisioning a headless --batch style situation
rather than an in-session background task, but regardless, what you say
about the cache makes sense in that more hurdles means more hassle,
which makes such a scenario all the more unlikely.
>>>
>>> I do not know, but this dependency is wrong either way, and can confuse
>>> users and the auth-source cache.
>>
>> So, it seems like we're saying that protecting an unlikely minority here
>> should not stand in the way of simplicity and consistency. I can't argue
>> against that, but I think it's important to be clear about the potential
>> consequences of such a sacrifice.
>
> Sure.
Don't kill me, but I have another rather unlikely scenario perhaps
worthy of passing consideration (or dismissal):
(setopt auth-source-pass-filename "/ssh:desktop.local:.password-store")
If those Tramp addresses don't continue to work after your suggested
changes, we should probably ask Michael Albinus whether their working
currently is just an accident or something intentional and supported.
>>> The only reason I noticed this is because *something* (and I have no
>>> idea what as of yet) somehow unhooks epa-file. When I noticed that, I
>>> figured I could use epa-file-disable to provide a simpler reproducer. I
>>> didn't actually notice the bug by using epa-file-disable (and I have
>>> never intentionally ran epa-file-disable).
>>>
>>> I'll be tracking that down next, but fixing this first seemed easier.
>>
>> Tracking that down would be nice, definitely.
>
> I will try to debug-on-variable-change f-h-n-a, but to do that I'll need
> to figure out a more effective approach than hitting 'c' repeatedly in
> the debugger (can debugs be conditional?), as that's getting tiring and
> imprecise.
Yeah, that sounds like a pain. If you haven't tried already, perhaps
hand rolling your own `add-variable-watcher' is worth a shot?
J.P.
next prev parent reply other threads:[~2023-12-22 14:27 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 16:57 bug#67937: 30.0.50; auth-source-pass relies on epa-file being enabled Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-20 18:26 ` Eli Zaretskii
2023-12-20 19:11 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-20 19:21 ` Eli Zaretskii
2023-12-20 19:58 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-21 9:45 ` Eli Zaretskii
2023-12-21 10:18 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-21 14:33 ` J.P.
2023-12-21 15:29 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-21 23:39 ` J.P.
2023-12-22 7:33 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-22 14:27 ` J.P. [this message]
2023-12-22 14:53 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-22 19:40 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-22 20:49 ` J.P.
2023-12-23 11:20 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-23 15:06 ` J.P.
2023-12-23 15:26 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-23 16:59 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-23 19:44 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 0:43 ` J.P.
2023-12-24 10:25 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 11:55 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 9:47 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 10:37 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 11:41 ` Eli Zaretskii
2023-12-24 12:00 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 15:00 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 16:11 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 17:26 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-29 8:27 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-29 9:38 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 12:00 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 12:14 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 15:03 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 16:31 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-23 15:50 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ttoas466.fsf@neverwas.me \
--to=jp@neverwas.me \
--cc=67937@debbugs.gnu.org \
--cc=arsen@aarsen.me \
--cc=damien@cassou.me \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).