bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

X-debbugs-cc: michael.albinus@gmx.de

Hello,

In NEWS we have

    +++
    *** Direct asynchronous processes are indicated by a connection-local variable.
    If direct asynchronous processes shall be used, set the connection-local
    variable 'tramp-direct-async-process' to a non-nil value.  This has been
    changed, in previous Emacs versions this was indicated by the now
    deprecated connection property "direct-async-process".  See the Tramp
    manual "(tramp) Improving performance of asynchronous remote processes".

In my init.el I have this:

    (with-eval-after-load 'tramp
      (add-to-list 'tramp-connection-properties
                   '("/ssh:" "direct-async-process" t))
      (add-to-list 'tramp-connection-properties '(nil "session-timeout" nil)))

I have some documentation issues:

(1) The NEWS entry, together with the Info manual, were not sufficient
    for me to figure how to migrate my config.  For example, is the
    whole of tramp-connection-properties being replaced?  Or do I only
    need to migrate the direct-async-process part?

    I think additional documentation, even an explicit migration guide,
    would be desirable.

(2) Turning on direct async processes for all SSH connections seems like
    a common configuration.  How about adding that to the manual, in
    addition to the example for enabling them for one host?

(3) Has there been a change to how password caching works?

    I tried deleting the session-timeout configuration.  I made a
    /sudo:: connection.  After some time passed, I saw the familiar
    "Tramp connection timed out" message.  But it reconnected
    immediately when I tried to save a buffer I had open.  Does it now
    cache the sudo password?  In the past, you had to type it again,
    after the timeout.

    If there is a change here, it's security-relevant, so it should
    probably be in NEWS.

Thanks.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

thank you for your report.

> In NEWS we have
>
>     +++
>     *** Direct asynchronous processes are indicated by a connection-local variable.
>     If direct asynchronous processes shall be used, set the connection-local
>     variable 'tramp-direct-async-process' to a non-nil value.  This has been
>     changed, in previous Emacs versions this was indicated by the now
>     deprecated connection property "direct-async-process".  See the Tramp
>     manual "(tramp) Improving performance of asynchronous remote processes".
>
> In my init.el I have this:
>
>     (with-eval-after-load 'tramp
>       (add-to-list 'tramp-connection-properties
>                    '("/ssh:" "direct-async-process" t))
>       (add-to-list 'tramp-connection-properties '(nil "session-timeout" nil)))
>
> I have some documentation issues:
>
> (1) The NEWS entry, together with the Info manual, were not sufficient
>     for me to figure how to migrate my config.  For example, is the
>     whole of tramp-connection-properties being replaced?  Or do I only
>     need to migrate the direct-async-process part?

No, the whole tramp-connection-properties machinery is not replaced,
otherwise it would have been mentioned in NEWS. Furthermore, it is still
explained in the Tramp info manual of Emacs 30, which could give you an
indication that it still exists.

>     I think additional documentation, even an explicit migration guide,
>     would be desirable.

What you need to set is described in the Tramp manual. That's why the
quoted NEWS entry says

--8<---------------cut here---------------start------------->8---
See the Tramp manual "(tramp) Improving performance of asynchronous
remote processes".
--8<---------------cut here---------------end--------------->8---

That doesn't need a migration guide.

> (2) Turning on direct async processes for all SSH connections seems like
>     a common configuration.  How about adding that to the manual, in
>     addition to the example for enabling them for one host?

I don't know whether it is common praxis. I, for example, access my NAS
boxes (QNAP) via ssh w/o direct async processes. Simply, because they
are special, and do not cooperate well with direct async processes.

Furthermore, there are also disadvantages using direct async processes,
which are listed in the Tramp manual (same node as above). This prevents
me from saying, all connections using "ssh" shall use direct async
processes. And what's about "scp"? You can also use direct async
processes for such connections. Would you like to recommend this as well
in general? Where do you want to stop? Recommend "rsync"?

Sorry, but the conditions are more complex than you expect. People shall
find out what's best suited for them.

> (3) Has there been a change to how password caching works?
>
>     I tried deleting the session-timeout configuration.  I made a
>     /sudo:: connection.  After some time passed, I saw the familiar
>     "Tramp connection timed out" message.  But it reconnected
>     immediately when I tried to save a buffer I had open.  Does it now
>     cache the sudo password?  In the past, you had to type it again,
>     after the timeout.
>
>     If there is a change here, it's security-relevant, so it should
>     probably be in NEWS.

Likely, you're trapped by the following change, documented in NEWS.29:

--8<---------------cut here---------------start------------->8---
*** Proper password prompts for methods "doas", "sudo" and "sudoedit".
The password prompts for these methods reflect now the credentials of
the user requesting such a connection, and not of the user who is the
target.  This has always been needed, just the password prompt and the
related 'auth-sources' entry were wrong.
--8<---------------cut here---------------end--------------->8---

If it is something else, we need to debug.

> Thanks.

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Wed 30 Oct 2024 at 12:54pm +01, Michael Albinus wrote:

> No, the whole tramp-connection-properties machinery is not replaced,
> otherwise it would have been mentioned in NEWS. Furthermore, it is still
> explained in the Tramp info manual of Emacs 30, which could give you an
> indication that it still exists.

How about explicitly saying that it's not deprecated?

NEWS says that the connection property is deprecated, and the manual
says that the old way will eventually stop working.

In the presence of the new connection local variables, it is very
natural to mistakenly think that connection properties as a whole are
going away.

So how about just

    In previous Emacs versions this was indicated by the
    connection property "direct-async-process".  That connection
    property (though not connection properties and
    `tramp-connection-properties' in general) is now deprecated.

> I don't know whether it is common praxis. I, for example, access my NAS
> boxes (QNAP) via ssh w/o direct async processes. Simply, because they
> are special, and do not cooperate well with direct async processes.

That's fair enough, it's easy to assume one's own practices are common.

> Furthermore, there are also disadvantages using direct async processes,
> which are listed in the Tramp manual (same node as above). This prevents
> me from saying, all connections using "ssh" shall use direct async
> processes. And what's about "scp"? You can also use direct async
> processes for such connections. Would you like to recommend this as well
> in general? Where do you want to stop? Recommend "rsync"?
>
> Sorry, but the conditions are more complex than you expect. People shall
> find out what's best suited for them.

I read the list of disadvantages, and I think most of them don't apply
to the majority of users.  I didn't know about it for SCP and rsync but
now intend to enable it :)

I still think it would be a good idea to give an answer to "just turn
this on for everything, please" in the manual.

>> (3) Has there been a change to how password caching works?
>>
>>     I tried deleting the session-timeout configuration.  I made a
>>     /sudo:: connection.  After some time passed, I saw the familiar
>>     "Tramp connection timed out" message.  But it reconnected
>>     immediately when I tried to save a buffer I had open.  Does it now
>>     cache the sudo password?  In the past, you had to type it again,
>>     after the timeout.
>>
>>     If there is a change here, it's security-relevant, so it should
>>     probably be in NEWS.
>
> Likely, you're trapped by the following change, documented in NEWS.29:
>
> --8<---------------cut here---------------start------------->8---
> *** Proper password prompts for methods "doas", "sudo" and "sudoedit".
> The password prompts for these methods reflect now the credentials of
> the user requesting such a connection, and not of the user who is the
> target.  This has always been needed, just the password prompt and the
> related 'auth-sources' entry were wrong.
> --8<---------------cut here---------------end--------------->8---
>
> If it is something else, we need to debug.

Hmm, isn't this NEWS.29 change just a change in the password prompt?  I
don't see how that applies to what I described.

Unless it caches the sudo password, it shouldn't have been able to
reconnect.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

> So how about just
>
>     In previous Emacs versions this was indicated by the
>     connection property "direct-async-process".  That connection
>     property (though not connection properties and
>     `tramp-connection-properties' in general) is now deprecated.

Thanks. I've pushed your rephrased version to the emacs-30 repo.

> I still think it would be a good idea to give an answer to "just turn
> this on for everything, please" in the manual.

I've added the following to the Tramp manual (after the example with
'remotehost'):

--8<---------------cut here---------------start------------->8---
   This enables direct async processes for the host ‘remotehost’.  If
you want to enable direct async processes for all remote hosts
connected via the same method (e.g., ‘ssh’), use instead

     (connection-local-set-profiles
      '(:application tramp :protocol "ssh")
      'remote-direct-async-process)
--8<---------------cut here---------------end--------------->8---

Pushed to emacs-30.

>>> (3) Has there been a change to how password caching works?
>>>
>>>     I tried deleting the session-timeout configuration.  I made a
>>>     /sudo:: connection.  After some time passed, I saw the familiar
>>>     "Tramp connection timed out" message.  But it reconnected
>>>     immediately when I tried to save a buffer I had open.  Does it now
>>>     cache the sudo password?  In the past, you had to type it again,
>>>     after the timeout.
>>>
>>>     If there is a change here, it's security-relevant, so it should
>>>     probably be in NEWS.
>>
>> Likely, you're trapped by the following change, documented in NEWS.29:
>>
>> --8<---------------cut here---------------start------------->8---
>> *** Proper password prompts for methods "doas", "sudo" and "sudoedit".
>> The password prompts for these methods reflect now the credentials of
>> the user requesting such a connection, and not of the user who is the
>> target.  This has always been needed, just the password prompt and the
>> related 'auth-sources' entry were wrong.
>> --8<---------------cut here---------------end--------------->8---
>>
>> If it is something else, we need to debug.
>
> Hmm, isn't this NEWS.29 change just a change in the password prompt?  I
> don't see how that applies to what I described.
>
> Unless it caches the sudo password, it shouldn't have been able to
> reconnect.

No, there is also a change in the user name in .authinfo and
friends. The Tramp manual says:

--8<---------------cut here---------------start------------->8---
   For the methods ‘doas’, ‘sudo’, ‘sudoedit’ and ‘nspawn’ the
password of the user requesting the connection is needed, and not the
password of the target user(1).  If these connections happen on the
local host, an entry with the local user and local host is used:

     machine HOST port sudo login USER password secret

   USER and HOST are the strings returned by ‘(user-login-name)’ and
‘(system-name)’.  If one of these methods is connected via a multi-hop
(*note Multi-hops::), the credentials of the previous hop are used.
--8<---------------cut here---------------end--------------->8---

Before Emacs 29, you needed "login root" instead. This change was
mentioned in the etc/NEWS file indirectly, by saying "related
'auth-sources' entry":

--8<---------------cut here---------------start------------->8---
This has always been needed, just the password prompt and the
related 'auth-sources' entry were wrong.
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Sun 03 Nov 2024 at 04:01pm +01, Michael Albinus wrote:

> [...]
> Pushed to emacs-30.

Nice, thanks for your consideration.

> No, there is also a change in the user name in .authinfo and
> friends.

I don't use any of those files.

I don't think the behaviour I described is covered by this.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Dmitry Gutov]

On 03/11/2024 09:15, Sean Whitton wrote:
> I read the list of disadvantages, and I think most of them don't apply
> to the majority of users.  I didn't know about it for SCP and rsync but
> now intend to enable it 🙂

"cannot be killed via ‘interrupt-process’" might bite at some occasions.

No support for interactive user authentication could stump some users 
too (do we have a meaningful error message when that happens?)

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Mon 04 Nov 2024 at 02:41am +02, Dmitry Gutov wrote:

> On 03/11/2024 09:15, Sean Whitton wrote:
>> I read the list of disadvantages, and I think most of them don't apply
>> to the majority of users.  I didn't know about it for SCP and rsync but
>> now intend to enable it 🙂
>
> "cannot be killed via ‘interrupt-process’" might bite at some occasions.

Good point.

> No support for interactive user authentication could stump some users
> too (do we have a meaningful error message when that happens?)

I guess I assume the majority use SSH keys, and Emacs-external
interactive authentication for those definitely still works.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

> On Mon 04 Nov 2024 at 02:41am +02, Dmitry Gutov wrote:

>> No support for interactive user authentication could stump some users
>> too (do we have a meaningful error message when that happens?)
>
> I guess I assume the majority use SSH keys, and Emacs-external
> interactive authentication for those definitely still works.

But if you have protected your ssh keys by a passphrase, you'll run into
the same problem. Unless you run ssh-agent in the background. I, for
example, don't run it.

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

>> No, there is also a change in the user name in .authinfo and
>> friends.
>
> I don't use any of those files.
>
> I don't think the behaviour I described is covered by this.

Hmm. Does it mean you have set auth-sources to nil? There is a possible
error indeed in tramp-read-passwd for this case. Could you, please,
apply the appended patch?

Furthermore, Tramp caches the passwords indeed. If auth-sources is not
enabled, it uses the function password-read from
password-cache.el. However, the password is cached just for
password-cache-expiry seconds (16 by default). If you want to disable
this, set password-cache to nil. See the Tramp manual, at the bottom of
(info "(tramp) Password handling")

I have no idea where the changed behavior you've described comes from.

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Mon 04 Nov 2024 at 06:32pm +01, Michael Albinus wrote:

> Sean Whitton <spwhitton@spwhitton.name> writes:
>
>> Hello,
>
> Hi Sean,
>
>>> No, there is also a change in the user name in .authinfo and
>>> friends.
>>
>> I don't use any of those files.
>>
>> I don't think the behaviour I described is covered by this.
>
> Hmm. Does it mean you have set auth-sources to nil?

No, I haven't done any particular setup, I just don't (intentionally)
use any password-saving features in Emacs.

> There is a possible error indeed in tramp-read-passwd for this
> case. Could you, please, apply the appended patch?

Just to confirm, do you still want me to do this even though I don't set
auth-sources to nil?

> I have no idea where the changed behavior you've described comes from.

Okay -- I'll investigate further, including from 'emacs -q'.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

>> Hmm. Does it mean you have set auth-sources to nil?
>
> No, I haven't done any particular setup, I just don't (intentionally)
> use any password-saving features in Emacs.
>
>> There is a possible error indeed in tramp-read-passwd for this
>> case. Could you, please, apply the appended patch?
>
> Just to confirm, do you still want me to do this even though I don't set
> auth-sources to nil?

No, not needed for you. But since this case is missing in
tramp-read-passwd, I'll apply this patch anyway.

auth-sources has the initial value '("~/.authinfo" "~/.authinfo.gpg" "~/.netrc")
Do you own one of these files?

>> I have no idea where the changed behavior you've described comes from.
>
> Okay -- I'll investigate further, including from 'emacs -q'.

It might be helpful to step through tramp-read-passwd with the debugger.

Another idea: if auth-sources doesn't cooperate due to lack of useful
data, Tramp uses password-cache.el as fallback. password-cache-expiry
has the default value of 16 seconds. Could this explain what you see?

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Thu 07 Nov 2024 at 09:23am +01, Michael Albinus wrote:

> auth-sources has the initial value '("~/.authinfo" "~/.authinfo.gpg" "~/.netrc")
> Do you own one of these files?

No, I don't have any of them.

>>> I have no idea where the changed behavior you've described comes from.
>>
>> Okay -- I'll investigate further, including from 'emacs -q'.
>
> It might be helpful to step through tramp-read-passwd with the debugger.
>
> Another idea: if auth-sources doesn't cooperate due to lack of useful
> data, Tramp uses password-cache.el as fallback. password-cache-expiry
> has the default value of 16 seconds. Could this explain what you see?

Here are steps to reproduce in 'emacs -q' on recent master:

1. Ensure sudo is set up to require a password, i.e., not configured
   with NOPASSWD for the current user.
2. emacs -q
3. Evaluate (with-eval-after-load 'tramp
  (add-to-list 'tramp-connection-properties '(nil "session-timeout" 8)))
4. Evaluate (with-eval-after-load 'password-cache
              (setq password-cache-expiry 8))
4. C-x C-f /sudo::/root/foo RET
5. Make some edits, C-x C-s.
6. Wait 10 seconds.
7. Make some more edits, C-x C-s.
   - Expected result: prompted for sudo password again.
   - Actual result: edits are saved, with no password prompts.

Where could the password be getting cached?

Thanks.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

> Here are steps to reproduce in 'emacs -q' on recent master:
>
> 1. Ensure sudo is set up to require a password, i.e., not configured
>    with NOPASSWD for the current user.
> 2. emacs -q
> 3. Evaluate (with-eval-after-load 'tramp
>   (add-to-list 'tramp-connection-properties '(nil "session-timeout" 8)))
> 4. Evaluate (with-eval-after-load 'password-cache
>               (setq password-cache-expiry 8))
> 4. C-x C-f /sudo::/root/foo RET
> 5. Make some edits, C-x C-s.
> 6. Wait 10 seconds.
> 7. Make some more edits, C-x C-s.
>    - Expected result: prompted for sudo password again.
>    - Actual result: edits are saved, with no password prompts.

Thanks for the recipe! It helped me to track down the problem.

> Where could the password be getting cached?

It turns out, that auth.el has its own idea of password cache. It
overwrites password-cache-expiry with its own option,
auth-source-cache-expiry.

If you set *that* option to 8, it should work as expected.

I've modified the Tramp manual accordingly. It now says

--8<---------------cut here---------------start------------->8---
4.14.2 Caching passwords
------------------------

TRAMP can cache passwords as entered and reuse when needed for the
same user or host name independent of the access method.

   ‘auth-source-cache-expiry’(2) sets the duration (in seconds) the
passwords are remembered.  Set ‘auth-source-cache-expiry’ to ‘nil’ to
disable expiration.

   Cached passwords are never saved permanently nor can they extend
beyond the lifetime of the current Emacs session unless you confirm
this interactively.

   Set ‘auth-source-do-cache’ to ‘nil’ to disable password caching.

   ---------- Footnotes ----------

   (2) It overrides ‘password-cache-expiry’.
--8<---------------cut here---------------end--------------->8---

Pushed to the emacs-30 branch of the Emacs git repo.

> Thanks.

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Sun 10 Nov 2024 at 01:50pm +01, Michael Albinus wrote:

> Thanks for the recipe! It helped me to track down the problem.
>
>> Where could the password be getting cached?
>
> It turns out, that auth.el has its own idea of password cache. It
> overwrites password-cache-expiry with its own option,
> auth-source-cache-expiry.
>
> If you set *that* option to 8, it should work as expected.
>
> I've modified the Tramp manual accordingly.

Thanks for the documentation update.  I think, though, that this is a
backwards-incompatible change (or even a regression, depending on your
point of view): in older Emacs, the password was not cached at all, by
default.  So, I think we need a NEWS entry as well.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

> Thanks for the documentation update.  I think, though, that this is a
> backwards-incompatible change (or even a regression, depending on your
> point of view): in older Emacs, the password was not cached at all, by
> default.  So, I think we need a NEWS entry as well.

I have difficulties to reproduce the problem with older Emacsen exactly
as you have described. Could you pls send me the value of auth-sources
on your Emacs as well as the output of the shell command

--8<---------------cut here---------------start------------->8---
# ls -al ~/.authinfo* ~/.netrc* xxx
--8<---------------cut here---------------end--------------->8---

I would like to understand in detail, why the behavior has been changed
for you.

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]

Michael Albinus via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Sean,

>> Thanks for the documentation update.  I think, though, that this is a
>> backwards-incompatible change (or even a regression, depending on your
>> point of view): in older Emacs, the password was not cached at all, by
>> default.  So, I think we need a NEWS entry as well.
>
> I have difficulties to reproduce the problem with older Emacsen exactly
> as you have described. Could you pls send me the value of auth-sources
> on your Emacs as well as the output of the shell command
>
> # ls -al ~/.authinfo* ~/.netrc* xxx
>
> I would like to understand in detail, why the behavior has been changed
> for you.

I've digged further. Surprisingly, Tramp has a feature that, when a
session times out, the corresponding password cache entry is removed. So
there shouldn't be a problem at all, right?

I wrote this years ago. But since I don't need it myself, I've forgotten
about.

However, this feature has errors in implementation. Most important, when
the user, for whom the password has to be cached, was changed back in
Emacs 29, this mechanism wasn't adapted properly.

I rewrote tramp-read-passwd and tramp-clear-passwd, see the appended a
patch. I've tested the change with all Emacsen from Emacs 27 .. 31,
seems to work.

The patch is written for Tramp 2.7.1.4 from GNU ELPA. However, it seems
to apply also on the Emacs 30 sources (where it wouldn't go due to the
pretest). Would you like to test?

Since this feature has been brought back to work, you don't need to care
about auth-source-cache-expiry or password-cache-expiry. Simply set the
connection property "session-timeout" to 8, as you do in your test, and
it shall work.

I'll continue to work on the patch. Next step, I want to add a test to
tramp-tests.el, in order to not break it again silently.

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 5461 bytes --]

diff --git a/lisp/tramp.el b/lisp/tramp.el
index b149b3b9..4303d25b 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -5646,7 +5646,11 @@ of."
       ;; Sometimes, the process returns a new password request
       ;; immediately after rejecting the previous (wrong) one.
       (unless (or tramp-password-prompt-not-unique
-		  (tramp-get-connection-property vec "first-password-request"))
+		  (tramp-get-connection-property
+		   (tramp-get-connection-property
+		    proc "password-vector"
+		    (process-get proc 'tramp-vector))
+		   "first-password-request"))
 	(tramp-clear-passwd vec))
       (goto-char (point-min))
       (tramp-check-for-regexp proc tramp-process-action-regexp)
@@ -6797,13 +6801,13 @@ Consults the auth-source package."
 	 (case-fold-search t)
 	 ;; In tramp-sh.el, we must use "password-vector" due to
 	 ;; multi-hop.
-	 (vec (tramp-get-connection-property
-	       proc "password-vector" (process-get proc 'tramp-vector)))
-	 (key (tramp-make-tramp-file-name vec 'noloc))
-	 (method (tramp-file-name-method vec))
-	 (user-domain (or (tramp-file-name-user-domain vec)
+	 (vec (process-get proc 'tramp-vector))
+	 (pw-vec (tramp-get-connection-property proc "password-vector" vec))
+	 (key (tramp-make-tramp-file-name pw-vec 'noloc))
+	 (method (tramp-file-name-method pw-vec))
+	 (user-domain (or (tramp-file-name-user-domain pw-vec)
 			  (tramp-get-connection-property key "login-as")))
-	 (host-port (tramp-file-name-host-port vec))
+	 (host-port (tramp-file-name-host-port pw-vec))
 	 (pw-prompt
 	  (string-trim-left
 	   (or prompt
@@ -6812,29 +6816,23 @@ Consults the auth-source package."
 		 (if (string-match-p "passphrase" (match-string 1))
 		     (match-string 0)
 		   (format "%s for %s " (capitalize (match-string 1)) key))))))
+	 ;; If there is no user name, `:create' triggers to ask for.
+	 ;; We suppress it.
+	 (pw-spec (list :max 1 :user user-domain :host host-port :port method
+			:require (cons :secret (and user-domain '(:user)))
+			:create (and user-domain t)))
 	 (auth-source-creation-prompts `((secret . ,pw-prompt)))
 	 ;; Use connection-local value.
 	 (auth-sources (buffer-local-value 'auth-sources (process-buffer proc)))
 	 auth-info auth-passwd tramp-dont-suspend-timers)

     (unwind-protect
-	;; We cannot use `with-parsed-tramp-file-name', because it
-	;; expands the file name.
 	(or
 	 (setq tramp-password-save-function nil)
-	 ;; See if auth-sources contains something useful.
+	 ;; See if `auth-sources' contains something useful.
 	 (ignore-errors
-	   (and auth-sources
-                (tramp-get-connection-property vec "first-password-request")
-		;; Try with Tramp's current method.  If there is no
-		;; user name, `:create' triggers to ask for.  We
-		;; suppress it.
-		(setq auth-info
-		      (car
-		       (auth-source-search
-			:max 1 :user user-domain :host host-port :port method
-			:require (cons :secret (and user-domain '(:user)))
-			:create (and user-domain t)))
+	   (and (tramp-get-connection-property pw-vec "first-password-request")
+		(setq auth-info (car (apply #'auth-source-search pw-spec))
 		      tramp-password-save-function
 		      (plist-get auth-info :save-function)
 		      auth-passwd
@@ -6842,16 +6840,23 @@ Consults the auth-source package."

 	 ;; Try the password cache.
 	 (with-tramp-suspended-timers
-	   (setq auth-passwd (password-read pw-prompt key)
+	   (setq auth-passwd
+		 (password-read
+		  pw-prompt (auth-source-format-cache-entry pw-spec))
 		 tramp-password-save-function
-		 (lambda () (password-cache-add key auth-passwd)))
+ 		 (when auth-source-do-cache
+		   (lambda ()
+		     (password-cache-add
+		      (auth-source-format-cache-entry pw-spec) auth-passwd))))
 	   auth-passwd))

       ;; Workaround.  Prior Emacs 28.1, auth-source has saved empty
       ;; passwords.  See discussion in Bug#50399.
       (when (tramp-string-empty-or-nil-p auth-passwd)
 	(setq tramp-password-save-function nil))
-      (tramp-set-connection-property vec "first-password-request" nil))))
+      ;; Remember the values.
+      (tramp-set-connection-property vec "pw-spec" pw-spec)
+      (tramp-set-connection-property pw-vec "first-password-request" nil))))

 (defun tramp-read-passwd-without-cache (proc &optional prompt)
   "Read a password from user (compat function)."
@@ -6868,17 +6873,11 @@ Consults the auth-source package."
 (defun tramp-clear-passwd (vec)
   "Clear password cache for connection related to VEC."
   (declare (tramp-suppress-trace t))
-  (let ((method (tramp-file-name-method vec))
-	(user-domain (tramp-file-name-user-domain vec))
-	(host-port (tramp-file-name-host-port vec))
-	(hop (tramp-file-name-hop vec)))
-    (when hop
-      ;; Clear also the passwords of the hops.
-      (tramp-clear-passwd (tramp-dissect-hop-name hop)))
-    (auth-source-forget
-     `(:max 1 ,(and user-domain :user) ,user-domain
-       :host ,host-port :port ,method))
-    (password-cache-remove (tramp-make-tramp-file-name vec 'noloc))))
+  (when-let* ((hop (tramp-file-name-hop vec)))
+    ;; Clear also the passwords of the hops.
+    (tramp-clear-passwd (tramp-dissect-hop-name hop)))
+  (when-let* ((pw-spec (tramp-get-connection-property vec "pw-spec")))
+    (auth-source-forget pw-spec)))

 (defun tramp-time-diff (t1 t2)
   "Return the difference between the two times, in seconds.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

On Tue 12 Nov 2024 at 05:11pm +01, Michael Albinus wrote:

> The patch is written for Tramp 2.7.1.4 from GNU ELPA. However, it seems
> to apply also on the Emacs 30 sources (where it wouldn't go due to the
> pretest). Would you like to test?

I am running Emacs 31.  Would you like me to test it on 30 or 31?

> Since this feature has been brought back to work, you don't need to care
> about auth-source-cache-expiry or password-cache-expiry. Simply set the
> connection property "session-timeout" to 8, as you do in your test, and
> it shall work.
>
> I'll continue to work on the patch. Next step, I want to add a test to
> tramp-tests.el, in order to not break it again silently.

I'm pleased I did indeed find a real bug and didn't waste your time :)

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

>> The patch is written for Tramp 2.7.1.4 from GNU ELPA. However, it seems
>> to apply also on the Emacs 30 sources (where it wouldn't go due to the
>> pretest). Would you like to test?
>
> I am running Emacs 31.  Would you like me to test it on 30 or 31?

On 30. The patch won't go to 30.1, but I fear Tramp's codebase is
different enough between 30 and 31, that the patch won't apply.

>> I'll continue to work on the patch. Next step, I want to add a test to
>> tramp-tests.el, in order to not break it again silently.
>
> I'm pleased I did indeed find a real bug and didn't waste your time :)

You don't waste my time anyway. Sometimes it is useful to step through
code written years ago. Even w/o finding a bug.

Best regards, Michael.

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Sean Whitton]

Hello,

Thanks.  I applied that to emacs-30 and tested using my regular init.el,
rather than 'emacs -q', and the problem seems resolved.

-- 
Sean Whitton

bug#74105: 30.0.92; Some issues in TRAMP NEWS entries

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Version 30.2

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

> Thanks.  I applied that to emacs-30 and tested using my regular init.el,
> rather than 'emacs -q', and the problem seems resolved.

Thanks for the feedback. I've pushed the patch, extended by handling of
multi-hops, to the Tramp repository. Will appear with Tramp 2.7.1.5 on
GNU ELPA later this month. It will also go into Emacs 30.2.

Furthermore, I've ported it to Emacs 31. Pushed to the Emacs repository,
master branch.

I'm still working on extending tramp-tests.el, this shall be available
next days.

So I'm closing the bug, because the problem is fixed.

Best regards, Michael.