Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for openssh known_hosts files #438

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

DanielVukelich
Copy link

We're moving our systems to .NET Core, and one of our dependencies is an incompatible sftp library. SSH.NET would be a perfect drop-in replacement, except that the sftp library supports verifying hosts through an openssh known_hosts file.

This pull request implements parsing and export for known_host files, and is compatible with all behavior specified in sshd's manpage entry with the exception of certificate authority signing.

Example usage:

using Renci.SshNet;
using Renci.SshNet.Common;
using System;

namespace SftpTest
{
    class Program
    {
        private static KnownHostStore _knownHosts;

        static void Main(string[] args)
        {
            _knownHosts = new KnownHostStore("~/.ssh/known_hosts");
            using (SftpClient client = new SftpClient("test.rebex.net", 22, "demo", "password"))
            {
                client.HostKeyReceived += (sender, eventArgs) =>
                {
                    eventArgs.CanTrust = CanTrustHost(client.ConnectionInfo.Host, eventArgs);
                };

                try
                {
                    client.Connect();
                    Console.WriteLine("Connected!");
                }
                catch (Exception ex)
                {
                    Console.WriteLine("Failed to connect: " + ex.Message);
                }
            }

            _knownHosts.ExportKnownHostsToFile("known_hosts.txt");
            Console.WriteLine("Press any key to continue");
            Console.ReadKey();
        }

        private static bool CanTrustHost(string hostname, HostKeyEventArgs e)
        {
            if (_knownHosts.Knows(hostname, e.HostKeyName, e.HostKey, 22))
                return true;

            Console.WriteLine("Hostname:\t" + hostname);
            Console.WriteLine("KeyType:\t" + e.HostKeyName);
            Console.WriteLine("Fingerprint:\t" + BitConverter.ToString(e.FingerPrint));
            Console.WriteLine(Environment.NewLine + "Unknown host.  Trust it?  y/N: ");

            int choice = Console.Read();
            if (choice != 'y' && choice != 'Y')
                return false;

            _knownHosts.AddHost(hostname, 22, e.HostKeyName, e.HostKey, false);
            return true;
        }
    }
}

Can read a file in the format of openssh's known_hosts, which allows identity verification of ssh hosts that you connect to.  Hosts can be added to a KnownHostStore object either programatically, or by parsing an openssh known_hosts file.
KnownHostStore can also be exported to text format, aloowing openssh or other compatible programs access to the hosts and their identities.  All features of the man 8 sshd secion on known_hosts are supported, except certificate authority
validation, which is not available in this library yet.
Copy link

@BorisAmelyan BorisAmelyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we release this support for OPENSSH?
looks like someone already put an effort to add this feature

@avr2222
Copy link

avr2222 commented May 18, 2022

Open SSH Known hosts still not supported?

@A9G-Data-Droid
Copy link

To avoid conflicts with efforts to make this library work on FIPS enabled systems we have to be careful to use approved hash algorithms. HMACSHA1 was recently deprecated by both the government and Microsoft.

Due to collision problems with SHA1, Microsoft recommends a security model based on SHA256 or better.

https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.hmacsha1.-ctor?view=net-6.0#system-security-cryptography-hmacsha1-ctor(system-byte()-system-boolean)

The new versions of HMACSHA256, HMACSHA384, and HMACSHA512 are all FIPS compliant now.

https://learn.microsoft.com/en-us/previous-versions/dotnet/netframework-4.0/w8h3skw9(v=vs.100)?redirectedfrom=MSDN

I would use HMACSHA256 at a minimum to avoid deprecation warnings.

HMACSHA256 hmac = new HMACSHA256(_hashSalt);

@DanielVukelich
Copy link
Author

From a compatibility standpoint, sha1 is the only hashing algorithm that I can find is supported by tools like OpenSSH. As far as I can tell, there is no way for a known hosts entry to specify which algorithm it uses for hashing, so everyone assumes sha1. If the goal is to support known_hosts files from any source, then the hashing would have to be sha1.

I can understand the concern with FIPs compliance, but from a security standpoint the hash is only used to obscure the hostnames from a human attacker who tries to enumerate all the known hosts. I'm not a cryptographer, but I think in this case that collision attacks would be meaningless, since any colliding hostname would necessarily be different from the actual hostname that the known_hosts entry is valid for, so there's no reason for an attacker to try and connect to the collided host. Again though, I know that FIPS is more about compliance than actual security.

Maybe if there are some FIPS compile flags, this could be selectively supported? Otherwise, support for hashed hosts should be removed. It's been years since I wrote this commit.

@A9G-Data-Droid
Copy link

Thanks for the thoughtful response. I think the solution is to have an option to turn off hashed known_hosts. At closer inspection of your code, it looks like it does support plaintext known_hosts. So I apologize for the detour.

Looks like people are moving away from hashing known_hosts because it doesn't have any security value:
https://security.stackexchange.com/questions/56268/ssh-benefits-of-using-hashed-known-hosts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants