Merge pull request #2473 from coreos-inc/certs-fixes

Fixes and improvements around custom certificate handling

conf/init/certs_install.sh

@@ -9,7 +9,7 @@ fi
 
 # Add extra trusted certificates (as a directory)
 if [ -d /conf/stack/extra_ca_certs ]; then
-  if test $(ls -A "/conf/stack/extra_ca_certs"); then
+  if test "$(ls -A "/conf/stack/extra_ca_certs")"; then
       echo "Installing extra certificates found in /conf/stack/extra_ca_certs directory"
       cp /conf/stack/extra_ca_certs/* /usr/local/share/ca-certificates/
       cat /conf/stack/extra_ca_certs/* >> /venv/lib/python2.7/site-packages/requests/cacert.pem

endpoints/api/superuser.py

@@ -3,6 +3,7 @@
 import logging
 import os
 import string
+import subprocess
 
 import pathvalidate
 
@@ -894,9 +895,27 @@ class SuperUserCustomCertificate(ApiResource):
       if not uploaded_file:
         abort(400)
 
+      # Save the certificate.
       certpath = pathvalidate.sanitize_filename(certpath)
+      if not certpath.endswith('.crt'):
+        abort(400)
+
       cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, certpath)
       config_provider.save_volume_file(cert_full_path, uploaded_file)
+
+      # Validate the certificate.
+      try:
+        with config_provider.get_volume_file(cert_full_path) as f:
+          load_certificate(f.read())
+
+        # Call the update script to install the certificate immediately.
+        if not app.config['TESTING']:
+          subprocess.check_call(['/conf/init/certs_install.sh'])
+      except CertInvalidException:
+        pass
+      except IOError:
+        pass
+
       return '', 204
 
     abort(403)

static/directives/config/config-certificates-field.html

@@ -12,6 +12,7 @@
           <p>
           Custom certificates are typically used in place of publicly signed certificates for corporate-internal services.
           </p>
+          <p>Please <strong>make sure</strong> that all custom names used for downstream services (such as Clair) are listed in the certificates below.</p>
         </div>
 
         <table class="config-table" style="margin-bottom: 20px;">
@@ -19,9 +20,10 @@
             <td>Upload certificates:</td>
             <td>
               <div class="file-upload-box"
-                     select-message="Select custom certificate to add to configuration. Must be in PEM format."
+                     select-message="Select custom certificate to add to configuration. Must be in PEM format and end extension '.crt'"
                      files-selected="handleCertsSelected(files, callback)"
-                     reset="resetUpload"></div>
+                     reset="resetUpload"
+                     extensions="['.crt']"></div>
             </td>
           </tr>
         </table>
@@ -33,7 +35,7 @@
             <td>Names Handled</td>
             <td class="options-col"></td>
           </thead>
-          <tr ng-repeat="certificate in certInfo.certs">
+          <tr ng-repeat="certificate in certInfo.certs" ng-if="!certsUploading">
             <td>{{ certificate.path }}</td>
             <td class="cert-status">
               <div ng-if="certificate.error" class="red">
@@ -62,7 +64,11 @@
             </td>
           </tr>
         </table>
-        <div class="empty" ng-if="!certInfo.certs.length" style="margin-top: 20px;">
+        <div ng-if="certsUploading" style="margin-top: 20px; text-align: center;">
+          <div class="cor-loader-inline"></div>
+          Uploading, validating and updating certificate(s)
+        </div>
+        <div class="empty" ng-if="!certInfo.certs.length && !certsUploading" style="margin-top: 20px;">
           <div class="empty-primary-msg">No custom certificates found.</div>
         </div>
       </div>

static/directives/config/config-setup-tool.html

@@ -14,7 +14,7 @@
     </div>
 
     <!-- Custom SSL certificates -->
-    <div class="co-panel">
+    <div class="co-panel" id="custom-ssl">
       <div class="co-panel-heading">
         <i class="fa fa-certificate"></i> Custom SSL Certificates
       </div>
@@ -342,6 +342,16 @@
         </div>
 
         <table class="config-table" ng-if="config.FEATURE_SECURITY_SCANNER">
+          <tr>
+            <td>Authentication Key:</td>
+            <td>
+              <span class="config-service-key-field" service-name="{{ config.SECURITY_SCANNER_ISSUER_NAME || 'secscan' }}"></span>
+              <div class="help-text">
+                The security scanning service requires an authorized service key to speak to Quay. Once setup, the key
+                can be managed in the Service Keys panel under the Super User Admin Panel.
+              </div>
+            </td>
+          </tr>
           <tr>
             <td>Security Scanner Endpoint:</td>
             <td>
@@ -351,15 +361,8 @@
               <div class="help-text">
                 The HTTP URL at which the security scanner is running.
               </div>
-            </td>
+              <div class="co-alert co-alert-info" ng-if="config.SECURITY_SCANNER_ENDPOINT.indexOf('https:') == 0" style="margin-top: 20px;">
-          </tr>
+                Is the security scanner behind a domain signed with a <strong>self-signed TLS certificate</strong>? If so, please make sure to register your SSL CA in the <a href="#custom-ssl">custom certificates panel</a> above.
-          <tr>
-            <td>Authentication Key:</td>
-            <td>
-              <span class="config-service-key-field" service-name="{{ config.SECURITY_SCANNER_ISSUER_NAME || 'secscan' }}"></span>
-              <div class="help-text">
-                The security scanning service requires an authorized service key to speak to Quay. Once setup, the key
-                can be managed in the Service Keys panel under the Super User Admin Panel.
               </div>
             </td>
           </tr>

static/directives/file-upload-box.html

@@ -2,7 +2,9 @@
   <div class="file-input-container">
     <div ng-show="state != 'uploading'">
       <form id="file-drop-form-{{ boxId }}">
-        <input id="file-drop-{{ boxId }}" name="file-drop-{{ boxId }}" class="file-drop" type="file" files-changed="handleFilesChanged(files)">
+        <input id="file-drop-{{ boxId }}" name="file-drop-{{ boxId }}" class="file-drop"
+               type="file" files-changed="handleFilesChanged(files)"
+               accept="{{ getAccepts(extensions) }}">
         <label for="file-drop-{{ boxId }}" ng-class="state">
           <span class="chosen-file">
             <span ng-if="selectedFiles.length">

static/js/core-config-setup.js

@@ -1322,10 +1322,12 @@ angular.module("core-config-setup", ['angularFileUpload'])
       },
       controller: function($scope, $element, $upload, ApiService, UserService) {
         $scope.resetUpload = 0;
+        $scope.certsUploading = false;
 
         var loadCertificates = function() {
           $scope.certificatesResource = ApiService.getCustomCertificatesAsResource().get(function(resp) {
             $scope.certInfo = resp;
+            $scope.certsUploading = false;
           });
         };
 
@@ -1336,6 +1338,7 @@ angular.module("core-config-setup", ['angularFileUpload'])
         });
 
         $scope.handleCertsSelected = function(files, callback) {
+          $scope.certsUploading = true;
           $upload.upload({
             url: '/api/v1/superuser/customcerts/' + files[0].name,
             method: 'POST',

static/js/directives/ui/file-upload-box.js

@@ -15,6 +15,8 @@ angular.module('quay').directive('fileUploadBox', function () {
       'filesCleared': '&filesCleared',
       'filesValidated': '&filesValidated',
 
+      'extensions': '<extensions',
+
       'reset': '=?reset'
     },
     controller: function($rootScope, $scope, $element, ApiService) {
@@ -150,6 +152,14 @@ angular.module('quay').directive('fileUploadBox', function () {
         }
       };
 
+      $scope.getAccepts = function(extensions) {
+        if (!extensions || !extensions.length) {
+          return '*';
+        }
+
+        return extensions.join(',');
+      };
+
       $scope.$watch('reset', function(reset) {
         if (reset) {
           $scope.state = 'clear';

test/test_api_usage.py

@@ -4457,21 +4457,21 @@ class TestSuperUserCustomCertificates(ApiTestCase):
 
     # Upload a certificate.
     cert_contents, _ = generate_test_cert(hostname='somecoolhost', san_list=['DNS:bar', 'DNS:baz'])
-    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert'),
+    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert.crt'),
-                      file=(StringIO(cert_contents), 'testcert'), expected_code=204)
+                      file=(StringIO(cert_contents), 'testcert.crt'), 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('testcert.crt', 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'))
+    self.deleteResponse(SuperUserCustomCertificate, params=dict(certpath='testcert.crt'))
 
     # Make sure it is gone.
     json = self.getJsonResponse(SuperUserCustomCertificates)
@@ -4482,15 +4482,15 @@ class TestSuperUserCustomCertificates(ApiTestCase):
 
     # Upload a certificate.
     cert_contents, _ = generate_test_cert(hostname='somecoolhost', expires=-10)
-    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert'),
+    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert.crt'),
-                      file=(StringIO(cert_contents), 'testcert'), expected_code=204)
+                      file=(StringIO(cert_contents), 'testcert.crt'), 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('testcert.crt', cert_info['path'])
 
     self.assertEquals(set(['somecoolhost']), set(cert_info['names']))
     self.assertTrue(cert_info['expired'])
@@ -4499,15 +4499,15 @@ class TestSuperUserCustomCertificates(ApiTestCase):
     self.login(ADMIN_ACCESS_USER)
 
     # Upload an invalid certificate.
-    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert'),
+    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert.crt'),
-                      file=(StringIO('some contents'), 'testcert'), expected_code=204)
+                      file=(StringIO('some contents'), 'testcert.crt'), 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('testcert.crt', cert_info['path'])
     self.assertEquals('no start line', cert_info['error'])
 
   def test_path_sanitization(self):
@@ -4515,15 +4515,15 @@ class TestSuperUserCustomCertificates(ApiTestCase):
 
     # Upload a certificate.
     cert_contents, _ = generate_test_cert(hostname='somecoolhost', expires=-10)
-    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert/../foobar'),
+    self.postResponse(SuperUserCustomCertificate, params=dict(certpath='testcert/../foobar.crt'),
-                      file=(StringIO(cert_contents), 'testcert/../foobar'), expected_code=204)
+                      file=(StringIO(cert_contents), 'testcert/../foobar.crt'), 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'])
+    self.assertEquals('foobar.crt', cert_info['path'])
 
 
 class TestSuperUserTakeOwnership(ApiTestCase):

util/config/validators/test/test_validate_secscan.py

@@ -11,26 +11,25 @@ def test_validate_noop(unvalidated_config, app):
   SecurityScannerValidator.validate(unvalidated_config, None, None)
 
 
-@pytest.mark.parametrize('unvalidated_config, expected_error, error_message', [
+@pytest.mark.parametrize('unvalidated_config, expected_error', [
   ({
     'TESTING': True,
     'DISTRIBUTED_STORAGE_PREFERENCE': [],
     'FEATURE_SECURITY_SCANNER': True,
     'SECURITY_SCANNER_ENDPOINT': 'http://invalidhost',
-  }, Exception, 'Connection error when trying to connect to security scanner endpoint'),
+  }, Exception),
 
   ({
     'TESTING': True,
     'DISTRIBUTED_STORAGE_PREFERENCE': [],
     'FEATURE_SECURITY_SCANNER': True,
     'SECURITY_SCANNER_ENDPOINT': 'http://fakesecurityscanner',
-  }, None, None),
+  }, None),
 ])
-def test_validate(unvalidated_config, expected_error, error_message, app):
+def test_validate(unvalidated_config, expected_error, app):
   with fake_security_scanner(hostname='fakesecurityscanner'):
     if expected_error is not None:
-      with pytest.raises(expected_error) as ipe:
+      with pytest.raises(expected_error):
         SecurityScannerValidator.validate(unvalidated_config, None, None)
-        assert ipe.value.message == error_message
     else:
       SecurityScannerValidator.validate(unvalidated_config, None, None)

util/secscan/api.py

@@ -166,15 +166,18 @@ class SecurityScannerAPI(object):
     """
     try:
       return self._call('GET', _API_METHOD_PING)
-    except requests.exceptions.Timeout:
+    except requests.exceptions.Timeout as tie:
       logger.exception('Timeout when trying to connect to security scanner endpoint')
-      raise Exception('Timeout when trying to connect to security scanner endpoint')
+      msg = 'Timeout when trying to connect to security scanner endpoint: %s' % tie.message
-    except requests.exceptions.ConnectionError:
+      raise Exception(msg)
+    except requests.exceptions.ConnectionError as ce:
       logger.exception('Connection error when trying to connect to security scanner endpoint')
-      raise Exception('Connection error when trying to connect to security scanner endpoint')
+      msg = 'Connection error when trying to connect to security scanner endpoint: %s' % ce.message
-    except (requests.exceptions.RequestException, ValueError):
+      raise Exception(msg)
+    except (requests.exceptions.RequestException, ValueError) as ve:
       logger.exception('Exception when trying to connect to security scanner endpoint')
-      raise Exception('Exception when trying to connect to security scanner endpoint')
+      msg = 'Exception when trying to connect to security scanner endpoint: %s' % ve
+      raise Exception(msg)
 
   def delete_layer(self, layer):
     """ Calls DELETE on the given layer in the security scanner, removing it from