Re: [PATCH] Check system known_hosts file

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Friday 12 July 2013 12:10:50 Tomas Trnka wrote:
> Hello,

Hi Tomáš,

> this short patch makes libssh check the /etc/ssh/known_hosts file
> before the per-user ~/.ssh/known_hosts file, making libssh behave
> the same as OpenSSH client.

thanks for the patch. I would like to have this backed up with a unit test.
 
> Tested only on Linux, but it shouldn't influence Windows at all, as
> the fopen("/etc/ssh/known_hosts") will always fail and per-user
> configuration will be checked right away. If the superfluous fopen is
> a concern, the system known_hosts check can be easily disabled:
> 
> #ifndef WIN32
> const char *current_knownhosts = "/etc/ssh/ssh_known_hosts";
> #else
> const char *current_knownhosts = session->knownhosts;
> #endif

const char *ssh_get_system_known_hosts(void)
{
#ifdef _WIN32
	return NULL;
#endif
	return SSH_SYSTEM_KNOWN_HOSTS;
}

> Signed-off-by: Tomáš Trnka <tomastrnka@xxxxxxx>
> --
> diff '--exclude=*~' -u -r libssh-0.5.4/src/known_hosts.c
> libssh-0.5.4-system-known_hosts/src/known_hosts.c ---
> libssh-0.5.4/src/known_hosts.c      2013-01-22 11:38:30.000000000 +0100 +++
> libssh-0.5.4-system-known_hosts/src/known_hosts.c   2013-03-11
> 14:20:42.593563301 +0100 @@ -419,6 +419,7 @@
>    const char *type;
>    int match;
>    int ret = SSH_SERVER_NOT_KNOWN;
> +  const char *current_knownhosts = "/etc/ssh/ssh_known_hosts";

I would like to see it like this:

	const char *knownhosts = session->knownhosts;

We need to check the user hosts file first!

> 
>    enter_function();
> 
> @@ -456,11 +457,22 @@
> 
>    do {
>      tokens = ssh_get_knownhost_line(session, &file,
> -        session->knownhosts, &type);
> +        current_knownhosts, &type);
> 
> -    /* End of file, return the current state */
> +    /* End of file */
>      if (tokens == NULL) {
> -      break;
> +      /* Checking system known_hosts file done, check user known_hosts now
> */ +      if (current_knownhosts != session->knownhosts) {
> +        current_knownhosts = session->knownhosts;

	knownhosts = ssh_get_system_known_hosts();
	if (knownhosts == NULL) {
		break;
	}
			

> +        if (file != NULL) {
> +          fclose(file);
> +        }
> +        file = NULL;
> +        continue;
> +      } else {
> +        /* Both files checked, return current state */
> +        break;
> +      }
>      }
>      match = match_hashed_host(session, host, tokens[0]);
>      if (match == 0){

Then extend the torture test and create a mock object 
__wrap_ssh_get_system_known_hosts() so we can hand in a file we created.


	-- andreas


-- 
Andreas Schneider                   GPG-ID: F33E3FC6
www.cryptomilk.org                asn@xxxxxxxxxxxxxx