OpenSSH sftp cannot connect with v1.4.14-stable, whilst it could with v1.4.13-stable about wolfssh HOT 11 CLOSED

@falemagn ,

I've assigned this to @bigbrett . He will be looking into this for you.

Warm regards, Anthony

from wolfssh.

With git bisect I was able to identify the first commit that caused this precise symptom.

6a3e4856b488f3d8598ce3a587436997fd2f6d9b is the first bad commit
commit 6a3e4856b488f3d8598ce3a587436997fd2f6d9b
Author: John Safranek <[email protected]>
Date:   Thu Jun 15 13:25:33 2023 -0700

    I/O Buffer Refactor
    
    1. Started changing LENGTH_SZ to UINT32_SZ. The RFCs define the length
       of strings and mpints as uint32 values.
    2. Some whitespace changes.
    3. The getting data into the input buffer is done based on the goal size.
       The pattern is to get the first block, decrypt it if necessary, then read
       the size out, then get that much data plus the size of the length and the
       MAC.

Analyzing this commit, turns out there's a copy'n'paste error. The following patch fixes it:

diff --git a/src/internal.c b/src/internal.c
index 1cde473..de86ea2 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -7461,7 +7461,7 @@ int DoReceive(WOLFSSH* ssh)
                                     + peerBlockSz,
                                 ssh->inputBuffer.buffer + ssh->inputBuffer.idx
                                     + peerBlockSz,
-                                ssh->curSz - peerBlockSz);
+                                ssh->curSz + UINT32_SZ - peerBlockSz);
                     }
                     else {
                         /* Entire packet fit in one block, don't need

After applying that patch over the aforementioned commit, it all works again, but applying that patch over v1.4.14-stable makes another problem show up: sftp stalls waiting for some replies that never arrives.

debug3: send packet: type 50
debug2: we sent a publickey packet, wait for reply

If I then rerun sftp with the -oPreferredAuthentications=password option, it works.

from wolfssh.

After further investigation, reverting the following two commits, and then applying the aforementioned patch, makes things work: 9d1996a, 169d2c6.

from wolfssh.

Hi @falemagn ,

Thank you so much for the report and the proposed fixes!! I think completely reverting 9d1996a, 169d2c6 might not be desirable. I will get this ticket assigned to a wolfSSL engineering team member so we can get to a more pin pointed solution.

Please stay tuned.

Warm regards, Anthony

from wolfssh.

Hi @falemagn ,

Thank you so much for the report and the proposed fixes!! I think completely reverting 9d1996a, 169d2c6 might not be desirable.

I am sure it wouldn't, I just thought I'd point out that there lies the source of the issue, so to help you guys find a proper fix. :)

Thanks for the prompt response!

from wolfssh.

When investigating another report I added this pull request (#559). I suspect it will resolve this issue too.

from wolfssh.

When investigating another report I added this pull request (#559). I suspect it will resolve this issue too.

Only the first one, though, not the other caused by something happened because of 9d1996a and 169d2c6, as reported previously.

from wolfssh.

For the second part of the issue I think this part of the pull request might be related https://github.com/wolfSSL/wolfssh/pull/559/files#diff-1c4e2f5adfa1ad30618e78ff459b2c0758ecf34278459ad0a8d58db4fec622eaR5845.

Before adding that I was seeing a hang in the example server after rejecting a peer's public key.

from wolfssh.

You are right, that PR solves this specific issue. I was misled by the fact we're experiencing an issue in our code base which I was able to identify as stemming from this very issue we are discussing.

However, even with that PR, in our code base we still experience the hang. Has anything changed about what's expected from the user auth callback, due to 9d1996a and 169d2c6?

from wolfssh.

Ok, I believe I have found it: there's a missing ret = WS_SUCCESS if the return code from the user auth callback is not SUCCESS and is not one of the errors being explicitly handled, which makes the function not return failure to the client.

I have added a review to PR #559.

from wolfssh.

Given the merge of #559 in master, the issue can now be closed.

from wolfssh.