improved extensibility of error handling in digest.py

Instead of randomly raising KeyErrors or ValueErrors we can now see more
meaningful ProtocolViolations and StaleNonces. Note that this change
should be invisible to users who do not mess with internals.

1 files changed, 103 insertions, 66 deletions

diff --git a/wsgitools/digest.py b/wsgitools/digest.py
index ead4939..cbb902e 100644
--- a/wsgitools/digest.py
+++ b/wsgitools/digest.py
@@ -89,6 +89,12 @@ class AuthenticationRequired(Exception):
     not visible to other code.
     """
 
+class ProtocolViolation(AuthenticationRequired):
+    pass
+
+class StaleNonce(AuthenticationRequired):
+    pass
+
 __all__.append("AbstractTokenGenerator")
 class AbstractTokenGenerator:
     """Interface class for generating authentication tokens for
@@ -599,6 +605,32 @@ class DBAPI2NonceStore(NonceStoreBase):
         self.dbhandle.commit()
         return True
 
+def check_uri(credentials, environ):
+    """internal method for verifying the uri credential
+    @raises AuthenticationRequired:
+    """
+    # Doing this by stripping known parts from the passed uri field
+    # until something trivial remains, as the uri cannot be
+    # reconstructed from the environment exactly.
+    try:
+        uri = credentials["uri"]
+    except KeyError:
+        raise ProtocolViolation("uri missing in client credentials")
+    if environ.get("QUERY_STRING"):
+        if not uri.endswith(environ["QUERY_STRING"]):
+            raise AuthenticationRequired("url mismatch")
+        uri = uri[:-len(environ["QUERY_STRING"])]
+    if environ.get("SCRIPT_NAME"):
+        if not uri.startswith(environ["SCRIPT_NAME"]):
+            raise AuthenticationRequired("url mismatch")
+        uri = uri[len(environ["SCRIPT_NAME"]):]
+    if environ.get("PATH_INFO"):
+        if not uri.startswith(environ["PATH_INFO"]):
+            raise AuthenticationRequired("url mismatch")
+        uri = uri[len(environ["PATH_INFO"]):]
+    if uri not in ('', '?'):
+        raise AuthenticationRequired("url mismatch")
+
 __all__.append("AuthDigestMiddleware")
 class AuthDigestMiddleware:
     """Middleware partly implementing RFC2617. (md5-sess was omited)
@@ -634,67 +666,64 @@ class AuthDigestMiddleware:
         """wsgi interface"""
 
         try:
-            auth = environ["HTTP_AUTHORIZATION"] # raises KeyError
-            method, rest = auth.split(' ', 1) # raises ValueError
+            try:
+                auth = environ["HTTP_AUTHORIZATION"]
+            except KeyError:
+                raise AuthenticationRequired("no Authorization header found")
+            try:
+                method, rest = auth.split(' ', 1)
+            except ValueError:
+                method, rest = auth, ""
 
             if method.lower() != "digest":
-                raise AuthenticationRequired
-            credentials = parse_digest_response(rest) # raises ValueError
+                raise AuthenticationRequired(
+                    "authorization method not implemented: %r" % method)
+            try:
+                credentials = parse_digest_response(rest)
+            except ValueError:
+                raise ProtocolViolation("failed to parse digest response")
 
             ### Check algorithm field
             credentials["algorithm"] = credentials.get("algorithm",
                                                        "md5").lower()
             if not credentials["algorithm"] in self.algorithms:
-                raise AuthenticationRequired
-
-            ### Check uri field
-            # Doing this by stripping known parts from the passed uri field
-            # until something trivial remains, as the uri cannot be
-            # reconstructed from the environment exactly.
-            uri = credentials["uri"] # raises KeyError
-            if "QUERY_STRING" in environ and environ["QUERY_STRING"]:
-                if not uri.endswith(environ["QUERY_STRING"]):
-                    raise AuthenticationRequired
-                uri = uri[:-len(environ["QUERY_STRING"])]
-            if "SCRIPT_NAME" in environ:
-                if not uri.startswith(environ["SCRIPT_NAME"]):
-                    raise AuthenticationRequired
-                uri = uri[len(environ["SCRIPT_NAME"]):]
-            if "PATH_INFO" in environ:
-                if not uri.startswith(environ["PATH_INFO"]):
-                    raise AuthenticationRequired
-                uri = uri[len(environ["PATH_INFO"]):]
-            if uri not in ('', '?'):
-                raise AuthenticationRequired
-            del uri
-
-            if ("username" not in credentials or
-                    "nonce" not in credentials or
-                    "response" not in credentials or
-                    "qop" in credentials and (
-                        credentials["qop"] != "auth" or 
-                        "nc" not in credentials or
-                        credentials["nc"].lower().strip("0123456789abcdef") or
-                        "cnonce" not in credentials)):
-                raise AuthenticationRequired
+                raise ProtocolViolation("algorithm not implemented: %r" %
+                                        credentials["algorithm"])
 
-            noncecount = 1
-            if credentials.get("qop") is not None:
-                # raises ValueError
-                noncecount = int(credentials["nc"], 16)
+            check_uri(credentials, environ)
 
-            if not self.noncestore.checknonce(credentials["nonce"], noncecount):
-                return self.authorization_required(environ, start_response,
-                                                   stale=True) # stale nonce!
-
-            # raises KeyError, ValueError
+            try:
+                nonce = credentials["nonce"]
+                credresponse = credentials["response"]
+            except KeyError, err:
+                raise ProtocolViolation("%s missing in credentials" %
+                                        err.args[0])
+            noncecount = 1
+            if "qop" in credentials:
+                if credentials["qop"] != "auth":
+                    raise ProtocolViolation("unimplemented qop: %r" %
+                                            credentials["qop"])
+                try:
+                    noncecount = int(credentials["nc"], 16)
+                except KeyError:
+                    raise ProtocolViolation("nc missing in qop=auth")
+                except ValueError:
+                    raise ProtocolViolation("non hexdigit found in nonce count")
+
+            # raises AuthenticationRequired
             response = self.auth_response(credentials,
                                           environ["REQUEST_METHOD"])
 
-            if response != credentials["response"]:
-                raise AuthenticationRequired
+            if not self.noncestore.checknonce(nonce, noncecount):
+                raise StaleNonce()
+
+            if response is None or response != credresponse:
+                raise AuthenticationRequired("wrong response")
 
-        except (KeyError, ValueError, AuthenticationRequired):
+        except StaleNonce:
+            return self.authorization_required(environ, start_response,
+                                               stale=True)
+        except AuthenticationRequired:
             return self.authorization_required(environ, start_response)
         else:
             environ["REMOTE_USER"] = credentials["username"]
@@ -711,30 +740,38 @@ class AuthDigestMiddleware:
 
     def auth_response(self, credentials, reqmethod):
         """internal method generating authentication tokens
-        @raises KeyError:
-        @raises ValueError:
+        @raises AuthenticationRequired:
         """
-        username = credentials["username"]
-        algo = credentials["algorithm"]
-        uri = credentials["uri"]
-        nonce = credentials["nonce"]
+        try:
+            username = credentials["username"]
+            algo = credentials["algorithm"]
+            uri = credentials["uri"]
+        except KeyError, err:
+            raise ProtocolViolation("%s missing in credentials" % err.args[0])
+        try:
+            dig = [credentials["nonce"]]
+        except KeyError:
+            raise ProtocolViolation("missing nonce in credentials")
+        qop = credentials.get("qop")
+        if qop is not None:
+            if qop != "auth":
+                raise AuthenticationRequired("unimplemented qop: %r" % qop)
+            try:
+                dig.append(credentials["nc"])
+                dig.append(credentials["cnonce"])
+            except KeyError, err:
+                raise ProtocolViolation(
+                    "missing %s in credentials with qop=auth" % err.args[0])
+            dig.append(qop)
+        dig.append(self.algorithms[algo]("%s:%s" % (reqmethod, uri)))
         try:
             a1h = self.gentoken(username, algo)
         except TypeError:
             a1h = self.gentoken(username)
         if a1h is None:
-            raise ValueError
-        a2h = self.algorithms[algo]("%s:%s" % (reqmethod, uri))
-        qop = credentials.get("qop", None)
-        if qop is None:
-            dig = ":".join((a1h, nonce, a2h))
-        else:
-            if qop != "auth":
-                raise ValueError
-            # raises KeyError
-            dig =  ":".join((a1h, nonce, credentials["nc"],
-                             credentials["cnonce"], qop, a2h))
-        return self.algorithms[algo](dig)
+            return None # delay the error for a nonexistent user
+        dig.insert(0, a1h)
+        return self.algorithms[algo](":".join(dig))
 
     def authorization_required(self, environ, start_response, stale=False):
         """internal method implementing wsgi interface, serving 401 page"""