CVE-2021-25940: Insufficient Session Expiration (CWE-613)
value: In ArangoDB, versions v3.7.6 through v3.8.3 are vulnerable to Insufficient Session Expiration. When a user’s password is changed by the administrator, the session isn’t invalidated, allowing a malicious user to still be logged in and perform arbitrary actions within the system.
CWE description: According to WASC, "Insufficient Session Expiration is when a web site permits an attacker to reuse old session credentials or session IDs for authorization."
CVSS v3 base score: 8
Published: 2021-11-16T10:15Z
Repository: https://github.com/arangodb/arangodb
Language: C++
Files changed: CHANGELOG, arangod/GeneralServer/AuthenticationFeature.cpp, arangod/GeneralServer/AuthenticationFeature.h, arangod/RestHandler/RestAuthHandler.cpp, arangod/RestHandler/RestAuthHandler.h, arangod/RestServer/ServerFeature.cpp, tests/js/client/server_parameters/test-server-session-timeout.js
Commit date: 2021-07-01 11:36:14 +0200
Commit message: Revive startup parameter `--server.session-timeout` (#14118)

Co-authored-by: Tobias Gödderz <tobias@arangodb.com>
Diff: diff --git a/CHANGELOG b/CHANGELOG
index cfbc6356f4..087ba1f529 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,8 +1,15 @@
 devel
 -----
 
+* Revive startup parameter `--server.session-timeout` to control the timeout
+  for web interface sessions and other sessions that are based on JWTs created
+  by the `/_open/auth` API.
+  
+  This PR also changes the default session timeout for web interface sessions
+  to one hour. Older versions of ArangoDB had longer session timeouts.
+
 * Add prefix parameter to LEVENSHTEIN_MATCH function in ArangoSearch
 
 * Removed redirects from /_admin/cluster* to /_admin/cluster/*. Adjusted
   internal requests to use the new url.
 
diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp
index 0caba2962b..f1b1860695 100644
--- a/arangod/GeneralServer/AuthenticationFeature.cpp
+++ b/arangod/GeneralServer/AuthenticationFeature.cpp
@@ -55,11 +55,12 @@ AuthenticationFeature::AuthenticationFeature(application_features::ApplicationSe
       _authCache(nullptr),
       _authenticationUnixSockets(true),
       _authenticationSystemOnly(true),
       _localAuthentication(true),
       _active(true),
-      _authenticationTimeout(0.0) {
+      _authenticationTimeout(0.0),
+      _sessionTimeout(static_cast<double>(1 * std::chrono::hours(1) / std::chrono::seconds(1))) { // 1 hour
   setOptional(false);
   startsAfter<application_features::BasicFeaturePhaseServer>();
 
 #ifdef USE_ENTERPRISE
   startsAfter<LdapFeature>();
@@ -87,27 +88,43 @@ void AuthenticationFeature::collectOptions(std::shared_ptr<ProgramOptions> optio
 
   options->addOption(
       "--server.authentication-timeout",
       "timeout for the authentication cache in seconds (0 = indefinitely)",
       new DoubleParameter(&_authenticationTimeout));
+  
+  options->addOption("--server.session-timeout",
+                     "timeout in seconds for web interface JWT sessions",
+                     new DoubleParameter(&_sessionTimeout),
+                     arangodb::options::makeFlags(
+                       arangodb::options::Flags::DefaultNoComponents,
+                       arangodb::options::Flags::OnCoordinator,
+                       arangodb::options::Flags::OnSingle))
+                     .setIntroducedIn(30900);
 
   options->addOption("--server.local-authentication",
                      "enable authentication using the local user database",
-                     new BooleanParameter(&_localAuthentication));
+                     new BooleanParameter(&_localAuthentication),
+                     arangodb::options::makeFlags(
+                       arangodb::options::Flags::DefaultNoComponents,
+                       arangodb::options::Flags::OnCoordinator,
+                       arangodb::options::Flags::OnSingle));
 
   options->addOption(
       "--server.authentication-system-only",
       "use HTTP authentication only for requests to /_api and /_admin",
       new BooleanParameter(&_authenticationSystemOnly));
 
 #ifdef ARANGODB_HAVE_DOMAIN_SOCKETS
   options->addOption("--server.authentication-unix-sockets",
                      "authentication for requests via UNIX domain sockets",
-                     new BooleanParameter(&_authenticationUnixSockets));
+                     new BooleanParameter(&_authenticationUnixSockets),
+                     arangodb::options::makeFlags(
+                       arangodb::options::Flags::DefaultNoOs,
+                       arangodb::options::Flags::OsLinux,
+                       arangodb::options::Flags::OsMac));
 #endif
 
-  // Maybe deprecate this option in devel
   options
       ->addOption("--server.jwt-secret",
                   "secret to use when doing jwt authentication",
                   new StringParameter(&_jwtSecretProgramOption))
       .setDeprecatedIn(30322)
@@ -149,10 +166,16 @@ void AuthenticationFeature::validateOptions(std::shared_ptr<ProgramOptions> opti
       LOG_TOPIC("9abfc", FATAL, arangodb::Logger::STARTUP)
           << "Given JWT secret too long. Max length is " << _maxSecretLength;
       FATAL_ERROR_EXIT();
     }
   }
+  
+  if (_sessionTimeout <= 1.0) {
+    LOG_TOPIC("85046", FATAL, arangodb::Logger::AUTHENTICATION)
+        << "--server.session-timeout has an invalid value: " << _sessionTimeout;
+    FATAL_ERROR_EXIT();
+  }
 
   if (options->processingResult().touched("server.jwt-secret")) {
     LOG_TOPIC("1aaae", WARN, arangodb::Logger::AUTHENTICATION)
         << "--server.jwt-secret is insecure. Use --server.jwt-secret-keyfile "
            "instead.";
diff --git a/arangod/GeneralServer/AuthenticationFeature.h b/arangod/GeneralServer/AuthenticationFeature.h
index 52708808af..f947d342c5 100644
--- a/arangod/GeneralServer/AuthenticationFeature.h
+++ b/arangod/GeneralServer/AuthenticationFeature.h
@@ -70,10 +70,12 @@ class AuthenticationFeature final : public application_features::ApplicationFeat
   std::string jwtActiveSecret() const;
 #ifdef USE_ENTERPRISE
   /// verification only secrets
   std::pair<std::string, std::vector<std::string>> jwtSecrets() const;
 #endif
+
+  double sessionTimeout() const { return _sessionTimeout; }
   
   // load secrets from file(s)
   [[nodiscard]] Result loadJwtSecretsFromFile();
 
  private:
@@ -89,10 +91,11 @@ class AuthenticationFeature final : public application_features::ApplicationFeat
   bool _authenticationUnixSockets;
   bool _authenticationSystemOnly;
   bool _localAuthentication;
   bool _active;
   double _authenticationTimeout;
+  double _sessionTimeout;
   
   mutable std::mutex _jwtSecretsLock;
 
   std::string _jwtSecretProgramOption;
   std::string _jwtSecretKeyfileProgramOption;
diff --git a/arangod/RestHandler/RestAuthHandler.cpp b/arangod/RestHandler/RestAuthHandler.cpp
index b402272447..97d0ca184c 100644
--- a/arangod/RestHandler/RestAuthHandler.cpp
+++ b/arangod/RestHandler/RestAuthHandler.cpp
@@ -25,33 +25,25 @@
 
 #include <fuerte/jwt.h>
 #include <velocypack/Builder.h>
 #include <velocypack/velocypack-aliases.h>
 
+#include "Basics/ScopeGuard.h"
 #include "Basics/StringUtils.h"
 #include "GeneralServer/AuthenticationFeature.h"
 #include "Logger/LogMacros.h"
 #include "Logger/Logger.h"
 #include "Logger/LoggerStream.h"
-#include "Ssl/SslInterface.h"
 #include "Utils/Events.h"
 
 using namespace arangodb;
 using namespace arangodb::basics;
 using namespace arangodb::rest;
 
 RestAuthHandler::RestAuthHandler(application_features::ApplicationServer& server,
                                  GeneralRequest* request, GeneralResponse* response)
-    : RestVocbaseBaseHandler(server, request, response),
-      _validFor(60 * 60 * 24 * 30) {}
-
-std::string RestAuthHandler::generateJwt(std::string const& username,
-                                         std::string const& password) {
-  AuthenticationFeature* af = AuthenticationFeature::instance();
-  TRI_ASSERT(af != nullptr);
-  return fuerte::jwt::generateUserToken(af->tokenCache().jwtSecret(), username, _validFor);
-}
+    : RestVocbaseBaseHandler(server, request, response) {}
 
 RestStatus RestAuthHandler::execute() {
   auto const type = _request->requestType();
   if (type != rest::RequestType::POST) {
     generateError(rest::ResponseCode::METHOD_NOT_ALLOWED, TRI_ERROR_HTTP_METHOD_NOT_ALLOWED);
@@ -73,48 +65,58 @@ RestStatus RestAuthHandler::execute() {
 
   if (!usernameSlice.isString() || !passwordSlice.isString()) {
     return badRequest();
   }
 
-  _username = usernameSlice.copyString();
+  std::string const username = usernameSlice.copyString();
   std::string const password = passwordSlice.copyString();
 
+  bool isValid = false;
+
+  auto guard = scopeGuard([&]() {
+    try {
+      if (isValid) {
+        events::LoggedIn(*_request, username);
+      } else {
+        events::CredentialsBad(*_request, username);
+      }
+    } catch (...) {
+      // nothing we can do
+    }
+  });
+  
   auth::UserManager* um = AuthenticationFeature::instance()->userManager();
   if (um == nullptr) {
     std::string msg = "This server does not support users";
     LOG_TOPIC("2e7d4", ERR, Logger::AUTHENTICATION) << msg;
     generateError(rest::ResponseCode::UNAUTHORIZED, TRI_ERROR_HTTP_UNAUTHORIZED, msg);
-  } else if (um->checkPassword(_username, password)) {
+  } else if (um->checkPassword(username, password)) {
     VPackBuilder resultBuilder;
     {
       VPackObjectBuilder b(&resultBuilder);
-      std::string jwt = generateJwt(_username, password);
-      resultBuilder.add("jwt", VPackValue(jwt));
+      resultBuilder.add("jwt", VPackValue(generateJwt(username)));
     }
 
-    _isValid = true;
+    isValid = true;
     generateDocument(resultBuilder.slice(), true, &VPackOptions::Defaults);
   } else {
     // mop: rfc 2616 10.4.2 (if credentials wrong 401)
     generateError(rest::ResponseCode::UNAUTHORIZED, TRI_ERROR_HTTP_UNAUTHORIZED,
                   "Wrong credentials");
   }
   return RestStatus::DONE;
 }
 
+std::string RestAuthHandler::generateJwt(std::string const& username) const {
+  AuthenticationFeature* af = AuthenticationFeature::instance();
+  TRI_ASSERT(af != nullptr);
+  return fuerte::jwt::generateUserToken(
+      af->tokenCache().jwtSecret(), 
+      username, 
+      std::chrono::seconds(uint64_t(af->sessionTimeout())));
+}
+
 RestStatus RestAuthHandler::badRequest() {
   generateError(rest::ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
                 "invalid JSON");
   return RestStatus::DONE;
 }
-
-void RestAuthHandler::shutdownExecute(bool isFinalized) noexcept {
-  try {
-    if (_isValid) {
-      events::LoggedIn(*_request, _username);
-    } else {
-      events::CredentialsBad(*_request, _username);
-    }
-  } catch (...) {
-  }
-  RestVocbaseBaseHandler::shutdownExecute(isFinalized);
-}
diff --git a/arangod/RestHandler/RestAuthHandler.h b/arangod/RestHandler/RestAuthHandler.h
index 34c4578fd7..b500cfeb21 100644
--- a/arangod/RestHandler/RestAuthHandler.h
+++ b/arangod/RestHandler/RestAuthHandler.h
@@ -31,23 +31,16 @@
 namespace arangodb {
 class RestAuthHandler : public RestVocbaseBaseHandler {
  public:
   RestAuthHandler(application_features::ApplicationServer&, GeneralRequest*, GeneralResponse*);
 
-  std::string generateJwt(std::string const&, std::string const&);
-
  public:
   char const* name() const override final { return "RestAuthHandler"; }
   RequestLane lane() const override final { return RequestLane::CLIENT_SLOW; }
   RestStatus execute() override;
-  void shutdownExecute(bool isFinalized) noexcept override;
 
  private:
+  std::string generateJwt(std::string const& username) const;
   RestStatus badRequest();
-
- private:
-  std::string _username;
-  bool _isValid = false;
-  std::chrono::seconds _validFor;
 };
 }  // namespace arangodb
 
diff --git a/arangod/RestServer/ServerFeature.cpp b/arangod/RestServer/ServerFeature.cpp
index 709a28d752..93c5c875c7 100644
--- a/arangod/RestServer/ServerFeature.cpp
+++ b/arangod/RestServer/ServerFeature.cpp
@@ -101,14 +101,10 @@ void ServerFeature::collectOptions(std::shared_ptr<ProgramOptions> options) {
   // add several obsoleted options here
   options->addSection("vst", "VelocyStream protocol", "", true, true);
   options->addObsoleteOption("--vst.maxsize", "maximal size (in bytes) "
                              "for a VelocyPack chunk", true);
   
-  options->addObsoleteOption(
-      "--server.session-timeout",
-      "timeout of web interface server sessions (in seconds)", true);
-
   // add obsolete MMFiles WAL options (obsoleted in 3.7)
   options->addSection("wal", "WAL of the MMFiles engine", "", true, true);
   options->addObsoleteOption("--wal.allow-oversize-entries",
                              "allow entries that are bigger than '--wal.logfile-size'", false);
   options->addObsoleteOption("--wal.use-mlock",
diff --git a/tests/js/client/server_parameters/test-server-session-timeout.js b/tests/js/client/server_parameters/test-server-session-timeout.js
new file mode 100644
index 0000000000..1721c6833d
--- /dev/null
+++ b/tests/js/client/server_parameters/test-server-session-timeout.js
@@ -0,0 +1,88 @@
+/*jshint globalstrict:false, strict:false */
+/* global getOptions, assertEqual, arango */
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test for server parameters
+///
+/// DISCLAIMER
+///
+/// Copyright 2010-2012 triagens GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB Inc, Cologne, Germany
+///
+/// @author Jan Steemann
+/// @author Copyright 2019, ArangoDB Inc, Cologne, Germany
+////////////////////////////////////////////////////////////////////////////////
+
+if (getOptions === true) {
+  return {
+    'server.session-timeout': '5',
+    'server.authentication': 'true',
+    'server.jwt-secret': 'haxxmann',
+  };
+}
+const jsunity = require('jsunity');
+const request = require('@arangodb/request');
+
+function testSuite() {
+  let baseUrl = function () {
+    return arango.getEndpoint().replace(/^tcp:/, 'http:').replace(/^ssl:/, 'https:');
+  };
+
+  return {
+    testSessionTimeout: function() {
+      let result = request.get(baseUrl() + "/_api/version");
+      // no access
+      assertEqual(401, result.statusCode);
+
+      result = request.post({
+        url: baseUrl() + "/_open/auth", 
+        body: {
+          username: "root",
+          password: ""
+        },
+        json: true
+      });
+
+      assertEqual(200, result.statusCode);
+      const jwt = result.json.jwt;
+      
+      result = request.get({
+        url: baseUrl() + "/_api/version",
+        auth: {
+          bearer: jwt,
+        }
+      });
+
+      // access granted
+      assertEqual(200, result.statusCode);
+
+      require("internal").sleep(7);
+
+      result = request.get({
+        url: baseUrl() + "/_api/version",
+        auth: {
+          bearer: jwt,
+        }
+      });
+
+      // JWT expired
+      assertEqual(401, result.statusCode);
+    },
+  };
+}
+
+jsunity.run(testSuite);
+return jsunity.done();

Vulnerable code:       _authenticationTimeout(0.0) {
                     new BooleanParameter(&_localAuthentication));
                     new BooleanParameter(&_authenticationUnixSockets));
  // Maybe deprecate this option in devel
#include "Ssl/SslInterface.h"
    : RestVocbaseBaseHandler(server, request, response),
      _validFor(60 * 60 * 24 * 30) {}

std::string RestAuthHandler::generateJwt(std::string const& username,
                                         std::string const& password) {
  AuthenticationFeature* af = AuthenticationFeature::instance();
  TRI_ASSERT(af != nullptr);
  return fuerte::jwt::generateUserToken(af->tokenCache().jwtSecret(), username, _validFor);
}
  _username = usernameSlice.copyString();
  } else if (um->checkPassword(_username, password)) {
      std::string jwt = generateJwt(_username, password);
      resultBuilder.add("jwt", VPackValue(jwt));
    _isValid = true;

void RestAuthHandler::shutdownExecute(bool isFinalized) noexcept {
  try {
    if (_isValid) {
      events::LoggedIn(*_request, _username);
    } else {
      events::CredentialsBad(*_request, _username);
    }
  } catch (...) {
  }
  RestVocbaseBaseHandler::shutdownExecute(isFinalized);
}
  std::string generateJwt(std::string const&, std::string const&);

  void shutdownExecute(bool isFinalized) noexcept override;

 private:
  std::string _username;
  bool _isValid = false;
  std::chrono::seconds _validFor;
  options->addObsoleteOption(
      "--server.session-timeout",
      "timeout of web interface server sessions (in seconds)", true);

Fixed code: * Revive startup parameter `--server.session-timeout` to control the timeout
  for web interface sessions and other sessions that are based on JWTs created
  by the `/_open/auth` API.
  
  This PR also changes the default session timeout for web interface sessions
  to one hour. Older versions of ArangoDB had longer session timeouts.

      _authenticationTimeout(0.0),
      _sessionTimeout(static_cast<double>(1 * std::chrono::hours(1) / std::chrono::seconds(1))) { // 1 hour
  
  options->addOption("--server.session-timeout",
                     "timeout in seconds for web interface JWT sessions",
                     new DoubleParameter(&_sessionTimeout),
                     arangodb::options::makeFlags(
                       arangodb::options::Flags::DefaultNoComponents,
                       arangodb::options::Flags::OnCoordinator,
                       arangodb::options::Flags::OnSingle))
                     .setIntroducedIn(30900);
                     new BooleanParameter(&_localAuthentication),
                     arangodb::options::makeFlags(
                       arangodb::options::Flags::DefaultNoComponents,
                       arangodb::options::Flags::OnCoordinator,
                       arangodb::options::Flags::OnSingle));
                     new BooleanParameter(&_authenticationUnixSockets),
                     arangodb::options::makeFlags(
                       arangodb::options::Flags::DefaultNoOs,
                       arangodb::options::Flags::OsLinux,
                       arangodb::options::Flags::OsMac));
  
  if (_sessionTimeout <= 1.0) {
    LOG_TOPIC("85046", FATAL, arangodb::Logger::AUTHENTICATION)
        << "--server.session-timeout has an invalid value: " << _sessionTimeout;
    FATAL_ERROR_EXIT();
  }

  double sessionTimeout() const { return _sessionTimeout; }
  double _sessionTimeout;
#include "Basics/ScopeGuard.h"
    : RestVocbaseBaseHandler(server, request, response) {}
  std::string const username = usernameSlice.copyString();
  bool isValid = false;

  auto guard = scopeGuard([&]() {
    try {
      if (isValid) {
        events::LoggedIn(*_request, username);
      } else {
        events::CredentialsBad(*_request, username);
      }
    } catch (...) {
      // nothing we can do
    }
  });
  
  } else if (um->checkPassword(username, password)) {
      resultBuilder.add("jwt", VPackValue(generateJwt(username)));
    isValid = true;
std::string RestAuthHandler::generateJwt(std::string const& username) const {
  AuthenticationFeature* af = AuthenticationFeature::instance();
  TRI_ASSERT(af != nullptr);
  return fuerte::jwt::generateUserToken(
      af->tokenCache().jwtSecret(), 
      username, 
      std::chrono::seconds(uint64_t(af->sessionTimeout())));
}

  std::string generateJwt(std::string const& username) const;
/*jshint globalstrict:false, strict:false */
/* global getOptions, assertEqual, arango */

////////////////////////////////////////////////////////////////////////////////
/// @brief test for server parameters
///
/// DISCLAIMER
///
/// Copyright 2010-2012 triagens GmbH, Cologne, Germany
///
/// Licensed under the Apache License, Version 2.0 (the "License");
/// you may not use this file except in compliance with the License.
/// You may obtain a copy of the License at
///
///     http://www.apache.org/licenses/LICENSE-2.0
///
/// Unless required by applicable law or agreed to in writing, software
/// distributed under the License is distributed on an "AS IS" BASIS,
/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
/// See the License for the specific language governing permissions and
/// limitations under the License.
///
/// Copyright holder is ArangoDB Inc, Cologne, Germany
///
/// @author Jan Steemann
/// @author Copyright 2019, ArangoDB Inc, Cologne, Germany
////////////////////////////////////////////////////////////////////////////////

if (getOptions === true) {
  return {
    'server.session-timeout': '5',
    'server.authentication': 'true',
    'server.jwt-secret': 'haxxmann',
  };
}
const jsunity = require('jsunity');
const request = require('@arangodb/request');

function testSuite() {
  let baseUrl = function () {
    return arango.getEndpoint().replace(/^tcp:/, 'http:').replace(/^ssl:/, 'https:');
  };

  return {
    testSessionTimeout: function() {
      let result = request.get(baseUrl() + "/_api/version");
      // no access
      assertEqual(401, result.statusCode);

      result = request.post({
        url: baseUrl() + "/_open/auth", 
        body: {
          username: "root",
          password: ""
        },
        json: true
      });

      assertEqual(200, result.statusCode);
      const jwt = result.json.jwt;
      
      result = request.get({
        url: baseUrl() + "/_api/version",
        auth: {
          bearer: jwt,
        }
      });

      // access granted
      assertEqual(200, result.statusCode);

      require("internal").sleep(7);

      result = request.get({
        url: baseUrl() + "/_api/version",
        auth: {
          bearer: jwt,
        }
      });

      // JWT expired
      assertEqual(401, result.statusCode);
    },
  };
}

jsunity.run(testSuite);
return jsunity.done();