Fix #5344 - OAuth NotImplemented exception

This introduces a migration which adds a dummy Client, RequestToken
and AccessToken. These are used when an invalid request comes in,
instead of bailing early, it needs dummy data to prevent timing
attacks.

This then implements the methods which get the IDs of the dummy
objects. If these are changed in the future a migration which checks
for the previous dummy object should be created and updates them to
reflect the new IDs/tokens.

diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py
index 00b0add12ce0a372affc1abde1f474e560d89352..d6806813b9e227ba13253354320dcd97e1a3fa0a 100644 (file)
--- a/mediagoblin/db/migrations.py
+++ b/mediagoblin/db/migrations.py
@@ -32,6 +32,8 @@ from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.sql import and_
 from sqlalchemy.schema import UniqueConstraint
 
+from mediagoblin import oauth
+from mediagoblin.tools import crypto
 from mediagoblin.db.extratypes import JSONEncoded, MutationDict
 from mediagoblin.db.migration_tools import (
     RegisterMigration, inspect_table, replace_table_hack)
@@ -1616,3 +1618,60 @@ def federation_media_entry(db):
         ))
 
     db.commit()
+
+@RegisterMigration(36, MIGRATIONS)
+def create_oauth1_dummies(db):
+    """
+    Creates a dummy client, request and access tokens.
+
+    This is used when invalid data is submitted but real clients and
+    access tokens. The use of dummy objects prevents timing attacks.
+    """
+    metadata = MetaData(bind=db.bind)
+    client_table = inspect_table(metadata, "core__clients")
+    request_token_table = inspect_table(metadata, "core__request_tokens")
+    access_token_table = inspect_table(metadata, "core__access_tokens")
+
+    # Whilst we don't rely on the secret key being unique or unknown to prevent
+    # unauthorized clients from using it to authenticate, we still as an extra
+    # layer of protection created a cryptographically secure key individual to
+    # each instance that should never be able to be known.
+    client_secret = crypto.random_string(50)
+    request_token_secret = crypto.random_string(50)
+    request_token_verifier = crypto.random_string(50)
+    access_token_secret = crypto.random_string(50)
+
+    # Dummy created/updated datetime object
+    epoc_datetime = datetime.datetime.fromtimestamp(0)
+
+    # Create the dummy Client
+    db.execute(client_table.insert().values(
+        id=oauth.DUMMY_CLIENT_ID,
+        secret=client_secret,
+        application_type="dummy",
+        created=epoc_datetime,
+        updated=epoc_datetime
+    ))
+
+    # Create the dummy RequestToken
+    db.execute(request_token_table.insert().values(
+        token=oauth.DUMMY_REQUEST_TOKEN,
+        secret=request_token_secret,
+        client=oauth.DUMMY_CLIENT_ID,
+        verifier=request_token_verifier,
+        created=epoc_datetime,
+        updated=epoc_datetime,
+        callback="oob"
+    ))
+
+    # Create the dummy AccessToken
+    db.execute(access_token_table.insert().values(
+        token=oauth.DUMMY_ACCESS_TOKEN,
+        secret=access_token_secret,
+        request_token=oauth.DUMMY_REQUEST_TOKEN,
+        created=epoc_datetime,
+        updated=epoc_datetime
+    ))
+
+    # Commit the changes
+    db.commit()

diff --git a/mediagoblin/oauth/__init__.py b/mediagoblin/oauth/__init__.py
index 719b56e7c231319f077d06ed4a036bc571d41fc8..6dfafea279905f3449819989b9506fa7a96e7316 100644 (file)
--- a/mediagoblin/oauth/__init__.py
+++ b/mediagoblin/oauth/__init__.py
@@ -14,3 +14,9 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# This represents a dummy ID for the Client, RequestToken and AccessToken
+# WARNING: Do not change these without providing a data migration to migrate
+#          existing dummy clients already in the database.
+DUMMY_CLIENT_ID = "dummy-client"
+DUMMY_ACCESS_TOKEN = "dummy-access-token"
+DUMMY_REQUEST_TOKEN = "dummy-request-token"

diff --git a/mediagoblin/oauth/oauth.py b/mediagoblin/oauth/oauth.py
index c7951734c9fb39916d117e748f3adba8df23ca76..2cc4e79019a0e331c4061e557b5854661eb14978 100644 (file)
--- a/mediagoblin/oauth/oauth.py
+++ b/mediagoblin/oauth/oauth.py
@@ -18,6 +18,7 @@ import datetime
 from oauthlib.common import Request
 from oauthlib.oauth1 import RequestValidator
 
+from mediagoblin import oauth
 from mediagoblin.db.models import NonceTimestamp, Client, RequestToken, AccessToken
 
 class GMGRequestValidator(RequestValidator):
@@ -94,7 +95,8 @@ class GMGRequestValidator(RequestValidator):
 
     def validate_client_key(self, client_key, request):
         """ Verifies client exists with id of client_key """
-        client = Client.query.filter_by(id=client_key).first()
+        client_query = Client.query.filter(Client.id != oauth.DUMMY_CLIENT_ID)
+        client = client_query.filter_by(id=client_key).first()
         if client is None:
             return False
 
@@ -102,15 +104,30 @@ class GMGRequestValidator(RequestValidator):
 
     def validate_access_token(self, client_key, token, request):
         """ Verifies token exists for client with id of client_key """
-        client = Client.query.filter_by(id=client_key).first()
-        token = AccessToken.query.filter_by(token=token)
-        token = token.first()
+        # Get the client for the request
+        client_query = Client.query.filter(Client.id != oauth.DUMMY_CLIENT_ID)
+        client = client_query.filter_by(id=client_key).first()
 
-        if token is None:
+        # If the client is invalid then it's invalid
+        if client is None:
             return False
 
-        request_token = RequestToken.query.filter_by(token=token.request_token)
-        request_token = request_token.first()
+        # Look up the AccessToken
+        access_token_query = AccessToken.query.filter(
+            AccessToken.token != oauth.DUMMY_ACCESS_TOKEN
+        )
+        access_token = access_token_query.filter_by(token=token).first()
+
+        # If there isn't one - we can't validate.
+        if access_token is None:
+            return False
+
+        # Check that the client matches the on
+        request_token_query = RequestToken.query.filter(
+            RequestToken.token != oauth.DUMMY_REQUEST_TOKEN,
+            RequestToken.token == access_token.request_token
+        )
+        request_token = request_token_query.first()
 
         if client.id != request_token.client:
             return False
@@ -131,6 +148,18 @@ class GMGRequestValidator(RequestValidator):
         access_token = AccessToken.query.filter_by(token=token).first()
         return access_token.secret
 
+    @property
+    def dummy_client(self):
+        return oauth.DUMMY_CLIENT_ID
+
+    @property
+    def dummy_request_token(self):
+        return oauth.DUMMY_REQUEST_TOKEN
+
+    @property
+    def dummy_access_token(self):
+        return oauth.DUMMY_ACCESS_TOKEN
+
 class GMGRequest(Request):
     """
         Fills in data to produce a oauth.common.Request object from a

diff --git a/mediagoblin/oauth/views.py b/mediagoblin/oauth/views.py
index 1b4787d6d90b133780868aca71ec1a06b8c8a181..2bfaab3e74ea7734ad3d9e04fc988fff70c63ee1 100644 (file)
--- a/mediagoblin/oauth/views.py
+++ b/mediagoblin/oauth/views.py
@@ -211,7 +211,7 @@ def request_token(request):
         error = "Invalid client_id"
         return json_response({"error": error}, status=400)
 
-   # make request token and return to client
+    # make request token and return to client
     request_validator = GMGRequestValidator(authorization)
     rv = RequestTokenEndpoint(request_validator)
     tokens = rv.create_request_token(request, authorization)