fix(api): - issue2551063 - Rest/Xmlrpc interfaces needs failed login protection.

Failed API login rate limiting with expiring lockout added.

Author: rouilj

CHANGES.txt

@@ -16,6 +16,10 @@ python 3.6 or newer (3.4/3.5 might work, but they are not tested).
 
 Fixed:
 
+- issue2551063 - Rest/Xmlrpc interfaces needs failed login protection.
+  Failed API login rate limiting with expiring lockout added. (John
+  Rouillard)
+
 Features:
 
 - issue2551103 - add pragma 'display_protected' to roundup-admin. If

doc/rest.txt

@@ -77,6 +77,39 @@ that is not hosted at the same origin as Roundup, you must permit
 the origin using the ``allowed_api_origins`` setting in
 ``config.ini``.
 
+Rate Limiting API Failed Logins
+-------------------------------
+
+To make brute force password guessing harder, the REST API has an
+invalid login rate limiter. It rate limits the number of failed login
+attempts with an invalid user or password. Valid login attempts are
+managed by the normal API rate limiter. The rate limiter is a GCRA
+leaky bucket variant. All APIs (REST/XMLRPC) share the same rate
+limiter. The rate limiter for the HTML/web interface is not shared by
+the API failed login rate limiter.
+
+It is configured by settings in config.ini. Setting
+``api_failed_login_limit`` to a non-zero value enabled the limiter.
+Setting it to 0 disables the limiter (not suggested). If a user fails
+to log in more than ``api_failed_login_limit`` times in
+``api_failed_login_interval_in_sec`` seconds, a 429 HTTP error will be
+returned. The error also tell the user how long they must wait to try
+to log in again.
+
+When a 429 error is returned, the account is locked until enough time
+has passed
+(``api_failed_login_interval_in_sec/api_failed_login_limit`` seconds)
+to make one additional login token available. Any attempt to log in
+while it is locked will fail. This is true even if a the correct
+password is supplied for a locked account. This means a brute force
+attempt can't try more than one password every
+``api_failed_login_interval_in_sec/api_failed_login_limit`` seconds on
+average.
+
+The default values allow up to 4 attempts to login before delaying the
+user by 2.5 minutes (150 seconds). At this time there is no supported
+method to reset the rate limiter.
+
 Rate Limiting the API
 ---------------------
 

doc/upgrading.txt

@@ -92,6 +92,42 @@ Contents:
 
 .. index:: Upgrading; 2.2.0 to 2.3.0
 
+Migrating from 2.3.0 to 2.4.0
+=============================
+
+Update your ``config.ini`` (required)
+-------------------------------------
+
+Upgrade tracker's config.ini file. Use::
+
+  roundup-admin -i /path/to/tracker updateconfig newconfig.ini
+
+to generate a new ini file preserving all your settings.
+You can then merge any local comments from the tracker's
+``config.ini`` to ``newconfig.ini`` and replace
+``config.ini`` with ``newconfig.ini``.
+
+``updateconfig`` will tell you if it is changing old default
+values or if a value must be changed manually.
+
+This will insert the bad API login rate limiting settings.
+
+Bad Login Rate Limiting and Locking (info)
+------------------------------------------
+
+Brute force logins have been rate limited in the HTML web interface
+for a while. This was not the case with the API interfaces.
+
+This release introduces rate limiting for invalid REST or XMLRPC API
+logins.  As with the web interface, users who have hit the rate limit
+have their accounts locked until after the recommended delay time has
+passed.  See `information on configuring the API rate limits`_ for
+details.
+
+.. _`information on configuring the API rate limits`: rest.html#rate-limiting-api-failed-logins
+
+.. index:: Upgrading; 2.2.0 to 2.3.0
+
 Migrating from 2.2.0 to 2.3.0
 =============================
 

doc/xmlrpc.txt

@@ -88,6 +88,14 @@ issues. Patches with tests to roundup to use defusedxml are welcome.
    be passed in cleartext unless the server is proxied behind
    another server (such as Apache or lighttpd) that provides SSL.
 
+Rate Limiting Failed Logins
+---------------------------
+
+See the `rest documentation
+<rest.html#rate-limiting-api-failed-logins>`_ for rate limiting failed
+logins on the API. The XML-RPC uses the same method as the REST API.
+Rate limiting is shared between the XMLRPC and REST APIs.
+
 Client API
 ==========
 The server currently implements seven methods/commands. Each method

roundup/cgi/actions.py

@@ -12,7 +12,7 @@
 from roundup.anypy.strings import StringIO
 from roundup.cgi import exceptions, templating
 from roundup.cgi.timestamp import Timestamped
-from roundup.exceptions import Reject, RejectRaw
+from roundup.exceptions import RateLimitExceeded, Reject, RejectRaw
 from roundup.i18n import _
 from roundup.mailgw import uidFromAddress
 from roundup.rate_limit import Gcra, RateLimit
@@ -26,6 +26,8 @@
 
 
 class Action:
+    loginLimit = None
+
     def __init__(self, client):
         self.client = client
         self.form = client.form
@@ -37,8 +39,6 @@ def __init__(self, client):
         self.base = client.base
         self.user = client.user
         self.context = templating.context(client)
-        self.loginLimit = RateLimit(client.db.config.WEB_LOGIN_ATTEMPTS_MIN,
-                                    timedelta(seconds=60))
 
     def handle(self):
         """Action handler procedure"""
@@ -149,7 +149,7 @@ def examine_url(self, url):
         if not allowed_pattern.match(parsed_url_tuple.fragment):
             raise ValueError(self._("Fragment component (%(url_fragment)s) in %(url)s is not properly escaped") % info)
 
-        return(urllib_.urlunparse(parsed_url_tuple))
+        return urllib_.urlunparse(parsed_url_tuple)
 
     name = ''
     permissionType = None
@@ -1298,36 +1298,6 @@ def handle(self):
                  redirect_url_tuple.fragment))
 
         try:
-            # Implement rate limiting of logins by login name.
-            # Use prefix to prevent key collisions maybe??
-            # set client.db.config.WEB_LOGIN_ATTEMPTS_MIN to 0
-            #  to disable
-            if self.client.db.config.WEB_LOGIN_ATTEMPTS_MIN:  # if 0 - off
-                rlkey = "LOGIN-" + self.client.user
-                limit = self.loginLimit
-                gcra = Gcra()
-                otk = self.client.db.Otk
-                try:
-                    val = otk.getall(rlkey)
-                    gcra.set_tat_as_string(rlkey, val['tat'])
-                except KeyError:
-                    # ignore if tat not set, it's 1970-1-1 by default.
-                    pass
-                # see if rate limit exceeded and we need to reject the attempt
-                reject = gcra.update(rlkey, limit)
-
-                # Calculate a timestamp that will make OTK expire the
-                # unused entry 1 hour in the future
-                ts = otk.lifetime(3600)
-                otk.set(rlkey, tat=gcra.get_tat_as_string(rlkey),
-                        __timestamp=ts)
-                otk.commit()
-
-                if reject:
-                    # User exceeded limits: find out how long to wait
-                    status = gcra.status(rlkey, limit)
-                    raise Reject(_("Logins occurring too fast. Please wait: %s seconds.") % status['Retry-After'])
-
             self.verifyLogin(self.client.user, password)
         except exceptions.LoginError as err:
             self.client.make_user_anonymous()
@@ -1347,6 +1317,28 @@ def handle(self):
                 raise exceptions.Redirect(redirect_url)
             # if no __came_from, send back to base url with error
             return
+        except RateLimitExceeded as err:
+            self.client.make_user_anonymous()
+            for arg in err.args:
+                self.client.add_error_message(arg)
+
+            if '__came_from' in self.form:
+                #  usually web only. If API uses this they will get
+                #  confused as the 429 isn't returned. Without this
+                #  a web user will get redirected back to the home
+                # page rather than stay on the page where they tried
+                # to login.
+                # set a new error message
+                query['@error_message'] = err.args
+                redirect_url = urllib_.urlunparse(
+                    (redirect_url_tuple.scheme,
+                     redirect_url_tuple.netloc,
+                     redirect_url_tuple.path,
+                     redirect_url_tuple.params,
+                     urllib_.urlencode(list(sorted(query.items())), doseq=True),
+                     redirect_url_tuple.fragment))
+                raise exceptions.Redirect(redirect_url)
+            raise
 
         # now we're OK, re-open the database for real, using the user
         self.client.opendb(self.client.user)
@@ -1370,7 +1362,139 @@ def handle(self):
 
             raise exceptions.Redirect(redirect_url)
 
-    def verifyLogin(self, username, password):
+    def rateLimitLogin(self, username, is_api=False, update=True):
+        """Implement rate limiting of logins by login name.
+
+           username - username attempting to log in. May or may not
+                      be valid.
+           is_api - set to False for login via html page
+                    set to "xmlrpc" for xmlrpc api
+                    set to "rest" for rest api
+           update - if False will raise RateLimitExceeded without
+                    updating the stored value. Default is True
+                    which updates stored value. Used to deny access
+                    when user successfully logs in but the user
+                    doesn't have a valid attempt available.
+
+           Login rates for a user are split based on html vs api
+           login.
+
+           Set self.client.db.config.WEB_LOGIN_ATTEMPTS_MIN to 0
+           to disable for web interface. Set
+           self.client.db.config.API_LOGIN_ATTEMPTS to 0
+           to disable for web interface.
+
+           By setting LoginAction.limitLogin, the admin can override
+           the HTML web page rate limiter if they need to change the
+           interval from 1 minute.
+        """
+        config = self.client.db.config
+
+        if not is_api:
+            # HTML web login. Period is fixed at 1 minute.
+            # Override by setting self.loginLimit. Yech.
+            allowed_attempts = config.WEB_LOGIN_ATTEMPTS_MIN
+            per_period = 60
+            rlkey = "LOGIN-" + username
+        else:
+            # api login. Both Rest and XMLRPC use this.
+            allowed_attempts = config.WEB_API_FAILED_LOGIN_LIMIT
+            per_period = config.WEB_API_FAILED_LOGIN_INTERVAL_IN_SEC
+            rlkey = "LOGIN-API" + username
+
+        if not allowed_attempts:  # if allowed_attempt == 0 - off
+            return
+
+        if self.loginLimit and not is_api:
+            # provide a way for user (via interfaces.py) to
+            # change the interval on the html login limit.
+            limit = self.loginLimit
+        else:
+            limit = RateLimit(allowed_attempts,
+                              timedelta(seconds=per_period))
+
+            gcra = Gcra()
+            otk = self.client.db.Otk
+
+            try:
+                val = otk.getall(rlkey)
+                gcra.set_tat_as_string(rlkey, val['tat'])
+            except KeyError:
+                # ignore if tat not set, tat is 1970-1-1 by default.
+                pass
+
+            # see if rate limit exceeded and we need to reject
+            # the attempt
+            reject = gcra.update(rlkey, limit)
+
+            # Calculate a timestamp that will make OTK expire the
+            # unused entry in twice the period defined for the
+            # rate limiter.
+            if update:
+                ts = otk.lifetime(per_period*2)
+                otk.set(rlkey, tat=gcra.get_tat_as_string(rlkey),
+                        __timestamp=ts)
+                otk.commit()
+
+            # User exceeded limits: find out how long to wait
+            limitStatus = gcra.status(rlkey, limit)
+
+            if not reject:
+                return
+
+            for header, value in limitStatus.items():
+                self.client.setHeader(header, value)
+
+            # User exceeded limits: tell humans how long to wait
+            # Headers above will do the right thing for api
+            # aware clients.
+            try:
+                retry_after = limitStatus['Retry-After']
+            except KeyError:
+                # handle race condition. If the time between
+                # the call to grca.update and grca.status
+                # is sufficient to reload the bucket by 1
+                # item, Retry-After will be missing from
+                # limitStatus. So report a 1 second delay back
+                # to the client. We treat update as sole
+                # source of truth for exceeded rate limits.
+                retry_after = 1
+                self.client.setHeader('Retry-After', '1')
+
+            # make rate limiting headers available to javascript
+            # even for non-api calls.
+            self.client.setHeader(
+                "Access-Control-Expose-Headers",
+                ", ".join([
+                    "X-RateLimit-Limit",
+                    "X-RateLimit-Remaining",
+                    "X-RateLimit-Reset",
+                    "X-RateLimit-Limit-Period",
+                    "Retry-After"
+                ])
+            )
+
+            raise RateLimitExceeded(_("Logins occurring too fast. Please wait: %s seconds.") % retry_after)
+
+    def verifyLogin(self, username, password, is_api=False):
+        """Authenticate the user with rate limits.
+
+           All logins (valid and failing) from a web page calling the
+           LoginAction method are rate limited to the config.ini
+           configured value in 1 minute. (Interval can be changed see
+           rateLimitLogin method.)
+
+           API logins are only rate limited if they fail. Successful
+           api logins are rate limited using the
+           api_calls_per_interval and api_interval_in_sec settings in
+           config.ini.
+
+           Once a user receives a rate limit notice, they must
+           wait the recommended time to try again as the account is
+           locked out for the recommended time. If a user tries to
+           log in while locked out, they will get a 429 rejection
+           even if the username and password are correct.
+        """
         # make sure the user exists
         try:
             # Note: lookup only searches non-retired items.
@@ -1381,10 +1505,24 @@ def verifyLogin(self, username, password):
             # delay caused by checking password only on valid
             # users.
             _discard = self.verifyPassword("2", password)          # noqa: F841
+
+            # limit logins to an acceptable rate. Do it for
+            # invalid usernames so attempts to break
+            # an invalid user will also be rate limited.
+            self.rateLimitLogin(username, is_api=is_api)
+
+            # this is not hit if rate limit is exceeded
             raise exceptions.LoginError(self._('Invalid login'))
 
+        # if we are rate limited and the user tries again,
+        # reject the login. update=false so we don't count
+        # a potentially valid login.
+        self.rateLimitLogin(username, is_api=is_api, update=False)
+
         # verify the password
         if not self.verifyPassword(self.client.userid, password):
+            self.rateLimitLogin(username, is_api=is_api)
+            # this is not hit if rate limit is exceeded
             raise exceptions.LoginError(self._('Invalid login'))
 
         # Determine whether the user has permission to log in.