Victor Duchovni wrote:
> On Fri, Dec 14, 2007 at 11:06:58AM +0100, Lutz Jaenicke wrote:
>
>   
>>>         else if (ret < 0) {
>>>             switch(SSL_get_error((SSL *)s, ret)) {
>>>             case SSL_ERROR_WANT_READ:
>>>             case SSL_ERROR_WANT_WRITE:
>>>                 break;
>>>             default:
>>>                 msg_info("%s:error in %s",
>>>                          str, SSL_state_string_long((SSL *) s));
>>>                 break;
>>>             }
>>>         }
>>>     }
>>>
>>> What is new here is the call to SSL_get_error() to skip the WANT_READ
>>> and WANT_WRITE cases, only logging other (unexpected) errors. Is this
>>> approach sound? Is it safe to call SSL_get_error() from the info callback,
>>> or might it modify the SSL state in some unsupported fashion.
>>>
>>> I am hoping this is a correct analysis and sensible solution..
>>>       
>> Hi Victor,
>>
>> your analysis is correct. The info callback is not very sensible when it
>> comes to the non-blocking case.
>> Your approach comes with an upside and a downside. What you are
>> seeing with your approach is more a view to the SSL handshake
>> state (which may be better understandable for the users).
>> You are however (as a developer) loosing the information about
>> additional calls being necessary to complete read/write operations.
>>     
>
> Provided SSL_get_error() has and continues to have no side-effects,
> I am inclined to implement this.
>   

It better should not have side-effects. Applications need SSL_get_error()
to learn about WANT_WRITE and WANT_READ states to act accordingly.

> The purpose of the detailed handshake logging is I think to let *users*
> understand the protocol interactions with the remote system, and identify
> where the handshake breaks down. This is not a developer tool, I can
> always comment out the first "break;" and get the old behaviour when
> testing new code.
>   

Ack.

> Users can of course get very detailed protocol insight with "ssldump"
> (abandoned) or wireshark (actively maintained), but providing info
> callback logging is a basic first step.
>   

Using a tool like the above is better. There was a time when I was the
developer of the component we are talking about :-) I implemented the
informational/debug output because doing so was "state of the art".
I never used it myself but preferred ssldump.
I was even thinking about removing it altogether...

> Real-world testing of this is a bit tricky, because real-world systems
> are not configured with this level of logging, almost everyone has
> the info-callback turned off. Only people debugging issues (and a few
> clueless newbies who crank the log level to "debug" for no reason at all)
> have the info callback enabled.
>
> My "lab testing shows no adverse effects on TLS sessions that work
> flawlessly (that's what happens in the lab). Instrumenting a server
> or client to misbehave in interesting ways is a lot of work.
>   

Before writing the statement about SSL_get_error() having no side
effects, I did perform a code review just to be sure. I would thus be
surprised if anything pops up.


Best regards,
    Lutz
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
User Support Mailing List                    openssl-users@openssl.org
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to