Bug 1159

Summary:

%u and %h not handled in IdentityFile

Product:

Portable OpenSSH

Reporter:

John Bowman <imaging>

Component:

ssh

Assignee:

Assigned to nobody <unassigned-bugs>

Status:

CLOSED FIXED

Severity:

normal

CC:

djm, imaging, jprondak

Priority:

Keywords:

patch

Version:

4.3p2

Hardware:

All

OS:

Linux

URL:

http://www.math.ualberta.ca/imaging/snfs/openssh.html

Bug Depends on:

Bug Blocks:

1155

Attachments:

Description	Flags
User-dependent IdentityFile	none
User-dependent IdentityFile	djm: ok-
Revised diff	none
Improved diff, with dtucker's suggestions	dtucker: ok+

Description John Bowman 2006-02-22 16:40:00 AEDT

Here is a patch to allow private key files to be placed system wide (for all users) in a secure (non-NFS) mounted location on systems where home directories are NFS mounted. This addresses an important security hole on systems where home directories are NFS mounted, particularly if there are users who use blank passphrases (or when lpd is tunneled through ssh on systems running lpd as user lp) instead of ssh-agent. IdentityFile now accepts the same %u, %h, %% options that AuthorizedKeysFile accepts (see man sshd). For example, one can specify a user-dependent IdentityFile in ssh_config:

IdentityFile /ssh/%u/id_rsa

Comment 1 John Bowman 2006-02-22 16:41:30 AEDT

Created attachment 1076 [details]
User-dependent IdentityFile

portable version

Comment 2 John Bowman 2006-02-22 16:42:47 AEDT

Created attachment 1077 [details]
User-dependent IdentityFile

OpenBSD version

Comment 3 Damien Miller 2006-02-26 12:49:03 AEDT

Comment on attachment 1077 [details]
User-dependent IdentityFile

>diff -ru ssh/ssh-keygen.c sshJ/ssh-keygen.c
>--- ssh/ssh-keygen.c	2005-11-28 19:04:55.000000000 -0700
>+++ sshJ/ssh-keygen.c	2006-02-21 15:52:36.000000000 -0700
...
>+ 	/* Read systemwide configuration file after user config. */
>+ 	(void)read_config_file(_PATH_HOST_CONFIG_FILE, hostname, &amp;options, 0);

I don't think we want the other tools to depend on ssh_config. 

>--- ssh/ssh.c	2005-12-19 21:41:07.000000000 -0700
>+++ sshJ/ssh.c	2006-02-21 15:52:36.000000000 -0700
...
> 	for (; i &lt; options.num_identity_files; i++) {
> 		filename = tilde_expand_filename(options.identity_files[i],
> 		    original_real_uid);
>+		filename = percent_expand(filename, "h", pw-&gt;pw_dir,	
>+					  "u", pw-&gt;pw_name, (char *)NULL);
> 		public = key_load_public(filename, NULL);

This leaks memory. Also, I think it would be better to have:

%u -> user
%h -> local hostname
%d -> home directory

I'll attach a revised patch

Comment 4 Damien Miller 2006-02-26 12:53:54 AEDT

Created attachment 1083 [details]
Revised diff

This diff fixes the memory leak and uses the mnemonics from Comment #3.

Note that the diff doesn't touch ssh-keygen.

Comment 5 Darren Tucker 2006-02-26 17:15:20 AEDT

(In reply to comment #3)
> Also, I think it would be better to have:
> 
> %u -> user
> %h -> local hostname
> %d -> home directory

The current uses of percent_expand() are, with this proposal at the bottom:

                ClHost  ClUser  SrvHost SrvUser SrvPort Homedir
authorized_keys                         %u              %h
ControlPath     %l              %h      %r      %p
ProxyCommand                    %h              %p
IdentityFile    %h      %u                              %d

To keep the client-side consistent, those could be:
%u -> local user
%l -> local hostname
%d -> home directory

The following might also be useful:
%h -> remote host
%r -> remote username

Comment 6 Damien Miller 2006-03-12 15:47:48 AEDT

Created attachment 1097 [details]
Improved diff, with dtucker's suggestions

Good points Darren, revised diff attached.

Comment 7 Damien Miller 2006-03-12 16:06:03 AEDT

*** Bug 95 has been marked as a duplicate of this bug. ***

Comment 8 Darren Tucker 2006-03-30 21:27:16 AEDT

Comment on attachment 1097 [details]
Improved diff, with dtucker's suggestions

Looks and tests fine here.

I would have used something other than "me" ("thishost"? "lhost"?) to prevent confusing it with the local user name.

Comment 9 Damien Miller 2006-03-30 21:42:40 AEDT

Patch applied (with s/me/thishost/) and will be in 4.4. Thanks

Comment 10 Darren Tucker 2006-09-28 19:25:51 AEST

With the release of 4.4, we believe that this bug is now closed.  For information about the release please see http://www.openssh.com/txt/release-4.4 .