bug: use better host validation for sub (#92)

While far from perfect, this is a little better about checking the sorts
of values passed in the `sub` claim. It checks for ipv6 like as well as
localhost in mailto and https forms. (Yeah, you'll still need to supply
`https` since that's part of the RFC. Plus, letsEncrypt is a thing, so
there's that.)

I've also introduced a new option `--no-strict` which turns off
`sub` checks. It has to be present, but what the value is can be up to
you. I'll note that this kinda ruins what VAPID is supposed to be about.
The `sub` is there so that if there's a problem with your subscription,
Ops can reach out to you to help fix it rather than just straight up
block you.

Closes: #90

Author: jrconlin

python/py_vapid/__init__.py

@@ -38,13 +38,16 @@ class Vapid01(object):
     _public_key = None
     _schema = "WebPush"
 
-    def __init__(self, private_key=None):
+    def __init__(self, private_key=None, conf=None):
         """Initialize VAPID with an optional private key.
 
         :param private_key: A private key object
         :type private_key: ec.EllipticCurvePrivateKey
 
         """
+        if conf is None:
+            conf = {}
+        self.conf = conf
         self.private_key = private_key
         if private_key:
             self._public_key = self.private_key.public_key()
@@ -259,9 +262,11 @@ def _base_sign(self, claims):
         cclaims = copy.deepcopy(claims)
         if not cclaims.get('exp'):
             cclaims['exp'] = str(int(time.time()) + 86400)
-        if not re.match(r'mailto:.+@.+\..+',
-                        cclaims.get('sub', ''),
-                        re.IGNORECASE):
+        if not self.conf.get('no-strict', False):
+            valid = _check_sub(cclaims.get('sub', ''))
+        else:
+            valid = cclaims.get('sub') is not None
+        if not valid:
             raise VapidException(
                 "Missing 'sub' from claims. "
                 "'sub' is your admin email as a mailto: link.")
@@ -345,5 +350,11 @@ def verify(cls, auth):
             verification_token=tokens[1]
         )
 
+def _check_sub(sub):
+    pattern =(
+        r"^(mailto:.+@((localhost|[%\w]+(\.[%\w]+)+|([0-9a-f]{1,4}):+([0-9a-f]{1,4})?)))|https:\/\/(localhost|\w+\.[\w\.]+|([0-9a-f]{1,4}:+)+([0-9a-f]{1,4})?)$"
+        )
+    return re.match(pattern, sub, re.IGNORECASE) is not None
+
 
 Vapid = Vapid02

python/py_vapid/main.py

@@ -31,6 +31,8 @@ def main():
                         default=False, action="store_true")
     parser.add_argument('--json',  help="dump as json",
                         default=False, action="store_true")
+    parser.add_argumet('--no-strict', help='Do not be strict about "sub"',
+                        default=False, action="store_true")
     parser.add_argument('--applicationServerKey',
                         help="show applicationServerKey value",
                         default=False, action="store_true")
@@ -52,7 +54,7 @@ def main():
                 if answer == 'n':
                     print("Sorry, can't do much for you then.")
                     exit(1)
-        vapid = Vapid()
+        vapid = Vapid(conf=args)
         vapid.generate_keys()
         print("Generating private_key.pem")
         vapid.save_key('private_key.pem')

python/py_vapid/tests/test_vapid.py

@@ -7,7 +7,7 @@
 from cryptography.hazmat.primitives import serialization
 from mock import patch, Mock
 
-from py_vapid import Vapid01, Vapid02, VapidException
+from py_vapid import Vapid01, Vapid02, VapidException, _check_sub
 from py_vapid.jwt import decode
 
 TEST_KEY_PRIVATE_DER = """
@@ -228,7 +228,12 @@ def test_bad_sign(self):
         self.assertRaises(VapidException,
                           v.sign,
                           {'sub': 'mailto:[email protected]',
-                              'aud': "https://p.example.com:8080/"})
+                           'aud': "https://p.example.com:8080/"})
+
+    def test_ignore_sub(self):
+        v = Vapid02.from_file("/tmp/private")
+        v.conf['no-strict'] = True
+        assert v.sign({"sub": "foo", "aud": "http://localhost:8000"})
 
     @patch('cryptography.hazmat.primitives.asymmetric'
            '.ec.EllipticCurvePublicNumbers')
@@ -247,3 +252,29 @@ def test_invalid_sig(self, mm):
                           decode,
                           'foo.bar.a',
                           'aaaa')
+
+    def test_sub(self):
+        valid = [
+            'mailto:me@localhost',
+            'mailto:[email protected]',
+            'mailto:me@1234::',
+            'mailto:me@1234::5678',
+            'mailto:[email protected]',
+            'https://localhost',
+            'https://8001::',
+            'https://8001:1000:0001',
+            'https://1.2.3.4'
+        ]
+        invalid = [
+            'mailto:@foobar.com',
+            'mailto:example.org',
+            'mailto:0123:',
+            'mailto:::1234',
+            'https://somehost',
+            'https://xyz:123',
+        ]
+
+        for val in valid:
+            assert _check_sub(val) is True
+        for val in invalid:
+            assert _check_sub(val) is False