From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "J.P." Newsgroups: gmane.emacs.bugs Subject: bug#67937: 30.0.50; auth-source-pass relies on epa-file being enabled Date: Fri, 22 Dec 2023 06:27:45 -0800 Message-ID: <87ttoas466.fsf@neverwas.me> References: <8734vwq06i.fsf@aarsen.me> <83frzwhgre.fsf@gnu.org> <87jzp8of97.fsf@aarsen.me> <83bkakhe8s.fsf@gnu.org> <87msu4myau.fsf@aarsen.me> <83y1dnga7u.fsf@gnu.org> <87sf3vlqj1.fsf@aarsen.me> <871qbf4ocp.fsf@neverwas.me> <871qbflg53.fsf@aarsen.me> <87h6kbxgzl.fsf@neverwas.me> <87jzp6is0s.fsf@aarsen.me> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16836"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Damien Cassou , Eli Zaretskii , 67937@debbugs.gnu.org To: Arsen =?UTF-8?Q?Arsenovi=C4=87?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Dec 22 15:28:39 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rGgVz-0004A1-5U for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 22 Dec 2023 15:28:39 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rGgVT-00081P-SN; Fri, 22 Dec 2023 09:28:07 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rGgVR-0007xt-RR for bug-gnu-emacs@gnu.org; Fri, 22 Dec 2023 09:28:05 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rGgVK-0003nf-DS for bug-gnu-emacs@gnu.org; Fri, 22 Dec 2023 09:28:05 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rGgVO-0005zW-VF for bug-gnu-emacs@gnu.org; Fri, 22 Dec 2023 09:28:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: "J.P." Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 22 Dec 2023 14:28:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 67937 X-GNU-PR-Package: emacs Original-Received: via spool by 67937-submit@debbugs.gnu.org id=B67937.170325528223022 (code B ref 67937); Fri, 22 Dec 2023 14:28:02 +0000 Original-Received: (at 67937) by debbugs.gnu.org; 22 Dec 2023 14:28:02 +0000 Original-Received: from localhost ([127.0.0.1]:46257 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rGgVN-0005zA-4Y for submit@debbugs.gnu.org; Fri, 22 Dec 2023 09:28:01 -0500 Original-Received: from mail-108-mta165.mxroute.com ([136.175.108.165]:36849) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rGgVK-0005z0-8I for 67937@debbugs.gnu.org; Fri, 22 Dec 2023 09:27:59 -0500 Original-Received: from filter006.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta165.mxroute.com (ZoneMTA) with ESMTPSA id 18c91ecdeb90003b4e.003 for <67937@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Fri, 22 Dec 2023 14:27:49 +0000 X-Zone-Loop: 3274dcdca409a6e32ee44c571782ec96477da266e0f0 X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Transfer-Encoding:Content-Type:MIME-Version:Message-ID:Date: References:In-Reply-To:Subject:Cc:To:From:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=aJCClRJRqlEYTfb8hWWOD0ZCvo1AzFzBaQqhNr5FI6E=; b=JvAD03sxgx3CK9hTOlWPtfz4MD CqUWanCMiNBmQbylkfEVu9WrpTjpNhrJnkSHlLR9iOy4enXeq2CMTi5VgFeapnXOHr5wapJrDB7Du rSNMZZjrecsdXGQIXaD5jgLG7r3loPpuUH0svUPJv6SBOTiSgRLzIrxeHFsn0o7Mdt4Iht9+FGUCV mscavh67XNToOK3+Kx1RETVaP8dVb/pocQcHRLooaqQMBTp+QUWdEGIkfUydSAu5RTj/f3Vzb5gRi xItlHn/f6J6h3mviCxZJBhuQapvqKM8DuPaGE/AcLzGnLW+XWczfjds+cAtycx0Tgdb9B9joC8tiE q70UfdlQ==; In-Reply-To: <87jzp6is0s.fsf@aarsen.me> ("Arsen =?UTF-8?Q?Arsenovi=C4=87?="'s message of "Fri, 22 Dec 2023 08:33:48 +0100") X-Authenticated-Id: masked@neverwas.me X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:276666 Archived-At: Hi Arsen, Arsen Arsenovi=C4=87 writes: > "J.P." 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 the= se >>>> 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.