public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks•im>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, Xi Ruoyao <xry111@xry111•site>,
	"brian m. carlson" <sandals@crustytoothpaste•net>,
	Lauri Tirkkonen <lauri@hacktheplanet•fi>
Subject: Re: [PATCH] t7528: work around ETOOMANY in OpenSSH 10.1 and newer
Date: Thu, 23 Oct 2025 15:24:36 +0200	[thread overview]
Message-ID: <aPoslANELg4V286u@pks.im> (raw)
In-Reply-To: <20251023124320.GA1163932@coredump.intra.peff.net>

On Thu, Oct 23, 2025 at 08:43:20AM -0400, Jeff King wrote:
> On Thu, Oct 23, 2025 at 09:14:59AM +0200, Patrick Steinhardt wrote:
> 
> > As it turns out this is caused by a change in OpenSSH 10.1 [1]:
> > 
> >  * ssh-agent(1), sshd(8): move agent listener sockets from /tmp to
> >    under ~/.ssh/agent for both ssh-agent(1) and forwarded sockets
> >    in sshd(8).
> > 
> > Instead of creating the socket in "/tmp", OpenSSH now creates the socket
> > in our home directory. And as the home directory gets modified to be
> > located in our test output directory we end up with paths that are
> > somewhat long. But Linux has a rather short limit of 108 characters for
> > socket paths, and other systems have even lower limits, so it is very
> > easy now to exceed the limit and run into the above error.
> 
> There's a secondary issue, too: even if the path is short enough, the
> space in "trash directory" of the path will break the shell eval. That's
> relevant below.
> 
> > Work around the issue by using `ssh-agent -T`, which instructs it to
> > use the old behaviour and create the socket in "/tmp" again. This switch
> > has only been introduced with 10.1 though, so for older versions we have
> > to fall back to not using it. That's fine though, as older versions know
> > to put the socket into "/tmp" already.
> 
> OK. I think this is an improvement over the status quo, though it leaves
> a lot of loose ends, like:
> 
>   - what happens if "ssh-agent" does not exist at all; we do not notice
>     the error because the eval succeeds anyway (with blank input)
> 
>   - one reason we did not notice this immediately is that the failure
>     mode is to fall back to using the user's SSH_AUTH_SOCK variable if
>     set (i.e., their real agent with their keys in it!). We should
>     perhaps be clearing that variable in test-lib.sh.
> 
> But those are not really new issues, and I'm OK with just un-breaking
> things in the most expedient way possible.

Yeah. I was wondering whether we should rather do:

    ( ssh-agent -F || ssh-agent ) >env &&
    source env

ssh-agent(1) knows to detach into the background unless told otherwise,
so we should notice the failure and can then source the environment if
it was successful. But I ultimately decided that for now I'd rather want
to fix the fallout, we can still make it more robust after the fact.

> > An alternative approach would be to abbreviate the socket name itself so
> > that we create it as e.g. "sshsock" in the trash directory. But taking
> > the above example we'd still end up with a path that is 91 characters
> > long. So we wouldn't really have a lot of headroom, and it is quite
> > likely that some developers would see the issue on their machines.
> 
> I assume you mean here something like:
> 
>   ssh-agent "$PWD/sshsock"
> 
> Yeah, that is not buying us that much in terms of headroom. Plus it
> would still run afoul of the space issue, since we know that $PWD will
> always contain "trash directory".

Yup, that was the idea, and yeah, I don't think it helps us much.

> If we are going to provide a fixed name, I think it would have to be a
> true relative path like:
> 
>   ssh-agent ./sshsock
> 
> That does work (and SSH_AUTH_SOCK contains the relative path), but is
> maybe a bit of a booby trap waiting to spring on somebody who tries to
> access the agent with a different current working directory.

Maybe. On the other hand we only have a single test anyway that uses
ssh-agent, so that's a problem for the future, I guess.

In any case, I'd say for now we should just fix the issue in the easiest
way possible, and we can then follow up and make this more robust in a
subsequent patch series. WDYT?

Patrick

  reply	other threads:[~2025-10-23 13:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23  7:14 [PATCH] t7528: work around ETOOMANY in OpenSSH 10.1 and newer Patrick Steinhardt
2025-10-23 12:43 ` Jeff King
2025-10-23 13:24   ` Patrick Steinhardt [this message]
2025-10-23 13:34     ` Jeff King

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPoslANELg4V286u@pks.im \
    --to=ps@pks$(echo .)im \
    --cc=git@vger$(echo .)kernel.org \
    --cc=lauri@hacktheplanet$(echo .)fi \
    --cc=peff@peff$(echo .)net \
    --cc=sandals@crustytoothpaste$(echo .)net \
    --cc=xry111@xry111$(echo .)site \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox