Bug 1060

Created attachment 935 [details]
allow group-writable ~/.ssh/config under certain conditions

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.

(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.

(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.

This won't be changing

(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.

Change all RESOLVED bug to CLOSED with the exception of the ones fixed post-4.4.