Commit 04b4ee54 authored by Jay Satiro's avatar Jay Satiro
Browse files

vtls: Only call add/getsession if session id is enabled 

Prior to this change we called Curl_ssl_getsessionid and
Curl_ssl_addsessionid regardless of whether session ID reusing was
enabled. According to comments that is in case session ID reuse was
disabled but then later enabled.

The old way was not intuitive and probably not something users expected.
When a user disables session ID caching I'd guess they don't expect the
session ID to be cached anyway in case the caching is later enabled.
parent 046c2c85

lib/vtls/axtls.c

+23 −21
Original line number Diff line number Diff line
@@ -143,8 +143,6 @@ static CURLcode connect_prep(struct connectdata *conn, int sockindex) 
  int cert_types[] = {SSL_OBJ_X509_CERT, SSL_OBJ_PKCS12, 0};

  int key_types[] = {SSL_OBJ_RSA_KEY, SSL_OBJ_PKCS8, SSL_OBJ_PKCS12, 0};

  int i, ssl_fcn_return;

  const uint8_t *ssl_sessionid;

  size_t ssl_idsize;



  /* Assuming users will not compile in custom key/cert to axTLS.

  *  Also, even for blocking connects, use axTLS non-blocking feature.
@@ -258,6 +256,10 @@ static CURLcode connect_prep(struct connectdata *conn, int sockindex) 
   * 2) setting up callbacks.  these seem gnutls specific

   */



  if(conn->ssl_config.sessionid) {

    const uint8_t *ssl_sessionid;

    size_t ssl_idsize;



    /* In axTLS, handshaking happens inside ssl_client_new. */

    Curl_ssl_sessionid_lock(conn);

    if(!Curl_ssl_getsessionid(conn, (void **) &ssl_sessionid, &ssl_idsize)) {
@@ -265,13 +267,13 @@ static CURLcode connect_prep(struct connectdata *conn, int sockindex) 
      infof (data, "SSL re-using session ID\n");

      ssl = ssl_client_new(ssl_ctx, conn->sock[sockindex],

                           ssl_sessionid, (uint8_t)ssl_idsize);

    Curl_ssl_sessionid_unlock(conn);

    }

  else {

    Curl_ssl_sessionid_unlock(conn);

    ssl = ssl_client_new(ssl_ctx, conn->sock[sockindex], NULL, 0);

  }



  if(!ssl)

    ssl = ssl_client_new(ssl_ctx, conn->sock[sockindex], NULL, 0);



  conn->ssl[sockindex].ssl = ssl;

  return CURLE_OK;

}
@@ -284,8 +286,6 @@ static CURLcode connect_finish(struct connectdata *conn, int sockindex) 
{

  struct SessionHandle *data = conn->data;

  SSL *ssl = conn->ssl[sockindex].ssl;

  const uint8_t *ssl_sessionid;

  size_t ssl_idsize;

  const char *peer_CN;

  uint32_t dns_altname_index;

  const char *dns_altname;
@@ -383,13 +383,15 @@ static CURLcode connect_finish(struct connectdata *conn, int sockindex) 
  conn->send[sockindex] = axtls_send;



  /* Put our freshly minted SSL session in cache */

  ssl_idsize = ssl_get_session_id_size(ssl);

  ssl_sessionid = ssl_get_session_id(ssl);

  if(conn->ssl_config.sessionid) {

    const uint8_t *ssl_sessionid = ssl_get_session_id_size(ssl);

    size_t ssl_idsize = ssl_get_session_id(ssl);

    Curl_ssl_sessionid_lock(conn);

    if(Curl_ssl_addsessionid(conn, (void *) ssl_sessionid, ssl_idsize)

       != CURLE_OK)

      infof (data, "failed to add session to cache\n");

    Curl_ssl_sessionid_unlock(conn);

  }



  return CURLE_OK;

}

lib/vtls/cyassl.c

+39 −32
Original line number Diff line number Diff line
@@ -137,7 +137,6 @@ cyassl_connect_step1(struct connectdata *conn, 
  struct SessionHandle *data = conn->data;

  struct ssl_connect_data* conssl = &conn->ssl[sockindex];

  SSL_METHOD* req_method = NULL;

  void* ssl_sessionid = NULL;

  curl_socket_t sockfd = conn->sock[sockindex];

#ifdef HAVE_SNI

  bool sni = FALSE;
@@ -378,19 +377,24 @@ cyassl_connect_step1(struct connectdata *conn, 
#endif /* HAVE_ALPN */



  /* Check if there's a cached ID we can/should use here! */

  if(conn->ssl_config.sessionid) {

    void *ssl_sessionid = NULL;



    Curl_ssl_sessionid_lock(conn);

    if(!Curl_ssl_getsessionid(conn, &ssl_sessionid, NULL)) {

      /* we got a session id, use it! */

      if(!SSL_set_session(conssl->handle, ssl_sessionid)) {

        Curl_ssl_sessionid_unlock(conn);

        failf(data, "SSL: SSL_set_session failed: %s",

            ERR_error_string(SSL_get_error(conssl->handle, 0), error_buffer));

              ERR_error_string(SSL_get_error(conssl->handle, 0),

              error_buffer));

        return CURLE_SSL_CONNECT_ERROR;

      }

      /* Informational message */

      infof (data, "SSL re-using session ID\n");

    }

    Curl_ssl_sessionid_unlock(conn);

  }



  /* pass the raw socket into the SSL layer */

  if(!SSL_set_fd(conssl->handle, (int)sockfd)) {
@@ -574,14 +578,16 @@ cyassl_connect_step3(struct connectdata *conn, 
                     int sockindex)

{

  CURLcode result = CURLE_OK;

  void *old_ssl_sessionid=NULL;

  struct SessionHandle *data = conn->data;

  struct ssl_connect_data *connssl = &conn->ssl[sockindex];

  bool incache;

  SSL_SESSION *our_ssl_sessionid;



  DEBUGASSERT(ssl_connect_3 == connssl->connecting_state);



  if(conn->ssl_config.sessionid) {

    bool incache;

    SSL_SESSION *our_ssl_sessionid;

    void *old_ssl_sessionid = NULL;



    our_ssl_sessionid = SSL_get_session(connssl->handle);



    Curl_ssl_sessionid_lock(conn);
@@ -604,6 +610,7 @@ cyassl_connect_step3(struct connectdata *conn, 
      }

    }

    Curl_ssl_sessionid_unlock(conn);

  }



  connssl->connecting_state = ssl_connect_done;

lib/vtls/darwinssl.c

+37 −34
Original line number Diff line number Diff line
@@ -1009,8 +1009,6 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn, 
#endif /* ENABLE_IPV6 */

  size_t all_ciphers_count = 0UL, allowed_ciphers_count = 0UL, i;

  SSLCipherSuite *all_ciphers = NULL, *allowed_ciphers = NULL;

  char *ssl_sessionid;

  size_t ssl_sessionid_len;

  OSStatus err = noErr;

#if CURL_BUILD_MAC

  int darwinver_maj = 0, darwinver_min = 0;
@@ -1474,6 +1472,10 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn, 
#endif /* CURL_BUILD_MAC_10_9 || CURL_BUILD_IOS_7 */



  /* Check if there's a cached ID we can/should use here! */

  if(conn->ssl_config.sessionid) {

    char *ssl_sessionid;

    size_t ssl_sessionid_len;



    Curl_ssl_sessionid_lock(conn);

    if(!Curl_ssl_getsessionid(conn, (void **)&ssl_sessionid,

                              &ssl_sessionid_len)) {
@@ -1511,6 +1513,7 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn, 
        return result;

      }

    }

  }



  err = SSLSetIOFuncs(connssl->ssl_ctx, SocketRead, SocketWrite);

  if(err != noErr) {

lib/vtls/gtls.c

+14 −12
Original line number Diff line number Diff line
@@ -370,8 +370,6 @@ gtls_connect_step1(struct connectdata *conn, 
  struct SessionHandle *data = conn->data;

  gnutls_session_t session;

  int rc;

  void *ssl_sessionid;

  size_t ssl_idsize;

  bool sni = TRUE; /* default is SNI enabled */

#ifdef ENABLE_IPV6

  struct in6_addr addr;
@@ -749,6 +747,9 @@ gtls_connect_step1(struct connectdata *conn, 


  /* This might be a reconnect, so we check for a session ID in the cache

     to speed up things */

  if(conn->ssl_config.sessionid) {

    void *ssl_sessionid;

    size_t ssl_idsize;



    Curl_ssl_sessionid_lock(conn);

    if(!Curl_ssl_getsessionid(conn, &ssl_sessionid, &ssl_idsize)) {
@@ -759,6 +760,7 @@ gtls_connect_step1(struct connectdata *conn, 
      infof (data, "SSL re-using session ID\n");

    }

    Curl_ssl_sessionid_unlock(conn);

  }



  return CURLE_OK;

}
@@ -841,8 +843,6 @@ gtls_connect_step3(struct connectdata *conn, 
  struct SessionHandle *data = conn->data;

  gnutls_session_t session = conn->ssl[sockindex].session;

  int rc;

  bool incache;

  void *ssl_sessionid;

#ifdef HAS_ALPN

  gnutls_datum_t proto;

#endif
@@ -1270,11 +1270,13 @@ gtls_connect_step3(struct connectdata *conn, 
  conn->recv[sockindex] = gtls_recv;

  conn->send[sockindex] = gtls_send;



  {

  if(conn->ssl_config.sessionid) {

    /* we always unconditionally get the session id here, as even if we

       already got it from the cache and asked to use it in the connection, it

       might've been rejected and then a new one is in use now and we need to

       detect that. */

    bool incache;

    void *ssl_sessionid;

    void *connect_sessionid;

    size_t connect_idsize = 0;

lib/vtls/mbedtls.c

+40 −32
Original line number Diff line number Diff line
@@ -162,7 +162,6 @@ mbed_connect_step1(struct connectdata *conn, 
  struct ssl_connect_data* connssl = &conn->ssl[sockindex];



  int ret = -1;

  void *old_session = NULL;

  char errorbuf[128];

  errorbuf[0]=0;
@@ -365,6 +364,11 @@ mbed_connect_step1(struct connectdata *conn, 


  mbedtls_ssl_conf_ciphersuites(&connssl->config,

                                mbedtls_ssl_list_ciphersuites());



  /* Check if there's a cached ID we can/should use here! */

  if(conn->ssl_config.sessionid) {

    void *old_session = NULL;



    Curl_ssl_sessionid_lock(conn);

    if(!Curl_ssl_getsessionid(conn, &old_session, NULL)) {

      ret = mbedtls_ssl_set_session(&connssl->ssl, old_session);
@@ -376,6 +380,7 @@ mbed_connect_step1(struct connectdata *conn, 
      infof(data, "mbedTLS re-using session\n");

    }

    Curl_ssl_sessionid_unlock(conn);

  }



  mbedtls_ssl_conf_ca_chain(&connssl->config,

                            &connssl->cacert,
@@ -591,12 +596,14 @@ mbed_connect_step3(struct connectdata *conn, 
  CURLcode retcode = CURLE_OK;

  struct ssl_connect_data *connssl = &conn->ssl[sockindex];

  struct SessionHandle *data = conn->data;

  void *old_ssl_sessionid = NULL;

  mbedtls_ssl_session *our_ssl_sessionid;

  int ret;



  DEBUGASSERT(ssl_connect_3 == connssl->connecting_state);



  if(conn->ssl_config.sessionid) {

    int ret;

    mbedtls_ssl_session *our_ssl_sessionid;

    void *old_ssl_sessionid = NULL;



    our_ssl_sessionid = malloc(sizeof(mbedtls_ssl_session));

    if(!our_ssl_sessionid)

      return CURLE_OUT_OF_MEMORY;
@@ -621,6 +628,7 @@ mbed_connect_step3(struct connectdata *conn, 
      failf(data, "failed to store ssl session");

      return retcode;

    }

  }



  connssl->connecting_state = ssl_connect_done;