Re: [Ncat] hang on ongoing ssl negotation in brokering mode

[Shinnok <admin () shinnok com>]

Hello David,

On 06/13/2011 11:09 PM, David Fifield wrote:
> Please find a way to factor this code into its own function. It's too
> much to have it repeated in each read loop.
>
>             if(o.ssl && info->ssl && !info->ssl_accept_done){
>                 int ret = SSL_accept(info->ssl);
>                 int sslerr = 0;
...
>                     goto loop_end;
>                 }
>             }

Attached is a patch that tries to resolve this duplication issue,
however, I am not pleased with the outcome that much, since there still
is code duplication as well as unwarranted obfuscation as I guessed
previously.

However, maybe I am thinking it wrong, and I should just grab the entire
ssl negotiation logic code as is and paste it to some function that
takes as an argument not only an fdinfo structure(which is the clean
way), but the fd_list_t pointers so that I can take the necessary action
upon them as needed(rm_fd,etc..) directly from the function. However, I
would refrain from doing that, since it breaks modularity if I put that
kind of ncat_listen/ncat_broker specific logic in ncat_ssl.c.
There's also that fact that I would need to pass fds_ready too, and then
there is the question of how to skip the infinite loops(the continue's).
Oh, and the goto..

Maybe we are worrying too much about this code duplication issue, then
we should. The code in ncat_broker.c and ncat_listen.c is already
similar enough, especially in ncat_listen_stream vs. ncat_broker and
handle_connection in both. Passing stuff around and calling functions
adds to performance slowdown too, besides obfuscation, for no real gain.

The only other way, is to refactor the entire logic code in the
functions in question in order to make de-duplication possible, which is
probably not warranted at this time.

If you think we're still not ok with either solution, let me know and
I'll pass this e-mail to nmap-dev and wait for suggestions. Is there any
other Ncat guru that I can pass this e-mail to, in order to get an
outside opinion?

Best wishes,
Shinnok

Attachment: ncat_ssl_hang_refactor.patch
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/