Bug 1060 - ~/.ssh/config check too strict on systems with per-user groups
Summary: ~/.ssh/config check too strict on systems with per-user groups
Status: CLOSED WONTFIX
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 4.1p1
Hardware: Other Linux
: P2 normal
Assignee: Assigned to nobody
URL: http://bugs.debian.org/314347
Keywords:
Depends on:
Blocks:
 
Reported: 2005-07-04 01:33 AEST by Colin Watson
Modified: 2006-10-07 11:41 AEST (History)
0 users

See Also:


Attachments
allow group-writable ~/.ssh/config under certain conditions (1021 bytes, patch)
2005-07-04 01:35 AEST, Colin Watson
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Watson 2005-07-04 01:33:14 AEST
On systems (such as Debian) that default to per-user groups and 'umask 002', the
new ~/.ssh/config permissions check is too strict. It should permit
group-writability provided that the group in question contains at most the
file's owner, so that one can have a mode 0664 ~/.ssh/config owned by user
cjwatson and group cjwatson.

I'll attach a patch for this in a moment. It would also be useful for this to be
permitted under StrictGroups, although I haven't implemented that. Thanks.
Comment 1 Colin Watson 2005-07-04 01:35:56 AEST
Created attachment 935 [details]
allow group-writable ~/.ssh/config under certain conditions
Comment 2 Damien Miller 2005-07-04 09:51:12 AEST
I don't like these complex heuristics, especially since they depend on
properties of the user and group name. It looks like it would also break on
users who are directly assigned to another user's group in /etc/passwd.

Why can't you just warn your users? (suggestion 5 in the Debian bug). The client
config file is security-critical (malicious modification can lead to account
compromise), so requiring users to pay attention is not a bad thing.
Comment 3 Colin Watson 2005-07-04 10:23:56 AEST
(In reply to comment #2)
> I don't like these complex heuristics, especially since they depend on
> properties of the user and group name.

The only reason the user name is used is because struct group only lets you
inspect group membership by means of the user name.

> It looks like it would also break on users who are directly assigned to
> another user's group in /etc/passwd.

Hmm, there is probably an issue if that other user was not explicitly listed in
his/her primary group in /etc/group, yes. I'll investigate that.

> Why can't you just warn your users? (suggestion 5 in the Debian bug).

The test is wrong for us and causes ssh not to work *by default* as soon as you
create a client config file, unless you take measures which aren't clearly
documented in the error message and ought to be unnecessary in any case. I do
not believe that giving up and merely documenting the problem is a valid
response from me to this bug report.

Suggestion 5 is about trawling through users' home directories on package
installation, which is entirely unacceptable and a cure worse than the disease.
Sorry.

Thanks for your response.
Comment 4 Damien Miller 2005-07-04 10:50:31 AEST
(In reply to comment #3)
> (In reply to comment #2)
> > I don't like these complex heuristics, especially since they depend on
> > properties of the user and group name.
> 
> The only reason the user name is used is because struct group only lets you
> inspect group membership by means of the user name.

No, I was referring to:

+			else if (gr->gr_mem[0]) {
+				if (strcmp(pw->pw_name, gr->gr_mem[0]) ||
+				    gr->gr_mem[1])

where you make decisions on the group name being the same as the user name. 

> > Why can't you just warn your users? (suggestion 5 in the Debian bug).
> 
> The test is wrong for us and causes ssh not to work *by default* as soon
> as you create a client config file, unless you take measures which aren't 
> clearly documented in the error message 

"Bad owner or permissions on /home/djm/.ssh/config" is pretty clear. Not many
error messages on Unix include detailed remedial instructions (I can't think of
one off the top of my head). The ssh(1) manpage makes it quite clear what those
permission needs to be.

> and ought to be unnecessary in any case. I do
> not believe that giving up and merely documenting the problem is a valid
> response from me to this bug report.

I wouldn't characterise warning your users of a potentially dangerous
misconfiguration of their software as "giving up"; it is a perfectly appropriate
response.

> Suggestion 5 is about trawling through users' home directories on package

oops, I meant suggestion 6.
Comment 5 Damien Miller 2005-11-06 04:38:43 AEDT
This won't be changing
Comment 6 Colin Watson 2005-11-06 06:35:56 AEDT
(In reply to comment #4)
> (In reply to comment #3)
> > The only reason the user name is used is because struct group only lets you
> > inspect group membership by means of the user name.
> 
> No, I was referring to:
> 
> +			else if (gr->gr_mem[0]) {
> +				if (strcmp(pw->pw_name, gr->gr_mem[0]) ||
> +				    gr->gr_mem[1])
> 
> where you make decisions on the group name being the same as the user name. 

For the record, gr_mem[0] is the first group member (i.e. a user name), not the group name.
Comment 7 Darren Tucker 2006-10-07 11:41:06 AEST
Change all RESOLVED bug to CLOSED with the exception of the ones fixed post-4.4.