authentication config: add optional max_validity_time, skew

The Zuul admin can configure authenticators with an optional
"max_validity_time" field, which is the maximum age in seconds
for a valid authentication token. By default there is no
maximum age set for tokens, except the one deduced from
the token's "exp" claim.
If "max_validity" is set, tokens without an "iat" claim will
be rejected.

This is meant as an extra security to avoid accidentally issueing
very long lived tokens through the CLI.

The "skew" field can be used to mitigate clocks discrepancies
between Zuul and a JWT emitter.

Change-Id: I9351ca016b60050b5f3b3950b840d5f719e919ce

doc/source/admin/components.rst

@@ -991,6 +991,13 @@ protected endpoints and configure JWT validation:
       of time in seconds. This is useful if the Zuul operator has no control
       over the service issueing JWTs, and the tokens are too long-lived.
 
+   .. attr:: skew
+      :default: 0
+
+      Optional integer value to compensate for skew between Zuul's and the
+      JWT emitter's respective clocks. Use a negative value if Zuul's clock
+      is running behind.
+
 This section can be repeated as needed with different authenticators, allowing
 access to privileged API actions from several JWT issuers.
 

doc/source/developer/specs/tenant-scoped-admin-web-API.rst

@@ -501,6 +501,8 @@ Here is an example defining the three supported types of authenticators:
     realm=openstack
     # (optional) Ensure a Token cannot be valid for longer than this amount of time, in seconds
     max_validity_time = 1800000
+    # (optional) Account for skew between clocks, in seconds
+    skew = 3
 
     # asymmetrical encryption
     [auth "my_oidc_idp"]
@@ -517,6 +519,8 @@ Here is an example defining the three supported types of authenticators:
     realm=openstack
     # (optional) Ensure a Token cannot be valid for longer than this amount of time, in seconds
     max_validity_time = 1800000
+    # (optional) Account for skew between clocks, in seconds
+    skew = 3
 
     # asymmetrical encryption using JWKS for validation
     # The signing secret being known to the Identity Provider only, this
@@ -532,7 +536,8 @@ Here is an example defining the three supported types of authenticators:
     uid_claim=name
     # Auth realm, used in 401 error messages
     realm=openstack
-
+    # (optional) Account for skew between clocks, in seconds
+    skew = 3
 
 Implementation
 ==============

etc/zuul.conf-sample

@@ -52,6 +52,8 @@ default=true
 client_id=zuul.example.com
 issuer_id=zuul_operator
 secret=NoDanaOnlyZuul
+max_validity_time=36000
+skew=0
 
 [connection gerrit]
 driver=gerrit

tests/fixtures/zuul-admin-web-token-expiry.conf

@@ -0,0 +1,32 @@
+[gearman]
+server=127.0.0.1
+
+[scheduler]
+tenant_config=main.yaml
+relative_priority=true
+
+[merger]
+git_dir=/tmp/zuul-test/merger-git
+git_user_email=zuul@example.com
+git_user_name=zuul
+
+[executor]
+git_dir=/tmp/zuul-test/executor-git
+
+[connection gerrit]
+driver=gerrit
+server=review.example.com
+user=jenkins
+sshkey=fake_id_rsa_path
+
+[web]
+static_cache_expiry=1200
+
+[auth zuul_operator]
+driver=HS256
+allow_authz_override=true
+realm=zuul.example.com
+client_id=zuul.example.com
+issuer_id=zuul_operator
+secret=NoDanaOnlyZuul
+max_validity_time=5

tests/unit/test_web.py

@@ -1716,6 +1716,162 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                              "%s got %s" % (authz['sub'], data))
 
 
+class TestTenantScopedWebApiTokenWithExpiry(BaseTestWeb):
+    config_file = 'zuul-admin-web-token-expiry.conf'
+
+    def test_iat_claim_mandatory(self):
+        """Test that the 'iat' claim is mandatory when
+        max_validity_time is set"""
+        authz = {'iss': 'zuul_operator',
+                 'sub': 'testuser',
+                 'aud': 'zuul.example.com',
+                 'zuul': {
+                     'admin': ['tenant-one', ]
+                 },
+                 'exp': time.time() + 3600}
+        token = jwt.encode(authz, key='NoDanaOnlyZuul',
+                           algorithm='HS256').decode('utf-8')
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/autohold",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'job': 'project-test1',
+                  'count': 1,
+                  'reason': 'because',
+                  'node_hold_expiration': 36000})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'change': '2,1',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'ref': 'abcd',
+                  'newrev': 'aaaa',
+                  'oldrev': 'bbbb',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+
+    def test_token_from_the_future(self):
+        authz = {'iss': 'zuul_operator',
+                 'sub': 'testuser',
+                 'aud': 'zuul.example.com',
+                 'zuul': {
+                     'admin': ['tenant-one', ],
+                 },
+                 'exp': time.time() + 7200,
+                 'iat': time.time() + 3600}
+        token = jwt.encode(authz, key='NoDanaOnlyZuul',
+                           algorithm='HS256').decode('utf-8')
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/autohold",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'job': 'project-test1',
+                  'count': 1,
+                  'reason': 'because',
+                  'node_hold_expiration': 36000})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'change': '2,1',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'ref': 'abcd',
+                  'newrev': 'aaaa',
+                  'oldrev': 'bbbb',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+
+    def test_token_expired(self):
+        authz = {'iss': 'zuul_operator',
+                 'sub': 'testuser',
+                 'aud': 'zuul.example.com',
+                 'zuul': {
+                     'admin': ['tenant-one', ],
+                 },
+                 'exp': time.time() + 3600,
+                 'iat': time.time()}
+        token = jwt.encode(authz, key='NoDanaOnlyZuul',
+                           algorithm='HS256').decode('utf-8')
+        time.sleep(10)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/autohold",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'job': 'project-test1',
+                  'count': 1,
+                  'reason': 'because',
+                  'node_hold_expiration': 36000})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'change': '2,1',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'ref': 'abcd',
+                  'newrev': 'aaaa',
+                  'oldrev': 'bbbb',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+
+    def test_autohold(self):
+        """Test that autohold can be set through the admin web interface"""
+        args = {"reason": "some reason",
+                "count": 1,
+                'job': 'project-test2',
+                'change': None,
+                'ref': None,
+                'node_hold_expiration': None}
+        authz = {'iss': 'zuul_operator',
+                 'aud': 'zuul.example.com',
+                 'sub': 'testuser',
+                 'zuul': {
+                     'admin': ['tenant-one', ],
+                 },
+                 'exp': time.time() + 3600,
+                 'iat': time.time()}
+        token = jwt.encode(authz, key='NoDanaOnlyZuul',
+                           algorithm='HS256').decode('utf-8')
+        req = self.post_url(
+            'api/tenant/tenant-one/project/org/project/autohold',
+            headers={'Authorization': 'Bearer %s' % token},
+            json=args)
+        self.assertEqual(200, req.status_code, req.text)
+        data = req.json()
+        self.assertEqual(True, data)
+
+        # Check result in rpc client
+        client = zuul.rpcclient.RPCClient('127.0.0.1',
+                                          self.gearman_server.port)
+        self.addCleanup(client.shutdown)
+
+        autohold_requests = client.autohold_list()
+        self.assertNotEqual([], autohold_requests)
+        self.assertEqual(1, len(autohold_requests))
+
+        ah_request = autohold_requests[0]
+        self.assertEqual('tenant-one', ah_request['tenant'])
+        self.assertIn('org/project', ah_request['project'])
+        self.assertEqual('project-test2', ah_request['job'])
+        self.assertEqual(".*", ah_request['ref_filter'])
+        self.assertEqual("some reason", ah_request['reason'])
+
+
 class TestWebMulti(BaseTestWeb):
     config_file = 'zuul-gerrit-github.conf'
 

zuul/cmd/client.py

@@ -540,7 +540,9 @@ class Client(zuul.cmd.ZuulApp):
             print('"%s" authenticator configuration not found.'
                   % self.args.auth_config)
             sys.exit(1)
-        token = {'exp': time.time() + self.args.expires_in,
+        now = time.time()
+        token = {'iat': now,
+                 'exp': now + self.args.expires_in,
                  'iss': get_default(self.config, auth_section, 'issuer_id'),
                  'aud': get_default(self.config, auth_section, 'client_id'),
                  'sub': self.args.user,

zuul/driver/auth/jwt.py

@@ -14,6 +14,7 @@
 # under the License.
 
 import logging
+import math
 import time
 import jwt
 import requests
@@ -36,11 +37,21 @@ class JWTAuthenticator(AuthenticatorInterface):
         self.audience = conf.get('client_id')
         self.realm = conf.get('realm')
         self.allow_authz_override = conf.get('allow_authz_override', False)
+        try:
+            self.skew = int(conf.get('skew', 0))
+        except Exception:
+            raise ValueError(
+                'skew must be an integer, got %s' % conf.get('skew'))
         if isinstance(self.allow_authz_override, str):
             if self.allow_authz_override.lower() == 'true':
                 self.allow_authz_override = True
             else:
                 self.allow_authz_override = False
+        try:
+            self.max_validity_time = float(conf.get('max_validity_time',
+                                                    math.inf))
+        except ValueError:
+            raise ValueError('"max_validity_time" must be a numerical value')
 
     def _decode(self, rawToken):
         raise NotImplementedError
@@ -68,13 +79,31 @@ class JWTAuthenticator(AuthenticatorInterface):
             raise exceptions.AuthTokenUnauthorizedException(
                 realm=self.realm,
                 msg=e)
+        # Missing claim tests
         if not all(x in decoded for x in ['aud', 'iss', 'exp', 'sub']):
             raise exceptions.MissingClaimError(realm=self.realm)
+        if self.max_validity_time < math.inf and 'iat' not in decoded:
+            raise exceptions.MissingClaimError(
+                msg='Missing "iat" claim',
+                realm=self.realm)
         if self.uid_claim not in decoded:
             raise exceptions.MissingUIDClaimError(realm=self.realm)
+        # Time related tests
         expires = decoded.get('exp', 0)
-        if expires < time.time():
+        issued_at = decoded.get('iat', 0)
+        now = time.time()
+        if issued_at + self.skew > now:
+            raise exceptions.AuthTokenUnauthorizedException(
+                msg='"iat" claim set in the future',
+                realm=self.realm
+            )
+        if now - issued_at > self.max_validity_time:
+            raise exceptions.TokenExpiredError(
+                msg='Token was issued too long ago',
+                realm=self.realm)
+        if expires + self.skew < now:
             raise exceptions.TokenExpiredError(realm=self.realm)
+        # Zuul-specific claims tests
         zuul_claims = decoded.get('zuul', {})
         admin_tenants = zuul_claims.get('admin', [])
         if not isinstance(admin_tenants, list):