Bound a circuit's reliable list, keep a secure login on TLS, give up on an unanswered local teleport

indra/llcorehttp/_httpoprequest.cpp

@@ -557,6 +557,10 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
         lastModified = (curl_off_t)mReqOptions->getLastModified();
     }
     check_curl_easy_setopt(mCurlHandle, CURLOPT_FOLLOWLOCATION, follow_redirect);
+    if (mReqOptions && mReqOptions->getRedirectsHttpsOnly())
+    {
+        check_curl_easy_setopt(mCurlHandle, CURLOPT_REDIR_PROTOCOLS, long(CURLPROTO_HTTPS));
+    }
 
     check_curl_easy_setopt(mCurlHandle, CURLOPT_SSL_VERIFYPEER, sslPeerV);
     check_curl_easy_setopt(mCurlHandle, CURLOPT_SSL_VERIFYHOST, sslHostV);

indra/llcorehttp/httpoptions.cpp

@@ -44,6 +44,7 @@ HttpOptions::HttpOptions() :
     mMaxRetryBackoff(HTTP_RETRY_BACKOFF_MAX_DEFAULT),
     mUseRetryAfter(HTTP_USE_RETRY_AFTER_DEFAULT),
     mFollowRedirects(true),
+    mRedirectsHttpsOnly(false),
     mVerifyPeer(sDefaultVerifyPeer),
     mVerifyHost(false),
     mDNSCacheTimeout(-1L),
@@ -105,6 +106,11 @@ void HttpOptions::setFollowRedirects(bool follow_redirect)
     mFollowRedirects = follow_redirect;
 }
 
+void HttpOptions::setRedirectsHttpsOnly(bool https_only)
+{
+    mRedirectsHttpsOnly = https_only;
+}
+
 void HttpOptions::setSSLVerifyPeer(bool verify)
 {
     mVerifyPeer = verify;

indra/llcorehttp/httpoptions.h

@@ -140,6 +140,15 @@ class HttpOptions
         return mFollowRedirects;
     }
 
+    /// Fails the request if a redirect it would follow goes anywhere but
+    /// https, for a request whose body and headers must stay on TLS.
+    /// Default: false
+    void                setRedirectsHttpsOnly(bool https_only);
+    bool                getRedirectsHttpsOnly() const
+    {
+        return mRedirectsHttpsOnly;
+    }
+
     /// Instructs the LLCore::HTTPRequest to verify that the exchanged security
     /// certificate is authentic.
     /// Default: sDefaultVerifyPeer
@@ -201,6 +210,7 @@ class HttpOptions
     HttpTime            mMaxRetryBackoff;
     bool                mUseRetryAfter;
     bool                mFollowRedirects;
+    bool                mRedirectsHttpsOnly;
     bool                mVerifyPeer;
     bool                mVerifyHost;
     int                 mDNSCacheTimeout;

indra/llcorehttp/tests/httprequest_test.cpp

@@ -890,6 +890,102 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET to the peer is a
 }
 
 
+namespace
+{
+
+// Sends a GET to the peer's redirect with these options, waits for its answer,
+// checks it against wanted, and stops the service again.
+void checkRedirectAnswer(HttpRequestTestData & state, const HttpOptions::ptr_t & options, const HttpStatus & wanted)
+{
+    ScopedCurlInit ready;
+
+    std::string url_base(get_base_url());
+
+    TestHandler2 handler(&state, "handler");
+    LLCore::HttpHandler::ptr_t handlerp(&handler, NoOpDeletor);
+    state.mHandlerCalls = 0;
+
+    HttpRequest * req = NULL;
+
+    try
+    {
+        HttpRequest::createService();
+        HttpRequest::startThread();
+        req = new HttpRequest();
+
+        state.mStatus = wanted;
+        HttpHandle handle = req->requestGet(HttpRequest::DEFAULT_POLICY_ID,
+                                            url_base + "redirect/",
+                                            options,
+                                            HttpHeaders::ptr_t(),
+                                            handlerp);
+        REQUIRE(handle != LLCORE_HTTP_HANDLE_INVALID);
+
+        int count(0);
+        int limit(LOOP_COUNT_LONG);
+        while (count++ < limit && state.mHandlerCalls < 1)
+        {
+            req->update(1000000);
+            usleep(LOOP_SLEEP_INTERVAL);
+        }
+        UNSCOPED_INFO("Request executed in reasonable time");
+        CHECK(count < limit);
+        CHECK(state.mHandlerCalls == 1);
+
+        state.mStatus = HttpStatus();
+        handle = req->requestStopThread(handlerp);
+        REQUIRE(handle != LLCORE_HTTP_HANDLE_INVALID);
+
+        count = 0;
+        limit = LOOP_COUNT_LONG;
+        while (count++ < limit && state.mHandlerCalls < 2)
+        {
+            req->update(1000000);
+            usleep(LOOP_SLEEP_INTERVAL);
+        }
+        UNSCOPED_INFO("Second request executed in reasonable time");
+        CHECK(count < limit);
+
+        count = 0;
+        limit = LOOP_COUNT_SHORT;
+        while (count++ < limit && ! HttpService::isStopped())
+        {
+            usleep(LOOP_SLEEP_INTERVAL);
+        }
+        CHECK(HttpService::isStopped());
+
+        delete req;
+        req = NULL;
+
+        HttpRequest::destroyService();
+    }
+    catch (...)
+    {
+        stop_thread(req);
+        delete req;
+        HttpRequest::destroyService();
+        throw;
+    }
+}
+
+}
+
+
+TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET follows a redirect to plain http by default", "[HttpRequest_Tests]")
+{
+    checkRedirectAnswer(*this, std::make_shared<HttpOptions>(), HttpStatus(200));
+}
+
+
+TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET that keeps its redirects on https fails one to plain http", "[HttpRequest_Tests]")
+{
+    HttpOptions::ptr_t options(std::make_shared<HttpOptions>());
+    options->setRedirectsHttpsOnly(true);
+
+    checkRedirectAnswer(*this, options, HttpStatus(HttpStatus::EXT_CURL_EASY, CURLE_UNSUPPORTED_PROTOCOL));
+}
+
+
 TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET with a byte range sends its Range header and takes a 200 for the whole body", "[HttpRequest_Tests]")
 {
     ScopedCurlInit ready;

indra/llcorehttp/tests/test_llcorehttp_peer.py

@@ -76,6 +76,7 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler):
     -- '/503/4/'            "Retry-After: (*#*(@*(@(")"
     -- '/503/5/'            "Retry-After: aklsjflajfaklsfaklfasfklasdfklasdgahsdhgasdiogaioshdgo"
     -- '/503/6/'            "Retry-After: 1 2 3 4 5 6 7 8 9 10"
+    - '/redirect/'      307 response sending the request on to '/'
 
     Some combinations make no sense, there's no effort to protect
     you from that.
@@ -150,7 +151,12 @@ def answer(self, data, withdata=True):
         if "/sleep/" in self.path:
             time.sleep(30)
 
-        if "/503/" in self.path:
+        if "/redirect/" in self.path:
+            self.send_response(307)
+            self.send_header("Location", "/")
+            self.send_header("Content-Length", "0")
+            self.end_headers()
+        elif "/503/" in self.path:
             # Tests for various kinds of 'Retry-After' header parsing
             body = None
             if "/503/0/" in self.path:

indra/llmessage/llcircuit.cpp

@@ -627,6 +627,26 @@ bool LLCircuitData::checkDuplicate(TPACKETID packetnum, bool resent)
 }
 
 
+void LLCircuitData::recordReliablePacket(TPACKETID packetnum)
+{
+    mRecentlyReceivedReliablePackets[packetnum] = LLMessageSystem::getMessageTimeUsecs();
+
+    // Only the peer's pings let the list shrink (clearDuplicateList), so a
+    // peer that sends reliable packets and no pings would grow it without
+    // end. The oldest number is the first past the highest, since numbers
+    // wrap, or the lowest if nothing is past it.
+    if (mRecentlyReceivedReliablePackets.size() > LL_MAX_RECENT_RELIABLE_PACKETS)
+    {
+        packet_time_map::iterator oldest = mRecentlyReceivedReliablePackets.upper_bound(mHighestPacketID);
+        if (oldest == mRecentlyReceivedReliablePackets.end())
+        {
+            oldest = mRecentlyReceivedReliablePackets.begin();
+        }
+        mRecentlyReceivedReliablePackets.erase(oldest);
+    }
+}
+
+
 void LLCircuit::dumpResends()
 {
     circuit_data_map::iterator end = mCircuitData.end();

indra/llmessage/llcircuit.h

@@ -50,6 +50,7 @@ const F32Milliseconds LL_AVERAGED_PING_MIN(100);    // increased to avoid retran
 const U32Milliseconds INITIAL_PING_VALUE_MSEC(1000); // initial value for the ping delay, or for ping delay for an unknown circuit
 
 const TPACKETID LL_MAX_OUT_PACKET_ID = 0x01000000;
+const size_t LL_MAX_RECENT_RELIABLE_PACKETS = 65536; // how many reliable packets a circuit remembers for duplicate suppression
 const int LL_ERR_CIRCUIT_GONE   = -23017;
 const int LL_ERR_TCP_TIMEOUT    = -23016;
 
@@ -171,6 +172,7 @@ class LLCircuitData
     TPACKETID       nextPacketOutID();
     void                setPacketInID(TPACKETID id);
     void                    checkPacketInID(TPACKETID id, bool receive_resent);
+    void            recordReliablePacket(TPACKETID packetnum);  // Remember a reliable packet for duplicate suppression
     void            setPingDelay(U32Milliseconds ping);
     bool            checkCircuitTimeout();  // Return false if the circuit is dead and should be cleaned up
 

indra/llmessage/message.cpp

@@ -744,7 +744,7 @@ bool LLMessageSystem::checkMessages(LockMessageChecker&, S64 frame_count,
                 if (cdp && recv_reliable)
                 {
                     // Add to the recently received list for duplicate suppression
-                    cdp->mRecentlyReceivedReliablePackets[mCurrentRecvPacketID] = getMessageTimeUsecs();
+                    cdp->recordReliablePacket(mCurrentRecvPacketID);
 
                     // Put it onto the list of packets to be acked, unless
                     // it was acked as it came in

indra/llmessage/tests/llcircuit_test.cpp

@@ -204,7 +204,13 @@ namespace ll_test
         static void handled(LLCircuitData& circuit, TPACKETID id)
         {
             circuit.checkPacketInID(id, false);
-            circuit.mRecentlyReceivedReliablePackets[id] = LLMessageSystem::getMessageTimeUsecs();
+            circuit.recordReliablePacket(id);
+        }
+
+        // How many reliable packets the circuit remembers.
+        static size_t recentCount(const LLCircuitData& circuit)
+        {
+            return circuit.mRecentlyReceivedReliablePackets.size();
         }
 
         static bool checkDuplicate(LLCircuitData& circuit, TPACKETID id, bool resent)
@@ -402,6 +408,50 @@ namespace
         CHECK(CircuitProbe::checkDuplicate(circuit, 7, true));
     }
 
+    TEST_CASE("llcircuit: a peer that never pings leaves only the newest reliable packets remembered", "[llcircuit]")
+    {
+        StubMessageSystemScope message_system;
+        LLCircuitData circuit(LLHost(), 0, HEARTBEAT, TIMEOUT);
+
+        const TPACKETID PAST_THE_BOUND = 100;
+        const TPACKETID COUNT = static_cast<TPACKETID>(LL_MAX_RECENT_RELIABLE_PACKETS) + PAST_THE_BOUND;
+        for (TPACKETID id = 0; id < COUNT; ++id)
+        {
+            CircuitProbe::handled(circuit, id);
+        }
+
+        CHECK(CircuitProbe::recentCount(circuit) == LL_MAX_RECENT_RELIABLE_PACKETS);
+        CHECK_FALSE(CircuitProbe::checkDuplicate(circuit, 0, true));
+        CHECK_FALSE(CircuitProbe::checkDuplicate(circuit, PAST_THE_BOUND - 1, true));
+        CHECK(CircuitProbe::checkDuplicate(circuit, PAST_THE_BOUND, true));
+        CHECK(CircuitProbe::checkDuplicate(circuit, COUNT - 1, true));
+    }
+
+    TEST_CASE("llcircuit: the reliable packets forgotten across a wrap are the ones before it", "[llcircuit]")
+    {
+        StubMessageSystemScope message_system;
+        // Enough numbers before the wrap to fill the list, and a few after.
+        const TPACKETID FIRST = LL_MAX_OUT_PACKET_ID - static_cast<TPACKETID>(LL_MAX_RECENT_RELIABLE_PACKETS);
+        LLCircuitData circuit(LLHost(), FIRST, HEARTBEAT, TIMEOUT);
+
+        for (TPACKETID id = FIRST; id < LL_MAX_OUT_PACKET_ID; ++id)
+        {
+            CircuitProbe::handled(circuit, id);
+        }
+        const TPACKETID AFTER_THE_WRAP = 10;
+        for (TPACKETID id = 0; id < AFTER_THE_WRAP; ++id)
+        {
+            CircuitProbe::handled(circuit, id);
+        }
+
+        CHECK(CircuitProbe::recentCount(circuit) == LL_MAX_RECENT_RELIABLE_PACKETS);
+        CHECK(CircuitProbe::checkDuplicate(circuit, 0, true));
+        CHECK(CircuitProbe::checkDuplicate(circuit, AFTER_THE_WRAP - 1, true));
+        CHECK_FALSE(CircuitProbe::checkDuplicate(circuit, FIRST, true));
+        CHECK_FALSE(CircuitProbe::checkDuplicate(circuit, FIRST + AFTER_THE_WRAP - 1, true));
+        CHECK(CircuitProbe::checkDuplicate(circuit, FIRST + AFTER_THE_WRAP, true));
+    }
+
     TEST_CASE("llcircuit: a ping keeps a number handled moments ago that its sender has had acked", "[llcircuit]")
     {
         StubMessageSystemScope message_system;

indra/newview/llagent.cpp

@@ -4218,6 +4218,7 @@ bool LLAgent::teleportCore(bool is_local)
     {
         LL_INFOS("Teleport") << "Setting teleport state to TELEPORT_LOCAL" << LL_ENDL;
         gAgent.setTeleportState( LLAgent::TELEPORT_LOCAL );
+        gTeleportDisplayTimer.reset();
     }
     else
     {

indra/newview/llviewerdisplay.cpp

@@ -295,17 +295,39 @@ void display_stats()
     }
 }
 
-static void update_tp_display(bool minimized)
+// How long a teleport may go unfinished before the viewer gives up on it.
+static F32 teleport_expiry_time()
 {
-    static LLCachedControl<F32> teleport_arrival_delay(gSavedSettings, "TeleportArrivalDelay");
-    static LLCachedControl<F32> teleport_local_delay(gSavedSettings, "TeleportLocalDelay");
-
     S32 attach_count = 0;
     if (isAgentAvatarValid())
     {
         attach_count = gAgentAvatarp->getAttachmentCount();
     }
-    F32 teleport_save_time = TELEPORT_EXPIRY + TELEPORT_EXPIRY_PER_ATTACHMENT * attach_count;
+    return TELEPORT_EXPIRY + TELEPORT_EXPIRY_PER_ATTACHMENT * attach_count;
+}
+
+// A teleport inside the region shows no display until its TeleportLocal comes,
+// so update_tp_display doesn't run for it until then. Gives up on one whose
+// answer never came, since otherwise every later teleport is refused as one
+// made while teleporting.
+static void give_up_on_unanswered_local_teleport()
+{
+    if (!gTeleportDisplay
+        && gAgent.getTeleportState() == LLAgent::TELEPORT_LOCAL
+        && gTeleportDisplayTimer.getElapsedTimeF32() > teleport_expiry_time())
+    {
+        LL_WARNS("Teleport") << "Giving up on a teleport inside the region after "
+                             << gTeleportDisplayTimer.getElapsedTimeF32() << " seconds with no answer" << LL_ENDL;
+        gAgent.setTeleportState(LLAgent::TELEPORT_NONE);
+    }
+}
+
+static void update_tp_display(bool minimized)
+{
+    static LLCachedControl<F32> teleport_arrival_delay(gSavedSettings, "TeleportArrivalDelay");
+    static LLCachedControl<F32> teleport_local_delay(gSavedSettings, "TeleportLocalDelay");
+
+    F32 teleport_save_time = teleport_expiry_time();
     F32 teleport_elapsed = gTeleportDisplayTimer.getElapsedTimeF32();
     F32 teleport_percent = teleport_elapsed * (100.f / teleport_save_time);
     if (gAgent.getTeleportState() != LLAgent::TELEPORT_START && teleport_percent > 100.f)
@@ -526,6 +548,8 @@ void display(bool rebuild, F32 zoom_factor, int subfield, bool for_snapshot)
     }
     gPipeline.applyRenderInputs();
 
+    give_up_on_unanswered_local_teleport();
+
     gSnapshot = for_snapshot;
 
     if (LLPipeline::sRenderDeferred)

indra/newview/llxmlrpctransaction.cpp

@@ -27,6 +27,7 @@
 #include "llviewerprecompiledheaders.h"
 // include this to get winsock2 because openssl attempts to include winsock1
 #include "llwin32headers.h"
+#include <boost/algorithm/string/predicate.hpp>
 #include <openssl/x509_vfy.h>
 #include <openssl/ssl.h>
 #include "llsecapi.h"
@@ -213,6 +214,10 @@ LLXMLRPCTransaction::Impl::Impl
     httpOpts->setSSLVerifyPeer(vefifySSLCert);
     httpOpts->setSSLVerifyHost(vefifySSLCert);
 
+    // The body holds the login's credentials or a payment's details, and a 307
+    // or 308 sends it on as it is.
+    httpOpts->setRedirectsHttpsOnly(boost::algorithm::istarts_with(mURI, "https://"));
+
     // LLRefCounted starts with a 1 ref, so don't add a ref in the smart pointer
     httpHeaders = std::make_shared<LLCore::HttpHeaders>();
 

indra/newview/skins/default/xui/en/strings.xml

@@ -136,6 +136,7 @@ https://www.alchemyviewer.org/downloads</string>
 If you feel this is an error, please contact Support.</string>
 	<string name="LoginIntermediateOptionalUpdateAvailable">Optional viewer update available: [VERSION]</string>
 	<string name="LoginFailedRequiredUpdate">Required viewer update: [VERSION]</string>
+	<string name="LoginFailedInsecureRedirect">The login server tried to send your login to an address that isn't secure, so the login was stopped.</string>
 	<string name="LoginFailedAlreadyLoggedIn">This agent is already logged in.
 </string>
 	<string name="LoginFailedAuthenticationFailed">Sorry! We couldn't log you in.