Change KEM Parsing to actually check claims

The name ('sub') and the time ('exp') must be checked before letting
the reuqest proceed.

Signed-off-by: Simo Sorce <simo@redhat.com>

6 files changed, 33 insertions, 21 deletions

diff --git a/API.md b/API.md
index aff7ba2..eb78581 100644
--- a/API.md
+++ b/API.md
@@ -51,9 +51,9 @@ Format:
  (flattened/decoded here for clarity)
   { "protected": { "kid": <public-key-dentifier>,
                    "alg": "a valid alg name"},
-    "payload": { "name": <name-of-secret>,
-                 "time": <unix-timestamp>,
-                 ["value": <arbitrary> ]},
+    "claims": { "sub": <name-of-secret>,
+                "exp": <unix-timestamp indicating expiration time>,
+                ["value": <arbitrary> ]},
     "signature": "XYZ...." }
 
  Attributes:
@@ -65,8 +65,10 @@ Format:
  - name-of-secret: this repeates the name of the secret embedded in the GET,
  This is used to prevent substitution attacks where a client is intercepted
  and its signed request is reused to request a different key.
- - unix-timestamp: used to limit replay attacks
- Additional payload attributes may be present, for example a 'value'.
+ - unix-timestamp: used to limit replay attacks, indicated expiration time,
+ and should be no further than 5 minutes in the future, with leway up to 10
+ minutes to account for clock skews
+ Additional claims may be present, for example a 'value'.
 
  The Message for a GET reply or a PUT is a JWS Encoded message (see above)
  nested in a JWE Encoded message:
diff --git a/custodia/message/common.py b/custodia/message/common.py
index 89deb59..25ce4e7 100644
--- a/custodia/message/common.py
+++ b/custodia/message/common.py
@@ -34,7 +34,7 @@ class MessageHandler(object):
         self.req = request
         self.payload = None
 
-    def parse(self, msg):
+    def parse(self, msg, name):
         """Parses the message.
 
         :param req: the original request
diff --git a/custodia/message/formats.py b/custodia/message/formats.py
index 00845a3..8093ba7 100644
--- a/custodia/message/formats.py
+++ b/custodia/message/formats.py
@@ -27,7 +27,7 @@ class Validator(object):
     def add_types(self, types):
         self.types.update(types)
 
-    def parse(self, request, msg):
+    def parse(self, request, msg, name):
         if not isinstance(msg, dict):
             raise InvalidMessage('The message must be a dict')
 
@@ -59,5 +59,5 @@ class Validator(object):
                                    msg_type,))
 
         handler = self.types[msg_type](request)
-        handler.parse(msg_value)
+        handler.parse(msg_value, name)
         return handler
diff --git a/custodia/message/kem.py b/custodia/message/kem.py
index f8dbd3a..0a4d406 100644
--- a/custodia/message/kem.py
+++ b/custodia/message/kem.py
@@ -106,7 +106,7 @@ class KEMHandler(MessageHandler):
             raise UnknownPublicKey('Key found [kid:%s]' % header['kid'])
         return json_decode(key)
 
-    def parse(self, msg):
+    def parse(self, msg, name):
         """Parses the message.
 
         We check that the message is properly formatted.
@@ -130,7 +130,7 @@ class KEMHandler(MessageHandler):
                 ekey = self._get_key(token.jose_header, KEY_USAGE_ENC)
                 self.client_keys = (JWK(**skey), JWK(**ekey))
                 token.verify(self.client_keys[0])
-                payload = token.payload
+                claims = json_decode(token.payload)
             elif isinstance(token, JWE):
                 token.decrypt(self.kkstore.server_keys[1])
                 # If an ecnrypted payload is received then there must be
@@ -141,17 +141,27 @@ class KEMHandler(MessageHandler):
                 ekey = self._get_key(nested.jose_header, KEY_USAGE_ENC)
                 self.client_keys = (JWK(**skey), JWK(**ekey))
                 nested.verify(self.client_keys[0])
-                payload = nested.payload
+                claims = json_decode(nested.payload)
             else:
                 raise TypeError("Invalid Token type: %s" % type(jtok))
         except Exception as e:
             raise InvalidMessage('Failed to validate message: %s' % str(e))
 
         # FIXME: check name/time
+        if 'sub' not in claims:
+            raise InvalidMessage('Missing subject in payload')
+        if claims['sub'] != name:
+            raise InvalidMessage('Key name does not match payload subject')
+        if 'exp' not in claims:
+            raise InvalidMessage('Missing request time in payload')
+        if claims['exp'] - (10 * 60) > int(time.time()):
+            raise InvalidMessage('Message expiration too long')
+        if claims['exp'] < int(time.time()):
+            raise InvalidMessage('Message Expired')
 
         return {'type': 'kem',
                 'value': {'kid': self.client_keys[0].key_id,
-                          'payload': payload}}
+                          'claims': claims}}
 
     def reply(self, output):
         if self.client_keys is None:
@@ -172,7 +182,7 @@ class KEMHandler(MessageHandler):
 
 
 def make_sig_kem(name, value, key, alg):
-    payload = {'name': name, 'time': int(time.time())}
+    payload = {'sub': name, 'exp': int(time.time() + (5 * 60))}
     if value is not None:
         payload['value'] = value
     S = JWS(json_encode(payload))
@@ -310,8 +320,8 @@ class KEMTests(unittest.TestCase):
         protected = {"typ": "JOSE+JSON",
                      "kid": key['kid'],
                      "alg": alg}
-        plaintext = {"name": name,
-                     "time": int(time.time())}
+        plaintext = {"sub": name,
+                     "exp": int(time.time()) + (5 * 60)}
         S = JWS(payload=json_encode(plaintext))
         S.add_signature(pri_key, None, json_encode(protected))
         return S.serialize()
@@ -320,7 +330,7 @@ class KEMTests(unittest.TestCase):
         cli_skey = JWK(**self.client_keys[0])
         jtok = make_sig_kem("mykey", None, cli_skey, "RS256")
         kem = KEMHandler({'KEMKeysStore': self.kk})
-        kem.parse(jtok)
+        kem.parse(jtok, "mykey")
         out = kem.reply('output')
         jtok = JWT(jwt=json_decode(out)['value'])
         cli_ekey = JWK(**self.client_keys[1])
diff --git a/custodia/message/simple.py b/custodia/message/simple.py
index c7f32ee..1df1310 100644
--- a/custodia/message/simple.py
+++ b/custodia/message/simple.py
@@ -9,7 +9,7 @@ import json
 class SimpleKey(MessageHandler):
     """Handles 'simple' messages"""
 
-    def parse(self, msg):
+    def parse(self, msg, name):
         """Parses a simple message
 
         :param req: ignored
diff --git a/custodia/secrets.py b/custodia/secrets.py
index 7aa43df..c896564 100644
--- a/custodia/secrets.py
+++ b/custodia/secrets.py
@@ -69,8 +69,8 @@ class Secrets(HTTPConsumer):
             f = self._db_key([default, ''])
         return f
 
-    def _parse(self, request, value):
-        return self._validator.parse(request, value)
+    def _parse(self, request, value, name):
+        return self._validator.parse(request, value, name)
 
     def _parent_exists(self, default, trail):
         # check that the containers exist
@@ -186,7 +186,7 @@ class Secrets(HTTPConsumer):
         if len(query) == 0:
             query = {'type': 'simple', 'value': ''}
         try:
-            msg = self._parse(request, query)
+            msg = self._parse(request, query, trail)
         except Exception as e:
             raise HTTPError(406, str(e))
         key = self._db_key(trail)
@@ -208,7 +208,7 @@ class Secrets(HTTPConsumer):
             raise HTTPError(400)
         value = bytes(body).decode('utf-8')
         try:
-            msg = self._parse(request, json.loads(value))
+            msg = self._parse(request, json.loads(value), trail)
         except UnknownMessageType as e:
             raise HTTPError(406, str(e))
         except UnallowedMessage as e: