From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Arsen =?UTF-8?Q?Arsenovi=C4=87?= via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#67937: 30.0.50; auth-source-pass relies on epa-file being enabled Date: Fri, 22 Dec 2023 15:53:02 +0100 Message-ID: <875y0qgtxq.fsf@aarsen.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> <87ttoas466.fsf@neverwas.me> Reply-To: Arsen =?UTF-8?Q?Arsenovi=C4=87?= Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13683"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Damien Cassou , Eli Zaretskii , 67937@debbugs.gnu.org To: "J.P." Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Dec 22 16:05:40 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 1rGh5n-0003Ek-7Q for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 22 Dec 2023 16:05:39 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rGh5H-0003Or-TY; Fri, 22 Dec 2023 10:05: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 1rGh5B-0003OI-OM for bug-gnu-emacs@gnu.org; Fri, 22 Dec 2023 10:05:01 -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 1rGh57-00067R-Pn for bug-gnu-emacs@gnu.org; Fri, 22 Dec 2023 10:05:00 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rGh5C-0006wq-B7 for bug-gnu-emacs@gnu.org; Fri, 22 Dec 2023 10:05:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Arsen =?UTF-8?Q?Arsenovi=C4=87?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 22 Dec 2023 15:05: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.170325747826649 (code B ref 67937); Fri, 22 Dec 2023 15:05:02 +0000 Original-Received: (at 67937) by debbugs.gnu.org; 22 Dec 2023 15:04:38 +0000 Original-Received: from localhost ([127.0.0.1]:47597 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rGh4n-0006vk-EX for submit@debbugs.gnu.org; Fri, 22 Dec 2023 10:04:38 -0500 Original-Received: from mout-p-101.mailbox.org ([80.241.56.151]:47128) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rGh4k-0006vT-5V for 67937@debbugs.gnu.org; Fri, 22 Dec 2023 10:04:36 -0500 Original-Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:b231:465::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4SxVv90d7Kz9sRX; Fri, 22 Dec 2023 16:04:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aarsen.me; s=MBO0001; t=1703257461; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VOOOusRGCAigfMEXTzbmzJRk0wb0+B/qeo1R+/WZfyc=; b=FgmHNheVmyENBbKdcd9WFe7Og7EVb+nLs8NF2vpBhk7AyW3R+QKkYBNsEpWE9pxGiLTeiy 7WmLFPUPGmJIR8tSmt6HWXcsIgP854SB/M7GiUbsNXZ2pFVXI3rYayxXPs38/3X/USPDdR a87F1lB5VHKMCifatXPXYIBB5OrGDzREJ5rAlkgcLWLyqOgpMCKxlOY2xLsfsUGAOvmqei XCwNeCi3DFk+E9IMks3MI4dg5STdh2HTOOsJPsdEyO6/QrKgGKNmLaB9OTweImD/ey5rAr 8/QpOtFk9h8x0fQHSKnPrvxsgwqX2tJxO671xVNaTPNWoKJjVXtwRW1g+UjzRw== In-reply-to: <87ttoas466.fsf@neverwas.me> X-Rspamd-Queue-Id: 4SxVv90d7Kz9sRX 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:276681 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi J.P, "J.P." writes: > 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? Yes, I consider this a bug and the patch a bug fix :-) >>> 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. Yes, but I think that the users of this function enumerate pass entries before calling it, and so "never" call it with a nonexistent file (though, that's perhaps not the case for non-a-s-p users.. unsure if this API is public and considered stable, but I suppose it is at least public since it's documented and lacks '--'?) >> 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. It does not, no. >> 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. They should propagate normally, AFAIK. >> 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. Right, that was my perspective. >>> 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 th= ese >>>>> 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. Ah, right. That could make more sense in a batch task, but I still doubt it for the same reason as before. >>>> >>>> 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. This is a worthy consideration. It is something a user could reasonably think to do. Suppose: (let ((ctx (epg-make-context 'OpenPGP))) (epg-decrypt-file ctx "/ssh:..." nil)) ... as you suspected, this does not work as gpg gets given the TRAMP locator. If the a-s-p authors think this is worthy of supporting, I will write an alternative patch that supports this via insert-file-literally (somewhat akin to the current code, but still explicit in decryption). I confirmed that insert-file-literally still supports TRAMP, so this is a viable path forward (but I can only do that in ~hour or so - need to do something now). >>>> 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? I figured to try that, but have been absent all day so I have not yet. I will do that ASAP, though. Thanks, have a lovely day. =2D- Arsen Arsenovi=C4=87 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iIYEARYKAC4WIQT+4rPRE/wAoxYtYGFSwpQwHqLEkwUCZYWlcRAcYXJzZW5AYWFy c2VuLm1lAAoJEFLClDAeosSTBDUA/04WvlZJbY0nG2GOY5/VK5ezfKFdYTJTLTCR SEH9zq42AP9pM1c/LTVJeGCnFsedCtXFYKsUb5Xn2zwPVIAUwBUdAw== =vBLA -----END PGP SIGNATURE----- --=-=-=--