Bug 2870

Summary: Support "%i" token expansion in the IdentityAgent option
Product: Portable OpenSSH Reporter: Hristo Venev <hristo>
Component: sshAssignee: Damien Miller <djm>
Status: CLOSED FIXED    
Severity: enhancement CC: bugtrack, djm, dtucker
Priority: P5    
Version: 7.7p1   
Hardware: Other   
OS: Linux   
Bug Depends on:    
Bug Blocks: 2852    
Attachments:
Description Flags
Support %U in sshd percent_expand for UID. Extend %i availability in ssh
none
Cast to safe integral value dtucker: ok+

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