Bug 2870 - Support "%i" token expansion in the IdentityAgent option
Summary: Support "%i" token expansion in the IdentityAgent option
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 7.7p1
Hardware: Other Linux
: P5 enhancement
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_7_8
  Show dependency treegraph
 
Reported: 2018-05-21 20:11 AEST by Hristo Venev
Modified: 2021-04-23 15:00 AEST (History)
3 users (show)

See Also:


Attachments
Support %U in sshd percent_expand for UID. Extend %i availability in ssh (8.33 KB, patch)
2018-05-25 14:18 AEST, Damien Miller
no flags Details | Diff
Cast to safe integral value (9.30 KB, patch)
2018-05-25 15:02 AEST, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hristo Venev 2018-05-21 20:11:03 AEST
It is very useful if someone wants the socket to live under /run/user/$UID.
Comment 1 Damien Miller 2018-05-25 14:18:40 AEST
Created attachment 3155 [details]
Support %U in sshd percent_expand for UID. Extend %i availability in ssh

This patch adds a %U expansion to all the sshd_config directives that accept a username (%u) token.

I would have used %i as is used in ssh, but unfortunately that's already collided by the certificate key-ID token in sshd (sorry, my fault).

This also expands the availability of %i in ssh to everywhere that currenly supports %u too.
Comment 2 Darren Tucker 2018-05-25 14:39:35 AEST
Comment on attachment 3155 [details]
Support %U in sshd percent_expand for UID. Extend %i availability in ssh

> snprintf(uidstr, sizeof(uidstr), "%d", pw->pw_uid);

on some platforms uid_t is long not int?
Comment 3 Damien Miller 2018-05-25 15:02:20 AEST
Created attachment 3156 [details]
Cast to safe integral value

POSIX says that uid_t can be any integral type, so cast to unsigned long long. IMO this is safe wrt -ve values, whereas a signed type with be undefined behaviour on a platform with unsigned uid_t and sufficiently large UIDs.
Comment 4 Darren Tucker 2018-05-25 19:32:14 AEST
Comment on attachment 3156 [details]
Cast to safe integral value

>+	snprintf(uidstr, sizeof(uidstr), "%llu",
>+	    (unsigned long long)pw->pw_uid);

check for truncation (or use xasprintf)?  shouldn't happen with 64bits, but maybe someone will make them GUIDs or something one day.

otherwise ok.
Comment 5 Damien Miller 2018-06-01 13:33:56 AEST
This has been committed and will be in OpenSSH 7.8 - thanks!
Comment 6 Roumen Petrov 2018-06-03 06:08:47 AEST
load_public_identity_files() expands %h and %i to same value. Please check.

In addition I would like client to use %U instead %i, i.e. %i to be considered as obsolete.
Of course for backward compatibility %i has to be available.
Using common TOKEN options (%U and %u) in client and daemon will minimize configuration errors.
Comment 7 Damien Miller 2018-06-08 13:36:22 AEST
Thanks I've fixed the incorrect %i expansion.
Comment 8 Damien Miller 2021-04-23 15:00:22 AEST
closing resolved bugs as of 8.6p1 release