From 7e0fbeb6258939ca69ac3d99e552b69306f06eb5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 11 Jan 2017 18:45:46 -0500 Subject: [PATCH] Custom SSL certificates config panel Adds a new panel to the superuser config tool, for managing custom SSL certificates in the config bundle [Delivers #135586525] --- endpoints/api/superuser.py | 87 +++++++++++++++++++ requirements-nover.txt | 1 + requirements.txt | 1 + static/css/core-ui.css | 31 +++++++ .../config/config-certificates-field.html | 70 +++++++++++++++ .../directives/config/config-setup-tool.html | 10 +++ static/js/core-config-setup.js | 50 +++++++++++ static/js/directives/ui/file-upload-box.js | 1 - test/test_api_security.py | 52 ++++++++++- test/test_api_usage.py | 79 ++++++++++++++++- test/test_ssl_util.py | 75 ++++++++-------- util/config/provider/fileprovider.py | 3 + util/config/provider/testprovider.py | 13 ++- util/config/validator.py | 2 +- 14 files changed, 434 insertions(+), 41 deletions(-) create mode 100644 static/directives/config/config-certificates-field.html diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index cdb12ca98..1474b84c8 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -4,6 +4,8 @@ import logging import os import string +import pathvalidate + from datetime import datetime from random import SystemRandom @@ -25,6 +27,8 @@ from data import model from data.database import ServiceKeyApprovalType from util.useremails import send_confirmation_email, send_recovery_email from util.license import decode_license, LicenseDecodeError +from util.security.ssl import load_certificate, CertInvalidException +from util.config.validator import EXTRA_CA_DIRECTORY logger = logging.getLogger(__name__) @@ -824,6 +828,89 @@ class SuperUserServiceKeyApproval(ApiResource): abort(403) +@resource('/v1/superuser/customcerts') +@internal_only +@show_if(features.SUPER_USERS) +class SuperUserCustomCertificates(ApiResource): + """ Resource for managing custom certificates. """ + @nickname('getCustomCertificates') + @require_fresh_login + @require_scope(scopes.SUPERUSER) + @verify_not_prod + def get(self): + if SuperUserPermission().can(): + has_extra_certs_path = config_provider.volume_file_exists(EXTRA_CA_DIRECTORY) + extra_certs_found = config_provider.list_volume_directory(EXTRA_CA_DIRECTORY) + if extra_certs_found is None: + return { + 'status': 'file' if has_extra_certs_path else 'none', + } + + cert_views = [] + for extra_cert_path in extra_certs_found: + try: + cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, extra_cert_path) + with config_provider.get_volume_file(cert_full_path) as f: + certificate = load_certificate(f.read()) + cert_views.append({ + 'path': extra_cert_path, + 'names': list(certificate.names), + 'expired': certificate.expired, + }) + except CertInvalidException as cie: + cert_views.append({ + 'path': extra_cert_path, + 'error': cie.message, + }) + except IOError as ioe: + cert_views.append({ + 'path': extra_cert_path, + 'error': ioe.message, + }) + + return { + 'status': 'directory', + 'certs': cert_views, + } + + abort(403) + + +@resource('/v1/superuser/customcerts/') +@internal_only +@show_if(features.SUPER_USERS) +class SuperUserCustomCertificate(ApiResource): + """ Resource for managing a custom certificate. """ + @nickname('uploadCustomCertificate') + @require_fresh_login + @require_scope(scopes.SUPERUSER) + @verify_not_prod + def post(self, certpath): + if SuperUserPermission().can(): + uploaded_file = request.files['file'] + if not uploaded_file: + abort(400) + + certpath = pathvalidate.sanitize_filename(certpath) + cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, certpath) + config_provider.save_volume_file(cert_full_path, uploaded_file) + return '', 204 + + abort(403) + + @nickname('deleteCustomCertificate') + @require_fresh_login + @require_scope(scopes.SUPERUSER) + @verify_not_prod + def delete(self, certpath): + if SuperUserPermission().can(): + cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, certpath) + config_provider.remove_volume_file(cert_full_path) + return '', 204 + + abort(403) + + @resource('/v1/superuser/license') @internal_only @show_if(features.SUPER_USERS) diff --git a/requirements-nover.txt b/requirements-nover.txt index 53b1659ab..e46f96ee6 100644 --- a/requirements-nover.txt +++ b/requirements-nover.txt @@ -38,6 +38,7 @@ mixpanel mock moto==0.4.25 # remove when 0.4.28+ is out namedlist +pathvalidate peewee==2.8.1 psutil psycopg2 diff --git a/requirements.txt b/requirements.txt index 2753c81b2..dc4664854 100644 --- a/requirements.txt +++ b/requirements.txt @@ -66,6 +66,7 @@ oslo.config==3.17.0 oslo.i18n==3.9.0 oslo.serialization==2.13.0 oslo.utils==3.16.0 +pathvalidate==0.13.0 pbr==1.10.0 peewee==2.8.1 Pillow==3.4.2 diff --git a/static/css/core-ui.css b/static/css/core-ui.css index 0a7fa69ef..cb7525cc9 100644 --- a/static/css/core-ui.css +++ b/static/css/core-ui.css @@ -519,6 +519,37 @@ a:focus { width: 350px; } +.config-certificates-field-element .dns-name { + display: inline-block; + margin-right: 10px; +} + +.config-certificates-field-element .cert-status .fa { + margin-right: 4px; +} + +.config-certificates-field-element .cert-status .green { + color: #2FC98E; +} + +.config-certificates-field-element .cert-status .orange { + color: #FCA657; +} + +.config-certificates-field-element .cert-status .red { + color: #D64456; +} + +.config-certificates-field-element .file-upload-box-element .file-input-container { + padding: 0px; + text-align: left; +} + +.config-certificates-field-element .file-upload-box-element .file-drop + label { + margin-top: 0px; + margin-bottom: 4px; +} + .config-list-field-element .empty { color: #ccc; margin-bottom: 10px; diff --git a/static/directives/config/config-certificates-field.html b/static/directives/config/config-certificates-field.html new file mode 100644 index 000000000..502475672 --- /dev/null +++ b/static/directives/config/config-certificates-field.html @@ -0,0 +1,70 @@ +
+
+ +
+ extra_ca_certs is a single file and cannot be processed by this tool. If a valid and appended list of certificates, they will be installed on container startup. +
+ +
+
+

This section lists any custom or self-signed SSL certificates that are installed in the container on startup after being read from the extra_ca_certs directory in the configuration volume. +

+

+ Custom certificates are typically used in place of publicly signed certificates for corporate-internal services. +

+
+ + + + + + +
Upload certificates: +
+
+ + + + + + + + + + + + + + +
Certificate FilenameStatusNames Handled
{{ certificate.path }} +
+ + Error: {{ certificate.error }} +
+
+ + Certificate is expired +
+
+ + Certificate is valid +
+ 		+
(None)
+ {{ name }} + 		+ + + Delete Certificate + + +
+
+
No custom certificates found.
+
+
+
+
\ No newline at end of file diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html index 0b4657e56..23b043399 100644 --- a/static/directives/config/config-setup-tool.html +++ b/static/directives/config/config-setup-tool.html @@ -13,6 +13,16 @@ + +
+
+ Custom SSL Certificates +
+
+
+
+
+
diff --git a/static/js/core-config-setup.js b/static/js/core-config-setup.js index 95c0bf1f4..827a78871 100644 --- a/static/js/core-config-setup.js +++ b/static/js/core-config-setup.js @@ -1254,6 +1254,56 @@ angular.module("core-config-setup", ['angularFileUpload']) return directiveDefinitionObject; }) + .directive('configCertificatesField', function () { + var directiveDefinitionObject = { + priority: 0, + templateUrl: '/static/directives/config/config-certificates-field.html', + replace: false, + transclude: false, + restrict: 'C', + scope: { + }, + controller: function($scope, $element, $upload, ApiService, UserService) { + $scope.resetUpload = 0; + + var loadCertificates = function() { + $scope.certificatesResource = ApiService.getCustomCertificatesAsResource().get(function(resp) { + $scope.certInfo = resp; + }); + }; + + UserService.updateUserIn($scope, function(user) { + if (!user.anonymous) { + loadCertificates(); + } + }); + + $scope.handleCertsSelected = function(files, callback) { + $upload.upload({ + url: '/api/v1/superuser/customcerts/' + files[0].name, + method: 'POST', + data: {'_csrf_token': window.__token}, + file: files[0] + }).success(function() { + callback(true); + $scope.resetUpload++; + loadCertificates(); + }); + }; + + $scope.deleteCert = function(path) { + var errorDisplay = ApiService.errorDisplay('Could not delete certificate'); + var params = { + 'certpath': path + }; + + ApiService.deleteCustomCertificate(null, params).then(loadCertificates, errorDisplay); + }; + } + }; + return directiveDefinitionObject; + }) + .directive('configLicenseField', function () { var directiveDefinitionObject = { priority: 0, diff --git a/static/js/directives/ui/file-upload-box.js b/static/js/directives/ui/file-upload-box.js index ee37d1a16..6a0931afa 100644 --- a/static/js/directives/ui/file-upload-box.js +++ b/static/js/directives/ui/file-upload-box.js @@ -9,7 +9,6 @@ angular.module('quay').directive('fileUploadBox', function () { transclude: true, restrict: 'C', scope: { - 'allowMultiple': '@allowMultiple', 'selectMessage': '@selectMessage', 'filesSelected': '&filesSelected', diff --git a/test/test_api_security.py b/test/test_api_security.py index 93240b36e..f3a2105ea 100644 --- a/test/test_api_security.py +++ b/test/test_api_security.py @@ -51,7 +51,9 @@ from endpoints.api.superuser import (SuperUserLogs, SuperUserList, SuperUserMana SuperUserOrganizationManagement, SuperUserOrganizationList, SuperUserAggregateLogs, SuperUserServiceKeyManagement, SuperUserServiceKey, SuperUserServiceKeyApproval, - SuperUserTakeOwnership, SuperUserLicense) + SuperUserTakeOwnership, SuperUserLicense, + SuperUserCustomCertificates, + SuperUserCustomCertificate) from endpoints.api.globalmessages import GlobalUserMessage, GlobalUserMessages from endpoints.api.secscan import RepositoryImageSecurity from endpoints.api.manifest import RepositoryManifestLabels, ManageRepositoryManifestLabel @@ -4170,6 +4172,54 @@ class TestSuperUserList(ApiTestCase): self._run_test('GET', 200, 'devtable', None) +class TestSuperUserCustomCertificates(ApiTestCase): + def setUp(self): + ApiTestCase.setUp(self) + self._set_url(SuperUserCustomCertificates) + + def test_get_anonymous(self): + self._run_test('GET', 401, None, None) + + def test_get_freshuser(self): + self._run_test('GET', 403, 'freshuser', None) + + def test_get_reader(self): + self._run_test('GET', 403, 'reader', None) + + def test_get_devtable(self): + self._run_test('GET', 200, 'devtable', None) + + +class TestSuperUserCustomCertificate(ApiTestCase): + def setUp(self): + ApiTestCase.setUp(self) + self._set_url(SuperUserCustomCertificate, certpath='somecert.crt') + + def test_post_anonymous(self): + self._run_test('POST', 401, None, None) + + def test_post_freshuser(self): + self._run_test('POST', 403, 'freshuser', None) + + def test_post_reader(self): + self._run_test('POST', 403, 'reader', None) + + def test_post_devtable(self): + self._run_test('POST', 400, 'devtable', None) + + def test_delete_anonymous(self): + self._run_test('DELETE', 401, None, None) + + def test_delete_freshuser(self): + self._run_test('DELETE', 403, 'freshuser', None) + + def test_delete_reader(self): + self._run_test('DELETE', 403, 'reader', None) + + def test_delete_devtable(self): + self._run_test('DELETE', 204, 'devtable', None) + + class TestSuperUserLicense(ApiTestCase): def setUp(self): ApiTestCase.setUp(self) diff --git a/test/test_api_usage.py b/test/test_api_usage.py index a91ead016..7840945f2 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -66,12 +66,14 @@ from endpoints.api.permission import (RepositoryUserPermission, RepositoryTeamPe RepositoryTeamPermissionList, RepositoryUserPermissionList) from endpoints.api.superuser import (SuperUserLogs, SuperUserList, SuperUserManagement, SuperUserServiceKeyManagement, SuperUserServiceKey, - SuperUserServiceKeyApproval, SuperUserTakeOwnership,) + SuperUserServiceKeyApproval, SuperUserTakeOwnership, + SuperUserCustomCertificates, SuperUserCustomCertificate) from endpoints.api.globalmessages import (GlobalUserMessage, GlobalUserMessages,) from endpoints.api.secscan import RepositoryImageSecurity from endpoints.api.suconfig import (SuperUserRegistryStatus, SuperUserConfig, SuperUserConfigFile, SuperUserCreateInitialSuperUser) from endpoints.api.manifest import RepositoryManifestLabels, ManageRepositoryManifestLabel +from test.test_ssl_util import generate_test_cert try: @@ -4240,6 +4242,81 @@ class TestRepositoryImageSecurity(ApiTestCase): self.assertEquals(1, response['data']['Layer']['IndexedByVersion']) +class TestSuperUserCustomCertificates(ApiTestCase): + def test_custom_certificates(self): + self.login(ADMIN_ACCESS_USER) + + # Upload a certificate. + cert_contents, _ = generate_test_cert(hostname='somecoolhost', san_list=['DNS:bar', 'DNS:baz']) + self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert'), + file=(StringIO(cert_contents), 'testcert'), expected_code=204) + + # Make sure it is present. + json = self.getJsonResponse(SuperUserCustomCertificates) + self.assertEquals(1, len(json['certs'])) + + cert_info = json['certs'][0] + self.assertEquals('testcert', cert_info['path']) + + self.assertEquals(set(['somecoolhost', 'bar', 'baz']), set(cert_info['names'])) + self.assertFalse(cert_info['expired']) + + # Remove the certificate. + self.deleteResponse(SuperUserCustomCertificate, params=dict(certpath='testcert')) + + # Make sure it is gone. + json = self.getJsonResponse(SuperUserCustomCertificates) + self.assertEquals(0, len(json['certs'])) + + def test_expired_custom_certificate(self): + self.login(ADMIN_ACCESS_USER) + + # Upload a certificate. + cert_contents, _ = generate_test_cert(hostname='somecoolhost', expires=-10) + self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert'), + file=(StringIO(cert_contents), 'testcert'), expected_code=204) + + # Make sure it is present. + json = self.getJsonResponse(SuperUserCustomCertificates) + self.assertEquals(1, len(json['certs'])) + + cert_info = json['certs'][0] + self.assertEquals('testcert', cert_info['path']) + + self.assertEquals(set(['somecoolhost']), set(cert_info['names'])) + self.assertTrue(cert_info['expired']) + + def test_invalid_custom_certificate(self): + self.login(ADMIN_ACCESS_USER) + + # Upload an invalid certificate. + self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert'), + file=(StringIO('some contents'), 'testcert'), expected_code=204) + + # Make sure it is present but invalid. + json = self.getJsonResponse(SuperUserCustomCertificates) + self.assertEquals(1, len(json['certs'])) + + cert_info = json['certs'][0] + self.assertEquals('testcert', cert_info['path']) + self.assertEquals('no start line', cert_info['error']) + + def test_path_sanitization(self): + self.login(ADMIN_ACCESS_USER) + + # Upload a certificate. + cert_contents, _ = generate_test_cert(hostname='somecoolhost', expires=-10) + self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert/../foobar'), + file=(StringIO(cert_contents), 'testcert/../foobar'), expected_code=204) + + # Make sure it is present. + json = self.getJsonResponse(SuperUserCustomCertificates) + self.assertEquals(1, len(json['certs'])) + + cert_info = json['certs'][0] + self.assertEquals('foobar', cert_info['path']) + + class TestSuperUserTakeOwnership(ApiTestCase): def test_take_ownership_superuser(self): self.login(ADMIN_ACCESS_USER) diff --git a/test/test_ssl_util.py b/test/test_ssl_util.py index 69bc0a437..f1e0120b0 100644 --- a/test/test_ssl_util.py +++ b/test/test_ssl_util.py @@ -5,42 +5,45 @@ from OpenSSL import crypto from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException +def generate_test_cert(hostname='somehostname', san_list=None, expires=1000000): + """ Generates a test SSL certificate and returns the certificate data and private key data. """ + + # Based on: http://blog.richardknop.com/2012/08/create-a-self-signed-x509-certificate-in-python/ + # Create a key pair. + k = crypto.PKey() + k.generate_key(crypto.TYPE_RSA, 1024) + + # Create a self-signed cert. + cert = crypto.X509() + cert.get_subject().CN = hostname + + # Add the subjectAltNames (if necessary). + if san_list is not None: + cert.add_extensions([crypto.X509Extension("subjectAltName", False, ", ".join(san_list))]) + + cert.set_serial_number(1000) + cert.gmtime_adj_notBefore(0) + cert.gmtime_adj_notAfter(expires) + cert.set_issuer(cert.get_subject()) + + cert.set_pubkey(k) + cert.sign(k, 'sha1') + + # Dump the certificate and private key in PEM format. + cert_data = crypto.dump_certificate(crypto.FILETYPE_PEM, cert) + key_data = crypto.dump_privatekey(crypto.FILETYPE_PEM, k) + + return (cert_data, key_data) + + class TestSSLCertificate(unittest.TestCase): - def _generate_cert(self, hostname='somehostname', san_list=None, expires=1000000): - # Based on: http://blog.richardknop.com/2012/08/create-a-self-signed-x509-certificate-in-python/ - # Create a key pair. - k = crypto.PKey() - k.generate_key(crypto.TYPE_RSA, 1024) - - # Create a self-signed cert. - cert = crypto.X509() - cert.get_subject().CN = hostname - - # Add the subjectAltNames (if necessary). - if san_list is not None: - cert.add_extensions([crypto.X509Extension("subjectAltName", False, ", ".join(san_list))]) - - cert.set_serial_number(1000) - cert.gmtime_adj_notBefore(0) - cert.gmtime_adj_notAfter(expires) - cert.set_issuer(cert.get_subject()) - - cert.set_pubkey(k) - cert.sign(k, 'sha1') - - # Dump the certificate and private key in PEM format. - cert_data = crypto.dump_certificate(crypto.FILETYPE_PEM, cert) - key_data = crypto.dump_privatekey(crypto.FILETYPE_PEM, k) - - return (cert_data, key_data) - def test_load_certificate(self): # Try loading an invalid certificate. with self.assertRaisesRegexp(CertInvalidException, 'no start line'): load_certificate('someinvalidcontents') # Load a valid certificate. - (public_key_data, _) = self._generate_cert() + (public_key_data, _) = generate_test_cert() cert = load_certificate(public_key_data) self.assertFalse(cert.expired) @@ -48,13 +51,13 @@ class TestSSLCertificate(unittest.TestCase): self.assertTrue(cert.matches_name('somehostname')) def test_expired_certificate(self): - (public_key_data, _) = self._generate_cert(expires=-100) + (public_key_data, _) = generate_test_cert(expires=-100) cert = load_certificate(public_key_data) self.assertTrue(cert.expired) def test_hostnames(self): - (public_key_data, _) = self._generate_cert(hostname='foo', san_list=['DNS:bar', 'DNS:baz']) + (public_key_data, _) = generate_test_cert(hostname='foo', san_list=['DNS:bar', 'DNS:baz']) cert = load_certificate(public_key_data) self.assertEquals(set(['foo', 'bar', 'baz']), cert.names) @@ -62,12 +65,12 @@ class TestSSLCertificate(unittest.TestCase): self.assertTrue(cert.matches_name(name)) def test_nondns_hostnames(self): - (public_key_data, _) = self._generate_cert(hostname='foo', san_list=['URI:yarg']) + (public_key_data, _) = generate_test_cert(hostname='foo', san_list=['URI:yarg']) cert = load_certificate(public_key_data) self.assertEquals(set(['foo']), cert.names) def test_validate_private_key(self): - (public_key_data, private_key_data) = self._generate_cert() + (public_key_data, private_key_data) = generate_test_cert() private_key = NamedTemporaryFile(delete=True) private_key.write(private_key_data) @@ -77,7 +80,7 @@ class TestSSLCertificate(unittest.TestCase): cert.validate_private_key(private_key.name) def test_invalid_private_key(self): - (public_key_data, _) = self._generate_cert() + (public_key_data, _) = generate_test_cert() private_key = NamedTemporaryFile(delete=True) private_key.write('somerandomdata') @@ -88,8 +91,8 @@ class TestSSLCertificate(unittest.TestCase): cert.validate_private_key(private_key.name) def test_mismatch_private_key(self): - (public_key_data, _) = self._generate_cert() - (_, private_key_data) = self._generate_cert() + (public_key_data, _) = generate_test_cert() + (_, private_key_data) = generate_test_cert() private_key = NamedTemporaryFile(delete=True) private_key.write(private_key_data) diff --git a/util/config/provider/fileprovider.py b/util/config/provider/fileprovider.py index f59c495f1..b2cd57eba 100644 --- a/util/config/provider/fileprovider.py +++ b/util/config/provider/fileprovider.py @@ -68,6 +68,9 @@ class FileConfigProvider(BaseProvider): if not os.path.exists(dirpath): return None + if not os.path.isdir(dirpath): + return None + return os.listdir(dirpath) def save_volume_file(self, filename, flask_file): diff --git a/util/config/provider/testprovider.py b/util/config/provider/testprovider.py index ecf2376e9..bae362bda 100644 --- a/util/config/provider/testprovider.py +++ b/util/config/provider/testprovider.py @@ -57,7 +57,7 @@ class TestConfigProvider(BaseProvider): return filename in self.files def save_volume_file(self, filename, flask_file): - self.files[filename] = '' + self.files[filename] = flask_file.read() def write_volume_file(self, filename, contents): self.files[filename] = contents @@ -68,6 +68,17 @@ class TestConfigProvider(BaseProvider): return io.BytesIO(self.files[filename]) + def remove_volume_file(self, filename): + self.files.pop(filename, None) + + def list_volume_directory(self, path): + paths = [] + for filename in self.files: + if filename.startswith(path): + paths.append(filename[len(path)+1:]) + + return paths + def requires_restart(self, app_config): return False diff --git a/util/config/validator.py b/util/config/validator.py index 60e44b443..9ff7ea973 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -43,7 +43,7 @@ ACI_CERT_FILENAMES = ['signing-public.gpg', 'signing-private.gpg'] LDAP_FILENAMES = [LDAP_CERT_FILENAME] CONFIG_FILENAMES = (SSL_FILENAMES + DB_SSL_FILENAMES + JWT_FILENAMES + ACI_CERT_FILENAMES + LDAP_FILENAMES) - +EXTRA_CA_DIRECTORY = 'extra_ca_certs' def get_storage_providers(config): storage_config = config.get('DISTRIBUTED_STORAGE_CONFIG', {})