Bug 1388 - Parts of auth2-pubkey.c are completely devoid of debug logging
Summary: Parts of auth2-pubkey.c are completely devoid of debug logging
Status: CLOSED WONTFIX
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 4.7p1
Hardware: Other All
: P2 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-04 11:42 AEDT by mvolaski
Modified: 2008-07-22 12:20 AEST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mvolaski 2007-11-04 11:42:11 AEDT
Consider this small section of code from the user_key_allowed2 function in auth2-pubkey.c


/* Fail quietly if file does not exist */
        if (stat(file, &st) < 0) {
                /* Restore the privileged uid. */
                restore_uid();
                return 0;
        }
        /* Open the file containing the authorized keys. */
        f = fopen(file, "r");
        if (!f) {
                /* Restore the privileged uid. */
                restore_uid();
                return 0;
        }

Fail quietly? Why? And what about debugging? Someone trying to figure out why authentication has failed is merely left with a statement from later in the code sequence that just says the "key was disallowed". That's not helpful, for it's not technically true.

(I haven't explored other sections of code, but in general, I think any return statement in the middle of function is a failure of that function should probably have an explanatory debug statement at some level associated with it.)
Comment 1 Damien Miller 2008-06-29 00:13:10 AEST
think about it: if the file does not exist, then there are no authorised keys. This is not an error, and doesn't warrant logspam, even at debug levels.
Comment 2 mvolaski 2008-06-29 08:44:14 AEST
After all this time we're waiting for a fix and you're just blowing it off? It doesn't even look like you read my report. Please hand it off to someone else if you don't feel so inclined.

By your own backward logic, how does a nonexistent key get reported as being disallowed?
Comment 3 Damien Miller 2008-06-29 10:00:58 AEST
Sorry, but I don't agree that there is any condition worth reporting here. You don't need debug output to figure out that a file that is critical to public key authentication is missing.
Comment 4 mvolaski 2008-06-29 10:42:12 AEST
Yeah, it took me the better part of a day to figure out that the message, "key is disallowed" really means the key file is missing.

Perhaps users in the future who get similarly stymied will find this page on Google. They surely can't depend on your nicely informative debug logs to help them.
Comment 5 Damien Miller 2008-07-22 12:20:22 AEST
Mass update RESOLVED->CLOSED after release of openssh-5.1