mikaelliljedahl / freesftpsharp

SFTP server with Web Admin GUI based on FxSSH
MIT License
16 stars 3 forks source link

ID mismatch (3 != 1) #6

Closed AndersMalmgren closed 1 month ago

AndersMalmgren commented 1 month ago

I'm trying to connect to the server using windows built in sftp client. It does not support the 2 built in key exchange algoritms so I made a third one diffie-hellman-group14-sha256

_keyExchangeAlgorithms.Add("diffie-hellman-group14-sha256", () => new DiffieHellmanGroupSha256(new DiffieHellman(2048)));

    public class DiffieHellmanGroupSha256 : KexAlgorithm
    {
        private readonly DiffieHellman _exchangeAlgorithm;

        public DiffieHellmanGroupSha256(DiffieHellman algorithm)
        {
            Contract.Requires(algorithm != null);

            _exchangeAlgorithm = algorithm;
            _hashAlgorithm = new SHA256CryptoServiceProvider();
        }

        public override byte[] CreateKeyExchange()
        {
            return _exchangeAlgorithm.CreateKeyExchange();
        }

        public override byte[] DecryptKeyExchange(byte[] exchangeData)
        {
            return _exchangeAlgorithm.DecryptKeyExchange(exchangeData);
        }
    }

It seems to authenticate correctly. It triggers SftoSubsystem.OnInput twice with msgtype = SSH_FXP_INIT

After that the client ends with ID mismatch (3 != 1)

Any ideas?

mikaelliljedahl commented 1 month ago

I could guess: In the SftpSubsystem.cs there is a method, HandleInit:

    private void HandleInit(SshDataWorker reader)
    {
        SshDataWorker writer = new SshDataWorker();
        var sftpclientversion = reader.ReadUInt32();
        writer.Write((byte)RequestPacketType.SSH_FXP_VERSION);
        var version = Math.Max(3, sftpclientversion); // <-- This is where the 3 comes from. I guess the sftpclientversion is 1, maybe it should be Math.Min instead, because the client expects a 1. The server cannot support a higher version than 3
        writer.Write((uint)version); // SFTP protocol version // this is the response to the SSH_FXP_INIT message.
        SendPacket(writer.ToByteArray());
    }

   If you would like to create a PR after confirming changing from Max to Min works, please do!
AndersMalmgren commented 1 month ago

I could guess: In the SftpSubsystem.cs there is a method, HandleInit:

    private void HandleInit(SshDataWorker reader)
    {
        SshDataWorker writer = new SshDataWorker();
        var sftpclientversion = reader.ReadUInt32();
        writer.Write((byte)RequestPacketType.SSH_FXP_VERSION);
        var version = Math.Max(3, sftpclientversion); // <-- This is where the 3 comes from. I guess the sftpclientversion is 1, maybe it should be Math.Min instead, because the client expects a 1. The server cannot support a higher version than 3
        writer.Write((uint)version); // SFTP protocol version // this is the response to the SSH_FXP_INIT message.
        SendPacket(writer.ToByteArray());
    }

   If you would like to create a PR after confirming changing from Max to Min works, please do!

HI, I got past the id mismatch error but I get this instead. Hard for me to diagnose, I'm not that knowing about the SFTP protocol

Expected SSH2_FXP_NAME(104) packet, got 2

mikaelliljedahl commented 1 month ago

https://datatracker.ietf.org/doc/pdf/draft-ietf-secsh-filexfer-02

Version 2 of the protocol is apparently not backwards compatible with version 1 even if I thought it was. This project would then need to be adjusted to respond in a different way when the client says it can only handle version 1. I only implemented support for one version.

The message seems related to sending a list of files in the directory, the client might not want to retrieve them all at once?

Are you somehow constrained to use that very old client?

AndersMalmgren commented 1 month ago

Version 2 of the protocol is apparently not backwards compatible with version 1. This project would then need to be adjusted to respond in a different way when the client says it can only handle version 1. I only implemented support for one version.

Are you somehow constrained to use that very old client version?

I'm integrating with the Swedish Bankgirot which uses SFTP. And they want the file transfer to comply with windows (windows 11) sftp client put command. We use your SFTP server to mock bankgirot and our bank. Works fine when using SSH.NET client but not windows client. I was hoping to use our SFTP mock to verify that the same underlaying commands are used both by sftp client and ssh.net.

mikaelliljedahl commented 1 month ago

Maybe I can help you do the adjustments needed.

Could you list the commands that you are running on the client in the batch?

AndersMalmgren commented 1 month ago

Maybe I can help you do the adjustments needed.

Could you list the commands that you are running on the client in the batch?

You mean our ssh.net client code? Sure, herfe it is

    public class SftpTransferFileDispatcher: ITransferFileDispatcher
    {
        private readonly SftpConfiguration _configuration;
        private readonly ISftpClientScoped _clientScope;

        public SftpTransferFileDispatcher(SftpConfiguration configuration, ISftpClientScoped clientScope)
        {
            _configuration = configuration;
            _clientScope = clientScope;
        }

        public async Task<DispatchFileResult> DispatchFileAsync(FileTransfer fileTransfer, StoredFile file)
        {
            try
            {
                var uri = new Uri(_configuration.SftpOutbound);
                await _clientScope.ConnectAsync(_configuration.SftpUsername, _configuration.SftpPassword, uri);

                await using var stream = _clientScope.Create($"{uri.LocalPath}/{file.Filename}");
                await file.Content.CopyToAsync(stream);
            }
            catch (Exception e)
            {
                throw new DispatchFileTransferException($"Dispatch file via sftp failed due to exception. File: {fileTransfer.ClientMessageId}", e);
            }

            return new DispatchFileResult();
        }
    }

Had to create a interface because the client doesnt mock and test easily.

public class SftpClientScoped : ISftpClientScoped
{
    private SftpClient? _client;

    public async Task ConnectAsync(string username, string password, Uri uri, CancellationToken token = default)
    {
        var port = uri.Port > 0 ? uri.Port : 22;

        _client = new SftpClient(new ConnectionInfo(uri.Host, port, username, new PasswordAuthenticationMethod(username, password)));
        await _client.ConnectAsync(token);
    }

    private SftpClient Client => _client ?? throw new NullReferenceException($"{nameof(ConnectAsync)} must be called before any other methods.");

    public Stream Create(string path) => Client.Create(path);
    public IAsyncEnumerable<ISftpFile> ListDirectoryAsync(string path, CancellationToken token = default) => Client.ListDirectoryAsync(path, token);

    public Stream OpenRead(string path) => Client.OpenRead(path);
    public Task DeleteFileAsync(string path, CancellationToken token = default) => Client.DeleteFileAsync(path, token);

    public void Dispose()
    {
        if (_client != null)
        {
            _client.Disconnect();
            _client.Dispose();
        }
    }
}

Tack

mikaelliljedahl commented 1 month ago

I meant the commands you ran in the sftp client in order to get the error: Expected SSH2_FXP_NAME(104) packet, got 2 but I realized it was already during handshake. It sends Init twice, then the second time it expects the file list in the root directory. So I have crated a PR where this is the behavior. But I actually don´t know how to test it.

AndersMalmgren commented 1 month ago

I meant the commands you ran in the sftp client in order to get the error: Expected SSH2_FXP_NAME(104) packet, got 2 but I realized it was already during handshake. It sends Init twice, then the second time it expects the file list in the root directory. So I have crated a PR where this is the behavior. But I actually don´t know how to test it.

ah got it. Its really easy to test on a windows machine. Just fireup a cmd

sftp myuser@127.0.0.1

Let in handshake public keys. And enter the password. It will call Init twice and fail

edit: You also need my code from the intial post btw. Otherwise key handshake will not work

mikaelliljedahl commented 1 month ago

If you grab the latest commit, you have a version that will not fail the handshake, but I don´t know what to do next. There is no prompt. I recall this client has a way to run commands in a batch, it would be good to have a test batch that I can run and see if it works.

mikaelliljedahl commented 1 month ago

The changes you suggested is part of the first commit in the feature branch, I also bumped the .Net version: https://github.com/mikaelliljedahl/freesftpsharp/pull/7/commits/02480c96bd88a3e5cb728b3465bb9d5e7c688691

mikaelliljedahl commented 1 month ago

I found another error connected to the first directory listing, but at least I am getting the prompt in the sftp client so the init is successful after my last commit.

AndersMalmgren commented 1 month ago

I found another error connected to the first directory listing, but at least I am getting the prompt in the sftp client so the init is successful after my last commit.

Cool, will try the latest version. Thanks

AndersMalmgren commented 1 month ago

Strange, in your branch it tries to use key instead of password even though I have created a user with password login

image

mikaelliljedahl commented 1 month ago

If I understand correctly in that case it is the client that asks for this kind of login in the SSH_MSG_USERAUTH_REQUEST. It should not have been affected by the .Net update. When I tested the client I created a new user and it worked using password authentication.

AndersMalmgren commented 1 month ago

If I understand correctly in that case it is the client that asks for this kind of login in the SSH_MSG_USERAUTH_REQUEST. It should not have been affected by the .Net update. When I tested the client I created a new user and it worked using password authentication.

ah, I had a public key in .ssh on my workstation at home, seems windows client wants to use that if present, Deleted it and now it works. Got this when trying to put

image

Probably another missmatch in protocol?

edit: another missmatch is

image

Maybe to great of work to get it to work with ssh2

mikaelliljedahl commented 1 month ago

Yes, I am experiencing the same, It seems the windows client is sending an SSH_FXP_INIT twice then a SSH_FXP_VERSION (which should only be called from the server) before the next call it makes. 2 is the ID of the response to Init (SSH_FXP_VERSION). If you want, we can collaborate to get this strange client to sort out which flow to use to make it happy.

AndersMalmgren commented 1 month ago

Yes, I am experiencing the same, It seems the windows client is sending an SSH_FXP_INIT twice then a SSH_FXP_VERSION (which should only be called from the server) before the next call it makes. 2 is the ID of the response to Init (SSH_FXP_VERSION). If you want, we can collaborate to get this strange client to sort out which flow to use to make it happy.

Maybe its better too cut our losses. Im not going to use the client against your server anyway. I just wanted to verify that ssh.net client uses the same commands under the hood.

But if you want help getting it compliment i can happily help.

Btw i have rewritten large parts of the subsystem and user/setting management to get it more DI friendly and written a virtual file system version instead of being hardcoded to System.IO. The code might be too specialized for my use case right now. But it might be interesting for you if i pushed a branch?

mikaelliljedahl commented 1 month ago

Making the subsystem DI friendly including user handling was actually on the roadmap. I would love your contribution. Since this is just a hobby project (I spent a few days last summer implementing the subsystem) it is far from finished and polished. I just hardcoded and tried out the LiteDb component for storing users and settings as a test but I admit it is pretty ugly. Please push a branch, I will be happy to merge it.

BTW I found out that the extra message SSH_FXP_VERSION sent by winsftp-client as part of every other message could probably be parsed and ignored and then the next message is next in line. It just have to read the correcy amount of bytes and then go forward to the next one, that could fix the "version" problems.

mikaelliljedahl commented 1 month ago

Even though I could handle the SSH_FXP_VERSION , it seems to be sending multiple command in one message. If I could split them before the OnInput is getting the input I think I could make it work.

AndersMalmgren commented 1 month ago

I created a draft pull request. I will try to get the none working parts working during next week. But please take a look and see if you like the direction its going

mikaelliljedahl commented 1 month ago

I think you PR is very much in the right direction. I originally had an idea to implement a cloud storage based virtual filesystem as an option. One thing I recently did was to update the .Net version to 8. Maybe you are contrained to using 6? However that should not be an issue to downgrade.

AndersMalmgren commented 1 month ago

My branch is just based on an older version before your changes. I'm on .NET 7 becasue we host the server in Azure Service Fabric and they only support upto .NET 7 sadly. :D

mikaelliljedahl commented 1 month ago

https://github.com/mikaelliljedahl/freesftpsharp/commit/dd675aaa848dd4a0cef0263e29e2caf1e30aa61b With this commit I can login and get directory listing to work with the native windows client. It seems it does not send the package size in the request message, I made a hack to identify this (which is not the correct way) and then read the message type directly instead of the message size.

AndersMalmgren commented 1 month ago

Very strange that the MS client is this hacky. I will give it a go later this evening :)

mikaelliljedahl commented 1 month ago

When I tried again the upload/download was interupted after a few packets, only paart of the file was saved. I don't know excatly what is causing this, but it seems now that we're close. During "put" the client send a lot of packets full with nulls (16384 bytes) that I just ignore.

AndersMalmgren commented 1 month ago

Maybe its not worth it, if the code base is harder to maintain. Maybe windows client is edge case, the ssh.net client have worked perfect for me.

mikaelliljedahl commented 1 month ago

It's all about knowing when it is sending the message length as the first 4 bytes. If not, it's the windows client and then the message type is the first byte. But for some message types it behaves like it should, so I added an if-clause to handle that. Now put should work