Uploaded image for project: 'Traffic Server'
  1. Traffic Server
  2. TS-1883

TLS origin connections do not support connection timeouts

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Not A Problem
    • None
    • 7.0.0
    • Core, SSL
    • None

    Description

      In proxy/http/HttpSM.cc, we can see that origin connections do not support timeouts if the scheme is HTTPS:

      void
      HttpSM::do_http_server_open(bool raw)
      {
      ...
        if (t_state.scheme == URL_WKSIDX_HTTPS) {
          DebugSM("http", "calling sslNetProcessor.connect_re");
          connect_action_handle = sslNetProcessor.connect_re(this,    // state machine
                                                             &t_state.current.server->addr.sa,    // addr + port
                                                             &opt);
        } else {
      ...
            // Setup the timeouts
            // Set the inactivity timeout to the connect timeout so that we
            //   we fail this server if it doesn't start sending the response
            //   header
            MgmtInt connect_timeout;
            if (t_state.method == HTTP_WKSIDX_POST || t_state.method == HTTP_WKSIDX_PUT) {
              connect_timeout = t_state.txn_conf->post_connect_attempts_timeout;
            } else if (t_state.current.server == &t_state.parent_info) {
              connect_timeout = t_state.http_config_param->parent_connect_timeout;
            } else {
              if (t_state.pCongestionEntry != NULL)
                connect_timeout = t_state.pCongestionEntry->connect_timeout();
              else
                connect_timeout = t_state.txn_conf->connect_attempts_timeout;
            }
            DebugSM("http", "calling netProcessor.connect_s");
            connect_action_handle = netProcessor.connect_s(this,      // state machine
                                                           &t_state.current.server->addr.sa,    // addr + port
                                                           connect_timeout, &opt);
      ...
        }
      
      

      Attachments

        Issue Links

          Activity

            zwoop Leif Hedstrom added a comment -

            Moving back to v3.3.5 for now.

            zwoop Leif Hedstrom added a comment - Moving back to v3.3.5 for now.
            zwoop Leif Hedstrom added a comment -

            Moving out to v3.3.6 for now, which means unless someone moves it back to v3.3.5, this will not go into v3.4.0.

            zwoop Leif Hedstrom added a comment - Moving out to v3.3.6 for now, which means unless someone moves it back to v3.3.5, this will not go into v3.4.0.
            zwoop Leif Hedstrom added a comment - Moving to 4.2.0 as per https://cwiki.apache.org/confluence/display/TS/New+Release+Processes
            shinrich Susan Hinrichs added a comment - - edited

            Actually, if we look at the do_http_server_open() code in 5.x more closely, we see that only the CONNECT method will set up the timeouts here. See the code snippet below with some extra SKH comments.

            It appears for the other methods, attach_server_session() sets up an inactivity timeout to enforce the connect timeout. This appears to hold for both http and https (if we are proxying the https). Verified by examining the code and setting break points while passing through requests.

            I'm guessing that this code has evolved since it was reported in 3.x, and was fixed along the way.

            In the non-proxy case, the SSL logic does not go through any of this. But I am assuming that this bug is concerning itself only with the proxied SSL connections.

              if (scheme_to_use == URL_WKSIDX_HTTPS) {
                DebugSM("http", "calling sslNetProcessor.connect_re");
                int len = 0;
                const char * host = t_state.hdr_info.server_request.host_get(&len);
                opt.set_sni_servername(host, len);
                connect_action_handle = sslNetProcessor.connect_re(this,    // state machine
                                                                   &t_state.current.server->addr.sa,    // addr + port
                                                                   &opt);
              } else {
                // SKH - If I'm anything other than a connect method, go ahead and set up the connections
                if (t_state.method != HTTP_WKSIDX_CONNECT) {
                  DebugSM("http", "calling netProcessor.connect_re");
                  connect_action_handle = netProcessor.connect_re(this,     // state machine
                                                                  &t_state.current.server->addr.sa,    // addr + port
                                                                  &opt);
                } else {
                  // Setup the timeouts
                  // Set the inactivity timeout to the connect timeout so that we
                  //   we fail this server if it doesn't start sending the response
                  //   header
                  MgmtInt connect_timeout;
                  // SKH Only t_state.method == HTTP_WKSIDX_CONNECT should get here, so this first case doesn't make any sense
                  // SKH In any case, the connect timeout is only passed into the connect_s code for the method=CONNECT case
                  if (t_state.method == HTTP_WKSIDX_POST || t_state.method == HTTP_WKSIDX_PUT) {
                    connect_timeout = t_state.txn_conf->post_connect_attempts_timeout;
                  } else if (t_state.current.server == &t_state.parent_info) {
                    connect_timeout = t_state.http_config_param->parent_connect_timeout;
                  } else {
                    if (t_state.pCongestionEntry != NULL)
                      connect_timeout = t_state.pCongestionEntry->connect_timeout();
                    else
                      connect_timeout = t_state.txn_conf->connect_attempts_timeout;
                  }
                  DebugSM("http", "calling netProcessor.connect_s");
                  connect_action_handle = netProcessor.connect_s(this,      // state machine
                                                                 &t_state.current.server->addr.sa,    // addr + port
                                                                 connect_timeout, &opt);
                }
              }
            
            shinrich Susan Hinrichs added a comment - - edited Actually, if we look at the do_http_server_open() code in 5.x more closely, we see that only the CONNECT method will set up the timeouts here. See the code snippet below with some extra SKH comments. It appears for the other methods, attach_server_session() sets up an inactivity timeout to enforce the connect timeout. This appears to hold for both http and https (if we are proxying the https). Verified by examining the code and setting break points while passing through requests. I'm guessing that this code has evolved since it was reported in 3.x, and was fixed along the way. In the non-proxy case, the SSL logic does not go through any of this. But I am assuming that this bug is concerning itself only with the proxied SSL connections. if (scheme_to_use == URL_WKSIDX_HTTPS) { DebugSM( "http" , "calling sslNetProcessor.connect_re" ); int len = 0; const char * host = t_state.hdr_info.server_request.host_get(&len); opt.set_sni_servername(host, len); connect_action_handle = sslNetProcessor.connect_re( this , // state machine &t_state.current.server->addr.sa, // addr + port &opt); } else { // SKH - If I'm anything other than a connect method, go ahead and set up the connections if (t_state.method != HTTP_WKSIDX_CONNECT) { DebugSM( "http" , "calling netProcessor.connect_re" ); connect_action_handle = netProcessor.connect_re( this , // state machine &t_state.current.server->addr.sa, // addr + port &opt); } else { // Setup the timeouts // Set the inactivity timeout to the connect timeout so that we // we fail this server if it doesn't start sending the response // header MgmtInt connect_timeout; // SKH Only t_state.method == HTTP_WKSIDX_CONNECT should get here, so this first case doesn't make any sense // SKH In any case , the connect timeout is only passed into the connect_s code for the method=CONNECT case if (t_state.method == HTTP_WKSIDX_POST || t_state.method == HTTP_WKSIDX_PUT) { connect_timeout = t_state.txn_conf->post_connect_attempts_timeout; } else if (t_state.current.server == &t_state.parent_info) { connect_timeout = t_state.http_config_param->parent_connect_timeout; } else { if (t_state.pCongestionEntry != NULL) connect_timeout = t_state.pCongestionEntry->connect_timeout(); else connect_timeout = t_state.txn_conf->connect_attempts_timeout; } DebugSM( "http" , "calling netProcessor.connect_s" ); connect_action_handle = netProcessor.connect_s( this , // state machine &t_state.current.server->addr.sa, // addr + port connect_timeout, &opt); } }
            bcall Bryan Call added a comment - - edited

            shinrich
            Can you please investigate in the next two weeks and see if we need to fix this?

            bcall Bryan Call added a comment - - edited shinrich Can you please investigate in the next two weeks and see if we need to fix this?
            shinrich Susan Hinrichs added a comment -

            I reviewed the code again as part of fixing TS-3046. We do set up inactivity timeouts for both HTTP/HTTPS elsewhere. And as noted we only set timeouts in the case of the CONNECT method here. There are issues with the connect timeout really being a TTFB timeout as described in TS-242, but I don't think anything really needs to be addressed here.

            shinrich Susan Hinrichs added a comment - I reviewed the code again as part of fixing TS-3046 . We do set up inactivity timeouts for both HTTP/HTTPS elsewhere. And as noted we only set timeouts in the case of the CONNECT method here. There are issues with the connect timeout really being a TTFB timeout as described in TS-242 , but I don't think anything really needs to be addressed here.

            People

              shinrich Susan Hinrichs
              jamespeach James Peach
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: