Mailing-List: contact ambari-commits-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: ambari-dev@incubator.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1500086 - in /incubator/ambari/trunk/ambari-server:
 conf/unix/ambari.properties src/main/python/ambari-server.py
 src/test/python/TestAmbaryServer.py
Date: Fri, 05 Jul 2013 17:57:24 -0000
To: ambari-commits@incubator.apache.org
From: mahadev@apache.org
Message-Id: <20130705175724.E830C2388A29@eris.apache.org>

Author: mahadev
Date: Fri Jul  5 17:57:24 2013
New Revision: 1500086

URL: http://svn.apache.org/r1500086
Log:
AMBARI-2579. Improvements in error handling for importing https cert from the user. (Oleksandr Diachenko via mahadev)

Modified:
    incubator/ambari/trunk/ambari-server/conf/unix/ambari.properties
    incubator/ambari/trunk/ambari-server/src/main/python/ambari-server.py
    incubator/ambari/trunk/ambari-server/src/test/python/TestAmbaryServer.py

Modified: incubator/ambari/trunk/ambari-server/conf/unix/ambari.properties
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/ambari-server/conf/unix/ambari.properties?rev=1500086&r1=1500085&r2=1500086&view=diff
==============================================================================
--- incubator/ambari/trunk/ambari-server/conf/unix/ambari.properties (original)
+++ incubator/ambari/trunk/ambari-server/conf/unix/ambari.properties Fri Jul  5 17:57:24 2013
@@ -28,3 +28,4 @@ bootstrap.script=/usr/lib/python2.6/site
 bootstrap.setup_agent.script=/usr/lib/python2.6/site-packages/ambari_server/setupAgent.py
 api.authenticate=true
 server.connection.max.idle.millis=900000
+agent.fqdn.service.url=http://169.254.169.254/latest/meta-data/public-hostname

Modified: incubator/ambari/trunk/ambari-server/src/main/python/ambari-server.py
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/ambari-server/src/main/python/ambari-server.py?rev=1500086&r1=1500085&r2=1500086&view=diff
==============================================================================
--- incubator/ambari/trunk/ambari-server/src/main/python/ambari-server.py (original)
+++ incubator/ambari/trunk/ambari-server/src/main/python/ambari-server.py Fri Jul  5 17:57:24 2013
@@ -96,6 +96,7 @@ RECURSIVE_RM_CMD = 'rm -rf {0}'
 # openssl command
 EXPRT_KSTR_CMD = "openssl pkcs12 -export -in {0} -inkey {1} -certfile {0} -out {3} -password pass:{2} -passin pass:{2}"
 CHANGE_KEY_PWD_CND = 'openssl rsa -in {0} -des3 -out {0}.secured -passout pass:{1}'
+GET_CRT_INFO_CMD = 'openssl x509 -dates -subject -in {0}'
 
 # constants
 STACK_NAME_VER_SEP = "-"
@@ -108,6 +109,11 @@ BOLD_OFF='\033[0m'
 #Common messages
 PRESS_ENTER_MSG="Press <enter> to continue."
 
+#SSL certificate metainfo
+COMMON_NAME_ATTR='CN'
+NOT_BEFORE_ATTR='notBefore'
+NOT_AFTER_ATTR='notAfter'
+
 if ambari_provider_module is not None:
   ambari_provider_module_option = "-Dprovider.module.class=" +\
                                   ambari_provider_module + " "
@@ -167,6 +173,7 @@ SSL_KEYSTORE_FILE_NAME = "https.keystore
 SSL_KEY_PASSWORD_FILE_NAME = "https.pass.txt"
 SSL_KEY_PASSWORD_LENGTH = 50
 DEFAULT_SSL_API_PORT = 8443
+SSL_DATE_FORMAT = '%b  %d %H:%M:%S %Y GMT'
 
 JDBC_RCA_PASSWORD_ALIAS = "ambari.db.password"
 CLIENT_SECURITY_KEY = "client.security"
@@ -286,6 +293,7 @@ JAVA_HOME_PROPERTY = "java.home"
 JDK_URL_PROPERTY='jdk.url'
 JCE_URL_PROPERTY='jce_policy.url'
 OS_TYPE_PROPERTY = "server.os_type"
+GET_FQDN_SERVICE_URL="agent.fqdn.service.url"
 
 JDK_DOWNLOAD_CMD = "curl --create-dirs -o {0} {1}"
 JDK_DOWNLOAD_SIZE_CMD = "curl -I {0}"
@@ -2767,6 +2775,7 @@ def setup_https(args):
     err = 'Ambari-server setup-https should be run with ' \
           'root-level privileges'
     raise FatalException(4, err)
+  args.exit_message = None
   if not SILENT:
     properties = get_ambari_properties()
     try:
@@ -2775,26 +2784,30 @@ def setup_https(args):
                             else properties.get_property(SSL_API_PORT)
       api_ssl = properties.get_property(SSL_API) in ['true']
       cert_was_imported = False
+      cert_must_import = True
       if api_ssl:
-       if get_YN_input("Do you want to disable SSL (y/n) n? ", False):
+       if get_YN_input("Do you want to disable SSL [y/n] n? ", False):
         properties.process_pair(SSL_API, "false")
+        cert_must_import=False
        else:
         properties.process_pair(SSL_API_PORT, \
                                 get_validated_string_input(\
                                 "SSL port ["+str(client_api_ssl_port)+"] ? ",\
                                 str(client_api_ssl_port),\
                                 "^[0-9]{1,5}$", "Invalid port.", False))   
-        import_cert_and_key_action(security_server_keys_dir, properties)
-        cert_was_imported = True  
+        cert_was_imported = import_cert_and_key_action(security_server_keys_dir, properties)
       else:
        if get_YN_input("Do you want to configure HTTPS (y/n) y? ", True):
         properties.process_pair(SSL_API_PORT,\
         get_validated_string_input("SSL port ["+str(client_api_ssl_port)+"] ? ",\
         str(client_api_ssl_port), "^[0-9]{1,5}$", "Invalid port.", False))   
-        import_cert_and_key_action(security_server_keys_dir, properties)
-        cert_was_imported = True        
+        cert_was_imported = import_cert_and_key_action(security_server_keys_dir, properties)
        else:
         return
+      
+      if cert_must_import and not cert_was_imported:
+        print 'Setup of HTTPS failed. Exiting.'
+        return
 
       conf_file = find_properties_file()
       f = open(conf_file, 'w')
@@ -2831,6 +2844,9 @@ def import_cert_and_key_action(security_
    properties.process_pair(SSL_SERVER_CERT_NAME, SSL_CERT_FILE_NAME)
    properties.process_pair(SSL_SERVER_KEY_NAME, SSL_KEY_FILE_NAME)
    properties.process_pair(SSL_API, "true")
+   return True
+  else:
+   return False
    
 def import_cert_and_key(security_server_keys_dir):
   import_cert_path = get_validated_filepath_input(\
@@ -2839,6 +2855,19 @@ def import_cert_and_key(security_server_
   import_key_path  =  get_validated_filepath_input(\
                       "Please enter path to Private Key: ", "Private Key not found")
   pem_password = get_validated_string_input("Please enter password for private key: ", "", None, None, True)
+  
+  certInfoDict = get_cert_info(import_cert_path)
+  
+  if not certInfoDict:
+    print_warning_msg('Error getting certificate information')
+  else:  
+    #Validate common name of certificate
+    if not is_valid_cert_host(certInfoDict):
+      print_warning_msg('Validation of certificate hostname failed')
+  
+    #Validate issue and expirations dates of certificate
+    if not is_valid_cert_exp(certInfoDict):
+      print_warning_msg('Validation of certificate issue and expiration dates failed')
 
   #jetty requires private key files with non-empty key passwords
   retcode = 0
@@ -2871,8 +2900,8 @@ def import_cert_and_key(security_server_
                           security_server_keys_dir, SSL_KEY_FILE_NAME))
    return True
   else:
-   print 'Could not import trusted cerificate and private key:'
-   print err
+   print_error_msg('Could not import Certificate and Private Key.')
+   print 'SSL error on exporting keystore: ' + err.rstrip() + '.'
    return False
  
 def import_file_to_keystore(source, destination):
@@ -2899,6 +2928,117 @@ def get_validated_filepath_input(prompt,
         print description
         input=False
 
+
+def get_cert_info(path):
+  retcode, out, err = run_os_command(GET_CRT_INFO_CMD.format(path))
+  
+  if retcode != 0:
+    print 'Error during getting certificate info'
+    print err
+    return None
+  
+  if out:
+    certInfolist = out.split(os.linesep)
+  else:
+    print 'Empty certificate info'
+    return None
+  
+  notBefore = None
+  notAfter = None
+  subject = None
+  
+  for item in range(len(certInfolist)):
+      
+    if certInfolist[item].startswith('notAfter='):
+      notAfter = certInfolist[item].split('=')[1]
+
+    if certInfolist[item].startswith('notBefore='):
+      notBefore = certInfolist[item].split('=')[1]
+      
+    if certInfolist[item].startswith('subject='):
+      subject = certInfolist[item].split('=', 1)[1]
+      
+  #Convert subj to dict
+  pattern = re.compile(r"[A-Z]{1,2}=[\w.-]{1,}")
+  if subject:
+    subjList = pattern.findall(subject)
+    keys = [item.split('=')[0] for item in subjList]
+    values = [item.split('=')[1] for item in subjList]
+    subjDict = dict(zip(keys, values))
+  
+    result = subjDict
+    result['notBefore'] = notBefore
+    result['notAfter'] = notAfter
+    result['subject'] = subject
+  
+    return result
+  else:
+    return {}
+
+def is_valid_cert_exp(certInfoDict):
+  if certInfoDict.has_key(NOT_BEFORE_ATTR):
+    notBefore = certInfoDict[NOT_BEFORE_ATTR]
+  else:
+    print_warning_msg('There is no Not Before value in certificate')
+    return False
+
+  if certInfoDict.has_key(NOT_AFTER_ATTR):
+    notAfter = certInfoDict['notAfter']
+  else:
+    print_warning_msg('There is no Not After value in certificate')
+    return False
+      
+  
+  notBeforeDate = datetime.datetime.strptime(notBefore, SSL_DATE_FORMAT)
+  notAfterDate = datetime.datetime.strptime(notAfter, SSL_DATE_FORMAT)
+  
+  currentDate = datetime.datetime.now()
+  
+  if currentDate > notAfterDate:
+    print_warning_msg('Certificate was expired on: ' + str(notAfterDate))
+    return False
+    
+  if currentDate < notBeforeDate:
+    print_warning_msg('Certificate will be active from: ' + str(notBeforeDate))
+    return False
+
+  return True
+
+def is_valid_cert_host(certInfoDict):
+  if certInfoDict.has_key(COMMON_NAME_ATTR):
+   commonName = certInfoDict[COMMON_NAME_ATTR]
+  else:
+    print_warning_msg('There is no Common name in certificate')
+    return False
+
+  fqdn = get_fqdn()
+
+  if not fqdn:
+    print_warning_msg('Failed to get server FQDN')
+    return False
+  
+  if commonName != fqdn:
+    print_warning_msg('Common name in certificate: ' + commonName + ' doesn\'t matches the server hostname: ' + fqdn)
+    return False
+
+  return True
+
+
+def get_fqdn():
+  properties = get_ambari_properties()
+  if properties == -1:
+    print "Error getting ambari properties"
+    return None
+
+  get_fqdn_service_url = properties[GET_FQDN_SERVICE_URL]
+  try:
+    handle = urllib2.urlopen(get_fqdn_service_url, '', 2)
+    str = handle.read()
+    handle.close()
+    return str
+  except Exception, e:
+    return socket.getfqdn()
+
 #
 # Main.
 #

Modified: incubator/ambari/trunk/ambari-server/src/test/python/TestAmbaryServer.py
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/ambari-server/src/test/python/TestAmbaryServer.py?rev=1500086&r1=1500085&r2=1500086&view=diff
==============================================================================
--- incubator/ambari/trunk/ambari-server/src/test/python/TestAmbaryServer.py (original)
+++ incubator/ambari/trunk/ambari-server/src/test/python/TestAmbaryServer.py Fri Jul  5 17:57:24 2013
@@ -985,12 +985,18 @@ class TestAmbariServer(TestCase):
   @patch("__builtin__.open")
   @patch("ambari-server.Properties")
   @patch.object(ambari_server, "is_root")
-  def test_setup_https(self, is_root_mock, Properties_mock, open_Mock, get_YN_input_mock,\
+  @patch.object(ambari_server, "is_valid_cert_host")  
+  @patch.object(ambari_server, "is_valid_cert_exp") 
+  def test_setup_https(self, is_valid_cert_exp_mock, is_valid_cert_host_mock,\
+                       is_root_mock, Properties_mock, open_Mock, get_YN_input_mock,\
                        import_cert_and_key_action_mock,
                        is_server_runing_mock, get_ambari_properties_mock,\
                        find_properties_file_mock,\
                        get_validated_string_input_mock,
                        read_ambari_user_method):
+      
+    is_valid_cert_exp_mock.return_value=True
+    is_valid_cert_host_mock.return_value=True
     args = MagicMock()
     open_Mock.return_value = file
     p = get_ambari_properties_mock.return_value
@@ -1119,12 +1125,18 @@ class TestAmbariServer(TestCase):
   @patch.object(ambari_server, "run_os_command")
   @patch("os.path.join")
   @patch.object(ambari_server, "get_validated_filepath_input")
-  @patch.object(ambari_server, "get_validated_string_input")  
-  def test_import_cert_and_key(self, get_validated_string_input_mock,\
+  @patch.object(ambari_server, "get_validated_string_input")
+  @patch.object(ambari_server, "is_valid_cert_host")  
+  @patch.object(ambari_server, "is_valid_cert_exp")  
+  def test_import_cert_and_key(self,is_valid_cert_exp_mock,\
+                               is_valid_cert_host_mock,\
+                               get_validated_string_input_mock,\
                                get_validated_filepath_input_mock,\
                                os_path_join_mock, run_os_command_mock,\
                                open_mock, import_file_to_keystore_mock,\
                                set_file_permissions_mock, read_ambari_user_mock):
+    is_valid_cert_exp_mock.return_value=True
+    is_valid_cert_host_mock.return_value=True
     get_validated_string_input_mock.return_value = "password"
     get_validated_filepath_input_mock.side_effect = \
                                             ["cert_file_path","key_file_path"]
@@ -1155,12 +1167,17 @@ class TestAmbariServer(TestCase):
   @patch("os.path.join")
   @patch.object(ambari_server, "get_validated_filepath_input")
   @patch.object(ambari_server, "get_validated_string_input")
+  @patch.object(ambari_server, "is_valid_cert_host")  
+  @patch.object(ambari_server, "is_valid_cert_exp")  
   def test_import_cert_and_key_with_empty_password(self, \
+    is_valid_cert_exp_mock, is_valid_cert_host_mock,                                         
     get_validated_string_input_mock, get_validated_filepath_input_mock,\
     os_path_join_mock, run_os_command_mock, open_mock, \
     import_file_to_keystore_mock, set_file_permissions_mock,
     read_ambari_user_mock, generate_random_string_mock):
-
+      
+    is_valid_cert_exp_mock.return_value=True
+    is_valid_cert_host_mock.return_value=True
     get_validated_string_input_mock.return_value = ""
     get_validated_filepath_input_mock.side_effect =\
     ["cert_file_path","key_file_path"]
@@ -1183,6 +1200,142 @@ class TestAmbariServer(TestCase):
       expect_import_file_to_keystore)
     self.assertTrue(generate_random_string_mock.called)
 
+
+  def test_is_valid_cert_exp(self):
+    
+    #No data in certInfo
+    certInfo = {}
+    is_valid = ambari_server.is_valid_cert_exp(certInfo)
+    self.assertFalse(is_valid)
+    
+    #Issued in future
+    issuedOn = (datetime.datetime.now() + datetime.timedelta(hours=1000)).strftime(ambari_server.SSL_DATE_FORMAT)
+    expiresOn = (datetime.datetime.now() + datetime.timedelta(hours=2000)).strftime(ambari_server.SSL_DATE_FORMAT)
+    certInfo = {ambari_server.NOT_BEFORE_ATTR : issuedOn,
+                ambari_server.NOT_AFTER_ATTR  : expiresOn}
+    is_valid = ambari_server.is_valid_cert_exp(certInfo)
+    self.assertFalse(is_valid)
+    
+    #Was expired
+    issuedOn = (datetime.datetime.now() - datetime.timedelta(hours=2000)).strftime(ambari_server.SSL_DATE_FORMAT)
+    expiresOn = (datetime.datetime.now() - datetime.timedelta(hours=1000)).strftime(ambari_server.SSL_DATE_FORMAT)
+    certInfo = {ambari_server.NOT_BEFORE_ATTR : issuedOn,
+                ambari_server.NOT_AFTER_ATTR  : expiresOn}
+    is_valid = ambari_server.is_valid_cert_exp(certInfo)
+    self.assertFalse(is_valid)
+    
+    #Valid
+    issuedOn = (datetime.datetime.now() - datetime.timedelta(hours=2000)).strftime(ambari_server.SSL_DATE_FORMAT)
+    expiresOn = (datetime.datetime.now() + datetime.timedelta(hours=1000)).strftime(ambari_server.SSL_DATE_FORMAT)
+    certInfo = {ambari_server.NOT_BEFORE_ATTR : issuedOn,
+                ambari_server.NOT_AFTER_ATTR  : expiresOn}
+    is_valid = ambari_server.is_valid_cert_exp(certInfo)
+    self.assertTrue(is_valid)
+    
+  @patch.object(ambari_server, "get_fqdn")
+  def is_valid_cert_host(self, get_fqdn_mock):
+    
+    #No data in certInfo
+    certInfo = {}
+    is_valid = ambari_server.is_valid_cert_host(certInfo)
+    self.assertFalse(is_valid)
+    
+    #Failed to get FQDN
+    get_fqdn_mock.return_value = None
+    is_valid = ambari_server.is_valid_cert_host(certInfo)
+    self.assertFalse(is_valid)
+    
+    #FQDN and Common name in certificated don't correspond
+    get_fqdn_mock.return_value = 'host1'
+    certInfo = {ambari_server.COMMON_NAME_ATTR : 'host2'}
+    is_valid = ambari_server.is_valid_cert_host(certInfo)
+    self.assertFalse(is_valid)
+    
+    #FQDN and Common name in certificated correspond
+    get_fqdn_mock.return_value = 'host1'
+    certInfo = {ambari_server.COMMON_NAME_ATTR : 'host1'}
+    is_valid = ambari_server.is_valid_cert_host(certInfo)
+    self.assertFalse(is_valid)
+
+  @patch("socket.getfqdn")
+  @patch("urllib2.urlopen")
+  @patch.object(ambari_server, "get_ambari_properties")
+  def test_get_fqdn(self, get_ambari_properties_mock, url_open_mock, getfqdn_mock):
+    
+    #No ambari.properties
+    get_ambari_properties_mock.return_value = -1
+    fqdn = ambari_server.get_fqdn()
+    self.assertEqual(fqdn, None)
+    
+    #Read FQDN from service
+    p = MagicMock()
+    p[ambari_server.GET_FQDN_SERVICE_URL] = 'someurl'
+    get_ambari_properties_mock.return_value = p
+    
+    u = MagicMock()
+    host = 'host1.domain.com'
+    u.read.return_value = host
+    url_open_mock.return_value = u
+    
+    fqdn = ambari_server.get_fqdn()
+    self.assertEqual(fqdn, host)
+    
+    #Failed to read FQDN from service, getting from socket
+    u.reset_mock()
+    u.side_effect = Exception("Failed to read FQDN from service")
+    getfqdn_mock.return_value = host
+    fqdn = ambari_server.get_fqdn()
+    self.assertEqual(fqdn, host)
+    
+
+  @patch.object(ambari_server, "run_os_command")
+  def test_get_cert_info(self, run_os_command_mock):
+    # Error running openssl command
+    path = 'path/to/certificate'
+    run_os_command_mock.return_value = -1, None, None
+    cert_info = ambari_server.get_cert_info(path)
+    self.assertEqual(cert_info, None)
+    
+    #Empty result of openssl command
+    run_os_command_mock.return_value = 0, None, None
+    cert_info = ambari_server.get_cert_info(path)
+    self.assertEqual(cert_info, None)
+    
+    #Positive scenario
+    notAfter = 'Jul  3 14:12:57 2014 GMT'
+    notBefore = 'Jul  3 14:12:57 2013 GMT'
+    attr1_key = 'A'
+    attr1_value = 'foo'
+    attr2_key = 'B'
+    attr2_value = 'bar'
+    attr3_key = 'CN'
+    attr3_value = 'host.domain.com'
+    subject_pattern = '/{attr1_key}={attr1_value}/{attr2_key}={attr2_value}/{attr3_key}={attr3_value}'
+    subject = subject_pattern.format(attr1_key = attr1_key, attr1_value = attr1_value,
+                                     attr2_key = attr2_key, attr2_value = attr2_value,
+                                     attr3_key = attr3_key, attr3_value = attr3_value)
+    out_pattern = """
+notAfter={notAfter}
+notBefore={notBefore}
+subject={subject}
+-----BEGIN CERTIFICATE-----
+MIIFHjCCAwYCCQDpHKOBI+Lt0zANBgkqhkiG9w0BAQUFADBRMQswCQYDVQQGEwJV
+...
+5lqd8XxOGSYoMOf+70BLN2sB
+-----END CERTIFICATE-----
+    """
+    out = out_pattern.format(notAfter = notAfter, notBefore = notBefore, subject = subject)
+    run_os_command_mock.return_value = 0, out, None
+    cert_info = ambari_server.get_cert_info(path)
+    self.assertEqual(cert_info['notAfter'], notAfter)
+    self.assertEqual(cert_info['notBefore'], notBefore)
+    self.assertEqual(cert_info['subject'], subject)
+    self.assertEqual(cert_info[attr1_key], attr1_value)
+    self.assertEqual(cert_info[attr2_key], attr2_value)
+    self.assertEqual(cert_info[attr3_key], attr3_value)
+
+      
+
   @patch.object(ambari_server, "run_os_command")
   @patch("__builtin__.open")
   @patch("os.path.exists")