Request for comment, logging patch

[Darren Tucker]

Vikash Badal - PCS wrote:
> Greetings. 
> 
> Attached is a patch that provides more logging information 
> for example: 
> Mar 19 08:34:54 secosr5 sshd[7667]: Accepted publickey of  vix at wormhole for root from 192.168.1.1 port 1256 ssh2 
> Mar 19 08:34:54 secosr5 sshd[7667]: executing command 'who' for  vix at wormhole as user root 
> Mar 19 10:37:16 secosr5 sshd[7725]: Accepted publickey of  vix at wormhole for root from 192.168.1.1 port 1257 ssh2 
> Mar 19 10:37:16 secosr5 sshd[7725]: executing command 'scp -f /usr/udd1/dev/openssh-3.8p1.patch' for  vix at wormhole as user root
> 
> Can this code be reviewed and possibly added to the code base ? 
> Please let me know what is incorrect with this code. 

 > +extern char realname[64];

"realname" is populated with a copy of the address part of the comment 
in the key.  Firstly, I'm not keen on logging too much user-controlled 
data, and there's no reason why the comment won't be longer than 64 chars...

If you must log it, you should probably run it through strnvis to escape 
any nasties.

 >               /* cp now points to the comment part. */
 > +             comment = cp;
 > +             commentlen = strlen(comment);
 > +             if (commentlen > 0 && comment[commentlen -1] == '\n')
 > +                comment[commentlen - 1] = '\0';

You're modifying the source string, although it looks like you're trying 
not to (comment and cp are just pointers that point to the same chunk of 
memory).  You should probably use xstrdup (but see above).

+extern char user_name[16];
 > +     strncpy(user_name, authctxt->user, sizeof(user_name) -1 );

Is it really neccessary to keep another copy of the_authctxt->user?  And 
what guarantee is there that it's less than 16 chars?

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
     Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.