[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: a small patch


Great!  Thank you.

Here are a couple of fixes on ssh2.

* libssh runs into a wrong state when connecting to a server that
supports keyboard-interactive (e.g., linux.mit.edu), "authenticated"
before asking for password.

In ssh_userauth_kbdint (auth.c), first the code checks if
session->kbdint == NULL, then goes to ask_userauth to send
"ssh-userauth" and call ssh_handle_packets, where it will be
dispatched to ssh_packet_userauth_pk_ok.  Note that session->kbdint is
still NULL, session->auth_state will then be set to
SSH_AUTH_STATE_PK_OK, though it should have been SSH_AUTH_STATE_INFO.
One possible fix is to initialize session->kbinit right after the NULL
check.

* two possible infinite loops in auth.c:218 and auth1.c:38, which can
be avoided by checking the return value of ssh_handle_packets.

* sample.c:451 calls ssh_userauth_none then later calls
authenticate_console.  actually authenticate_console will invoke
ssh_userauth_none again, which fails some server (e.g., bbs@xxxxxx).

The attached patch also includes the fixes mentioned in my previous email.

-xi

On Wed, Apr 21, 2010 at 5:08 AM, Aris Adamantiadis <aris@xxxxxxxxxxxx> wrote:
> Hi Xi,
>
> Thanks for your patch. Libssh master is currently in architectural
> rework and SSH1 has been a little less polished than the rest. I'm happy
>  you managed to make it work.
>
> I have yet to verify that the \r\n -> \n thing does not break anything
> on openssh and cisco SSH.
>
> Regards,
>
> Aris
>
> Xi Wang a écrit :
>> Cool.  Here goes a few more fixes when I was trying libssh with
>> guest@xxxxxxxxxxx, an ssh-based forum using v1.
>>
>> * banner/identification: ssh_send_banner (client.c).  libssh reports
>> crc error after sending the session key.  The actual problem is that
>> libssh uses "...\r\n" in its identification, while some sever doesn't
>> parse the identification correctly and stops at "\r".  Since the ssh
>> specification (http://www.snailbook.com/docs/protocol-1.5.txt) says it
>> should be "...\n", the patch removes "\r".
>>
>> * hang (packet1.c:337).  libssh hangs after authentication.  I guess
>> it's because libssh doesn't run into the correct state, i.e., from
>> SSH_SESSION_STATE_AUTHENTICATING to SSH_SESSION_STATE_AUTHENTICATED.
>> Not sure if the fix is at the right place.
>>
>> * close: ssh_connection_callback (client.c:645).  When in error state
>> (e.g., the remote server closes the connection), libssh/samplessh
>> doesn't close automatically.  Not sure if this fix is necessary.
>>
>> * samplessh hangs (sample.c), for example, when channel_poll returns -1 (error).
>>
>> Thanks.
>>
>> - xi
>>
>> On Tue, Apr 20, 2010 at 7:32 AM, Andreas Schneider <mail@xxxxxxxxxxxx> wrote:
>>> On Tuesday 20 April 2010 13:05:45 you wrote:
>>>> Hi,
>>> Hi Xi,
>>>
>>>> Attached is a patch that addresses the following problems.
>>>>
>>>> * two memory leaks: enc_session (kex.c:781), session->packet_callbacks
>>>> (session.c:209);
>>>>
>>>> * openssl configuration problem: use OPENSSL_FOUND instead of
>>>> CRYPTO_FOUND, which doesn't exist in my cmake 2.8.
>>>>
>>>> * compile warnings: avoid using the name "signal" (channels.c);
>>>>
>>>> * link error when WITH_SERVER=0 (packet.c:59);
>>>>
>>>> * minor compile error (torture_options.c).
>>> thank you very much for your patch. I've split it up in smaller patches and
>>> pushed it to the repository.
>>>
>>>
>>> Cheers,
>>>
>>>        -- andreas
>>>
>>>
>>>
>
>
>

Attachment: b1.patch
Description: Binary data


Follow-Ups:
Re: a small patchAris Adamantiadis <aris@xxxxxxxxxxxx>
Re: a small patchAris Adamantiadis <aris@xxxxxxxxxxxx>
References:
a small patchXi Wang <xi.wang@xxxxxxxxx>
Re: a small patchAndreas Schneider <mail@xxxxxxxxxxxx>
Re: a small patchXi Wang <xi.wang@xxxxxxxxx>
Re: a small patchAris Adamantiadis <aris@xxxxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org