diff --git a/doc/source/admin/components.rst b/doc/source/admin/components.rst index a6cdaebb4b..10c99bba11 100644 --- a/doc/source/admin/components.rst +++ b/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. diff --git a/doc/source/developer/specs/tenant-scoped-admin-web-API.rst b/doc/source/developer/specs/tenant-scoped-admin-web-API.rst index fe060bbf9a..fb60aa2e50 100644 --- a/doc/source/developer/specs/tenant-scoped-admin-web-API.rst +++ b/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 ============== diff --git a/etc/zuul.conf-sample b/etc/zuul.conf-sample index c0916c6c43..dd1d37e5ae 100644 --- a/etc/zuul.conf-sample +++ b/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 diff --git a/tests/fixtures/zuul-admin-web-token-expiry.conf b/tests/fixtures/zuul-admin-web-token-expiry.conf new file mode 100644 index 0000000000..7123455a0b --- /dev/null +++ b/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 diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 836e94c2ed..d8443c50e2 100644 --- a/tests/unit/test_web.py +++ b/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' diff --git a/zuul/cmd/client.py b/zuul/cmd/client.py index 8e01f8084a..82487ff2a1 100755 --- a/zuul/cmd/client.py +++ b/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, diff --git a/zuul/driver/auth/jwt.py b/zuul/driver/auth/jwt.py index 2579c3a33d..3517ff3e5b 100644 --- a/zuul/driver/auth/jwt.py +++ b/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):