Merge pull request #1432 from coreos-inc/fix-keyserver

Fix key server to not list expired keys
This commit is contained in:
josephschorr 2016-05-03 17:59:35 -04:00
commit ee6d632e1b
4 changed files with 129 additions and 34 deletions

View file

@ -137,7 +137,7 @@ def delete_service_key(kid):
def set_key_expiration(kid, expiration_date):
try:
service_key = get_service_key(kid)
service_key = get_service_key(kid, alive_only=False, approved_only=False)
except ServiceKey.DoesNotExist:
raise ServiceKeyDoesNotExist
@ -163,12 +163,17 @@ def approve_service_key(kid, approver, approval_type, notes=''):
return key
def _list_service_keys_query(kid=None, service=None, approved_only=False, approval_type=None):
def _list_service_keys_query(kid=None, service=None, approved_only=True, alive_only=True,
approval_type=None):
query = ServiceKey.select().join(ServiceKeyApproval, JOIN_LEFT_OUTER)
if approved_only:
query = query.where(~(ServiceKey.approval >> None))
if alive_only:
query = query.where((ServiceKey.expiration_date > datetime.utcnow()) |
(ServiceKey.expiration_date >> None))
if approval_type is not None:
query = query.where(ServiceKeyApproval.approval_type == approval_type)
@ -185,15 +190,16 @@ def _list_service_keys_query(kid=None, service=None, approved_only=False, approv
def list_all_keys():
return list(_list_service_keys_query())
return list(_list_service_keys_query(approved_only=False, alive_only=False))
def list_service_keys(service):
return list(_list_service_keys_query(service=service, approved_only=True))
return list(_list_service_keys_query(service=service))
def get_service_key(kid, service=None):
def get_service_key(kid, service=None, alive_only=True, approved_only=True):
try:
return _list_service_keys_query(kid=kid, service=service).get()
return _list_service_keys_query(kid=kid, service=service, approved_only=approved_only,
alive_only=alive_only).get()
except ServiceKey.DoesNotExist:
raise ServiceKeyDoesNotExist

View file

@ -639,7 +639,7 @@ class SuperUserServiceKey(ApiResource):
def get(self, kid):
if SuperUserPermission().can():
try:
key = model.service_keys.get_service_key(kid)
key = model.service_keys.get_service_key(kid, approved_only=False, alive_only=False)
return jsonify(key_view(key))
except model.service_keys.ServiceKeyDoesNotExist:
abort(404)
@ -655,7 +655,7 @@ class SuperUserServiceKey(ApiResource):
if SuperUserPermission().can():
body = request.get_json()
try:
key = model.service_keys.get_service_key(kid)
key = model.service_keys.get_service_key(kid, approved_only=False, alive_only=False)
except model.service_keys.ServiceKeyDoesNotExist:
abort(404)
@ -690,7 +690,8 @@ class SuperUserServiceKey(ApiResource):
model.service_keys.update_service_key(kid, body.get('name'), body.get('metadata'))
log_action('service_key_modify', None, key_log_metadata)
return jsonify(key_view(model.service_keys.get_service_key(kid)))
updated_key = model.service_keys.get_service_key(kid, approved_only=False, alive_only=False)
return jsonify(key_view(updated_key))
abort(403)

View file

@ -60,14 +60,19 @@ def _validate_jwt(encoded_jwt, jwk, service):
abort(400)
def _signer_kid(encoded_jwt):
def _signer_kid(encoded_jwt, allow_none=False):
headers = get_unverified_header(encoded_jwt)
return headers.get('kid', None)
kid = headers.get('kid', None)
if not kid and not allow_none:
abort(400)
return kid
def _signer_key(service, signer_kid):
def _lookup_service_key(service, signer_kid, approved_only=True):
try:
return data.model.service_keys.get_service_key(signer_kid, service=service)
return data.model.service_keys.get_service_key(signer_kid, service=service,
approved_only=approved_only)
except data.model.ServiceKeyDoesNotExist:
abort(403)
@ -81,7 +86,7 @@ def list_service_keys(service):
@key_server.route('/services/<service>/keys/<kid>', methods=['GET'])
def get_service_key(service, kid):
try:
key = data.model.service_keys.get_service_key(kid)
key = data.model.service_keys.get_service_key(kid, alive_only=False, approved_only=False)
except data.model.ServiceKeyDoesNotExist:
abort(404)
@ -126,8 +131,7 @@ def put_service_key(service, kid):
_validate_jwk(jwk)
signer_kid = _signer_kid(encoded_jwt)
signer_kid = _signer_kid(encoded_jwt, allow_none=True)
if kid == signer_kid or signer_kid is None:
# The key is self-signed. Create a new instance and await approval.
_validate_jwt(encoded_jwt, jwk, service)
@ -147,11 +151,10 @@ def put_service_key(service, kid):
log_action('service_key_create', None, metadata=key_log_metadata, ip=request.remote_addr)
return make_response('', 202)
# Key is going to be rotated.
metadata.update({'created_by': 'Key Rotation'})
signer_key = _signer_key(service, signer_kid)
signer_key = _lookup_service_key(service, signer_kid)
signer_jwk = signer_key.jwk
if signer_key.service != service:
abort(403)
_validate_jwt(encoded_jwt, signer_jwk, service)
@ -184,9 +187,9 @@ def delete_service_key(service, kid):
encoded_jwt = match.group(1)
signer_kid = _signer_kid(encoded_jwt)
signer_key = _signer_key(service, signer_kid)
signer_key = _lookup_service_key(service, signer_kid, approved_only=False)
self_signed = kid == signer_kid or signer_kid == ''
self_signed = kid == signer_kid
approved_key_for_service = signer_key.approval is not None
if self_signed or approved_key_for_service:

View file

@ -6,6 +6,7 @@ import unittest
from urllib import urlencode
from urlparse import urlparse, urlunparse, parse_qs
from datetime import datetime, timedelta
import jwt
@ -201,22 +202,27 @@ class KeyServerTestCase(EndpointTestCase):
}
def test_list_service_keys(self):
unapproved_key = model.service_keys.get_service_key(kid='kid3')
expired_key = model.service_keys.get_service_key(kid='kid6')
# Retrieve all the keys.
all_keys = model.service_keys.list_all_keys()
visible_jwks = [key.jwk for key in model.service_keys.list_service_keys('sample_service')]
invisible_jwks = []
for key in all_keys:
is_expired = key.expiration_date and key.expiration_date <= datetime.utcnow()
if key.service != 'sample_service' or key.approval is None or is_expired:
invisible_jwks.append(key.jwk)
rv = self.getResponse('key_server.list_service_keys', service='sample_service')
jwkset = py_json.loads(rv)
# Make sure the hidden keys are not returned and the visible ones are returned.
self.assertTrue(len(jwkset['keys']) > 0)
expired_key_found = False
self.assertTrue(len(visible_jwks) > 0)
self.assertTrue(len(invisible_jwks) > 0)
self.assertEquals(len(visible_jwks), len(jwkset['keys']))
for jwk in jwkset['keys']:
self.assertNotEquals(jwk, unapproved_key.jwk)
if expired_key.jwk == jwk:
expired_key_found = True
self.assertTrue(expired_key_found)
self.assertIn(jwk, visible_jwks)
self.assertNotIn(jwk, invisible_jwks)
def test_get_service_key(self):
@ -269,6 +275,17 @@ class KeyServerTestCase(EndpointTestCase):
self.getResponse('key_server.get_service_key', service='sample_service', kid='kid420',
expected_code=409)
# Attempt to rotate the key. Since not approved, it will fail.
token = jwt.encode(payload, private_key.exportKey('PEM'), 'RS256', headers={'kid': 'kid420'})
self.putResponse('key_server.put_service_key', service='sample_service', kid='kid6969',
headers={
'Authorization': 'Bearer %s' % token,
'Content-Type': 'application/json',
}, data=jwk, expected_code=403)
# Approve the key.
model.service_keys.approve_service_key('kid420', 1, ServiceKeyApprovalType.SUPERUSER)
# Rotate that new key
with assert_action_logged('service_key_rotate'):
token = jwt.encode(payload, private_key.exportKey('PEM'), 'RS256', headers={'kid': 'kid420'})
@ -289,20 +306,88 @@ class KeyServerTestCase(EndpointTestCase):
}, data=jwk, expected_code=403)
def test_delete_service_key(self):
def test_attempt_delete_service_key_with_no_kid_signer(self):
# Generate two keys, approving the first.
private_key, _ = model.service_keys.generate_service_key('sample_service', None, kid='first')
# Mint a JWT with our test payload but *no kid*.
token = jwt.encode(self._get_test_jwt_payload(), private_key.exportKey('PEM'), 'RS256',
headers={})
# Using the credentials of our key, attempt to delete our unapproved key
self.deleteResponse('key_server.delete_service_key',
headers={'Authorization': 'Bearer %s' % token},
expected_code=400, service='sample_service', kid='first')
def test_attempt_delete_service_key_with_expired_key(self):
# Generate two keys, approving the first.
private_key, _ = model.service_keys.generate_service_key('sample_service', None, kid='first')
model.service_keys.approve_service_key('first', 1, ServiceKeyApprovalType.SUPERUSER)
model.service_keys.generate_service_key('sample_service', None, kid='second')
# Mint a JWT with our test payload
token = jwt.encode(self._get_test_jwt_payload(), private_key.exportKey('PEM'), 'RS256',
headers={'kid': 'first'})
# Set the expiration of the first to now - some time.
model.service_keys.set_key_expiration('first', datetime.utcnow() - timedelta(seconds=100))
# Using the credentials of our second key, attempt to delete our unapproved key
self.deleteResponse('key_server.delete_service_key',
headers={'Authorization': 'Bearer %s' % token},
expected_code=403, service='sample_service', kid='second')
# Set the expiration to the future and delete the key.
model.service_keys.set_key_expiration('first', datetime.utcnow() + timedelta(seconds=100))
with assert_action_logged('service_key_delete'):
self.deleteResponse('key_server.delete_service_key',
headers={'Authorization': 'Bearer %s' % token},
expected_code=204, service='sample_service', kid='second')
def test_delete_unapproved_service_key(self):
# No Authorization header should yield a 400
self.deleteResponse('key_server.delete_service_key', expected_code=400,
service='sample_service', kid='kid1')
# Generate two keys and approve one
# Generate an unapproved key.
private_key, _ = model.service_keys.generate_service_key('sample_service', None,
kid='unapprovedkeyhere')
# Mint a JWT with our test payload
token = jwt.encode(self._get_test_jwt_payload(), private_key.exportKey('PEM'), 'RS256',
headers={'kid': 'unapprovedkeyhere'})
# Delete our unapproved key with itself.
with assert_action_logged('service_key_delete'):
self.deleteResponse('key_server.delete_service_key',
headers={'Authorization': 'Bearer %s' % token},
expected_code=204, service='sample_service', kid='unapprovedkeyhere')
def test_delete_chained_service_key(self):
# No Authorization header should yield a 400
self.deleteResponse('key_server.delete_service_key', expected_code=400,
service='sample_service', kid='kid1')
# Generate two keys.
private_key, _ = model.service_keys.generate_service_key('sample_service', None, kid='kid123')
model.service_keys.generate_service_key('sample_service', None, kid='kid321')
model.service_keys.approve_service_key('kid123', 1, ServiceKeyApprovalType.SUPERUSER)
# Mint a JWT with our test payload
token = jwt.encode(self._get_test_jwt_payload(), private_key.exportKey('PEM'), 'RS256',
headers={'kid': 'kid123'})
# Using the credentials of our second key, attempt tp delete our unapproved key
self.deleteResponse('key_server.delete_service_key',
headers={'Authorization': 'Bearer %s' % token},
expected_code=403, service='sample_service', kid='kid321')
# Approve the second key.
model.service_keys.approve_service_key('kid123', 1, ServiceKeyApprovalType.SUPERUSER)
# Using the credentials of our approved key, delete our unapproved key
with assert_action_logged('service_key_delete'):
self.deleteResponse('key_server.delete_service_key',