cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: lib/gtls.c question about handshake()

From: Alessandro Ghedini <al3xbio_at_gmail.com>
Date: Wed, 24 Oct 2012 11:53:06 +0200

On 10/23, Nick Zitzmann wrote:
> A user recently wrote to me off-list about Secure Transport, and also stated he checked out the latest GnuTLS source code and found something strange after the latest change to the handshake() function.
>
> I looked, and though I'm not a GnuTLS expert, I did notice something that seemed wrong: Shouldn't lines 305 and 308 be reversed? As of right now, if gnutls_handshake() returns an error condition other than GNUTLS_E_AGAIN or GNUTLS_E_INTERRUPTED, if gnutls_error_is_fatal() returns true, then the code loops around again, but if the function returns false, then it returns CURLE_SSL_CONNECT_ERROR. That doesn't seem right to me; shouldn't it be the other way around? Just wondering...

I don't remember the original idea but looking at the code right now, I think
it should've been "rc < 0 and *not* gnutls_error_is_fatal" and then it should
reset the state machine (i.e. without failing):

    ...
    else if((rc < 0) && !gnutls_error_is_fatal(rc)) {
      failf(data, "gnutls_handshake() warning: %s", gnutls_strerror(rc));
      goto reset;
    }
    else if(rc < 0) {
      failf(data, "gnutls_handshake() failed: %s", gnutls_strerror(rc));
      return CURLE_SSL_CONNECT_ERROR;
    }
    else {
reset:
      /* Reset our connect state machine */
      connssl->connecting_state = ssl_connect_1;
      return CURLE_OK;
    }
    ...

Or a simpler and less ugly solution, would be to not print the warning message:

    ...
    else if((rc < 0) && gnutls_error_is_fatal(rc)) {
      failf(data, "gnutls_handshake() failed: %s", gnutls_strerror(rc));
      return CURLE_SSL_CONNECT_ERROR;
    }
    else {
      /* Reset our connect state machine */
      connssl->connecting_state = ssl_connect_1;
      return CURLE_OK;
    }
    ...

Probably this is also what caused the problem with git. Thinking about it, it
seems weird that it actually fixed the problem with non-fatal gnutls warnings.

Attached is the patch that implements the second solution. I am testing it right
now to see if it works, so do not merge this yet.

Oh, and sorry for all the problems :/

Cheers

-- 
perl -E '$_=q;$/= @{[@_]};and s;\S+;<inidehG ordnasselA>;eg;say~~reverse'

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html

Received on 2012-10-24