openssh unix dev - Sep 2024 - [PATCH] ssh-add: Support '@' in the user part of destination constraints

Properly adding a (complete) host constraint for one of my Git SSH
identities was impossible because the string got split into username
and host at the first '@', yet the username itself contains an
'@'.

This patch changes the `ssh-add` command to support destination
constraints which contain '@' in the user part and therefor the
creation of fully qualified SSH agent restrictions for such hosts.
Specifying and using those user names is already possible with the
OpenSSH client. This patch makes `ssh-add -h` match the existing
behaviour of `ssh` and fixes what seemingly was an oversight.

Previously the lines were split on the first '@', now they are split
on the last '@' which matches the behaviour of the client (see lines
1104 and following of the current src/usr.bin/ssh/ssh.c.)

In addition to running the patched version against all my constraints,
I also tested it with the additional line `debug3_f("User: \"%s\"
Host: \"%s\"", dch->user, dch->hostname);` to make sure
that I have no
off-by-one error which would lead to wrongly parsed components.
And it obviously also works when connecting to the SSH server that
expects those user names while using the agent.

I already tried to get this patch into OpenBSD directly but was
pointed to this address in a private reply. See the thread on
tech at openbsd.org: https://marc.info/?t=172412009100002
I knew of the list but thought that submitting the patch to the core
project would be preferable.

If you are unsure about the current behaviour of the OpenSSH client
and need further help to verify it, just read the linked thread which
should contain all the details.
I'm getting a bit tired of arguing a one character diff in
excruciating detail.

Ok?

Index: ssh-add.c
==================================================================RCS file:
/cvs/src/usr.bin/ssh/ssh-add.c,v
retrieving revision 1.172
diff -u -p -r1.172 ssh-add.c
--- ssh-add.c	11 Jan 2024 01:45:36 -0000	1.172
+++ ssh-add.c	19 Aug 2024 23:35:16 -0000
@@ -691,7 +691,7 @@ parse_dest_constraint_hop(const char *s,
 
 	memset(dch, '\0', sizeof(*dch));
 	os = xstrdup(s);
-	if ((host = strchr(os, '@')) == NULL)
+	if ((host = strrchr(os, '@')) == NULL)
 		host = os;
 	else {
 		*host++ = '\0';

On Thu, 5 Sep 2024, Max Zettlmei?l via openssh-unix-dev wrote:
> Properly adding a (complete) host constraint for one of my Git SSH
> identities was impossible because the string got split into username
> and host at the first '@', yet the username itself contains an
'@'.
[snip]
> If you are unsure about the current behaviour of the OpenSSH client
> and need further help to verify it, just read the linked thread which
> should contain all the details.
> I'm getting a bit tired of arguing a one character diff in
> excruciating detail.
Save yourself some trouble next time and read
https://www.openssh.com/report.html -- tech at openbsd.org isn't a great
place to report OpenSSH bugs. I read it only infrequently.
> Ok?
Patch seems ok, but I think it makes sense to review all the
user at host parsing; grep "str.*chr.*@" is a mess...

I'll look at it.