Merge pull request #19323 from jacksontj/aes_cleanup

Make AES key never hit disk on the master

salt/config.py

@@ -2056,8 +2056,6 @@ def apply_master_config(overrides=None, defaults=None):
     if len(opts['sock_dir']) > len(opts['cachedir']) + 10:
         opts['sock_dir'] = os.path.join(opts['cachedir'], '.salt-unix')
 
-    opts['aes'] = salt.crypt.Crypticle.generate_key_string()
-
     opts['extension_modules'] = (
         opts.get('extension_modules') or
         os.path.join(opts['cachedir'], 'extmods')

salt/crypt.py

@@ -12,7 +12,6 @@ import os
 import sys
 import time
 import hmac
-import shutil
 import hashlib
 import logging
 import traceback
@@ -40,61 +39,27 @@ from salt.exceptions import (
 log = logging.getLogger(__name__)
 
 
-def dropfile(cachedir, user=None, sock_dir=None):
+def dropfile(cachedir, user=None):
     '''
-    Set an AES dropfile to update the publish session key
-
-    A dropfile is checked periodically by master workers to determine
-    if AES key rotation has occurred.
+    Set an AES dropfile to request the master update the publish session key
     '''
-    dfnt = os.path.join(cachedir, '.dfnt')
     dfn = os.path.join(cachedir, '.dfn')
-
-    def ready():
-        '''
-        Because MWorker._update_aes uses second-precision mtime
-        to detect changes to the file, we must avoid writing two
-        versions with the same mtime.
-
-        Note that this only makes rapid updates in serial safe: concurrent
-        updates could still both pass this check and then write two different
-        keys with the same mtime.
-        '''
-        try:
-            stats = os.stat(dfn)
-        except os.error:
-            # Not there, go ahead and write it
-            return True
-        else:
-            if stats.st_mtime == time.time():
-                # The mtime is the current time, we must
-                # wait until time has moved on.
-                return False
-            else:
-                return True
-
-    while not ready():
-        log.warning('Waiting before writing {0}'.format(dfn))
-        time.sleep(1)
-
-    log.info('Rotating AES key')
-    aes = Crypticle.generate_key_string()
+    # set a mask (to avoid a race condition on file creation) and store original.
     mask = os.umask(191)
-    with salt.utils.fopen(dfnt, 'w+') as fp_:
-        fp_.write(aes)
-    if user:
-        try:
-            import pwd
-            uid = pwd.getpwnam(user).pw_uid
-            os.chown(dfnt, uid, -1)
-        except (KeyError, ImportError, OSError, IOError):
-            pass
+    try:
+        log.info('Rotating AES key')
 
-    shutil.move(dfnt, dfn)
-    os.umask(mask)
-    if sock_dir:
-        event = salt.utils.event.SaltEvent('master', sock_dir)
-        event.fire_event({'rotate_aes_key': True}, tag='key')
+        with salt.utils.fopen(dfn, 'w+') as fp_:
+            fp_.write('')
+        if user:
+            try:
+                import pwd
+                uid = pwd.getpwnam(user).pw_uid
+                os.chown(dfn, uid, -1)
+            except (KeyError, ImportError, OSError, IOError):
+                pass
+    finally:
+        os.umask(mask)  # restore original umask
 
 
 def gen_keys(keydir, keyname, keysize, user=None):
@@ -745,7 +710,8 @@ class Crypticle(object):
     SIG_SIZE = hashlib.sha256().digest_size
 
     def __init__(self, opts, key_string, key_size=192):
-        self.keys = self.extract_keys(key_string, key_size)
+        self.key_string = key_string
+        self.keys = self.extract_keys(self.key_string, key_size)
         self.key_size = key_size
         self.serial = salt.payload.Serial(opts)
 

salt/key.py

@@ -752,7 +752,7 @@ class Key(object):
                     pass
         self.check_minion_cache(preserve_minions=matches.get('minions', []))
         if self.opts.get('rotate_aes_key'):
-            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'], self.opts['sock_dir'])
+            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'])
         return (
             self.name_match(match) if match is not None
             else self.dict_match(matches)
@@ -774,7 +774,7 @@ class Key(object):
                     pass
         self.check_minion_cache()
         if self.opts.get('rotate_aes_key'):
-            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'], self.opts['sock_dir'])
+            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'])
         return self.list_keys()
 
     def reject(self, match=None, match_dict=None, include_accepted=False):
@@ -812,7 +812,7 @@ class Key(object):
                     pass
         self.check_minion_cache()
         if self.opts.get('rotate_aes_key'):
-            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'], self.opts['sock_dir'])
+            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'])
         return (
             self.name_match(match) if match is not None
             else self.dict_match(matches)
@@ -843,7 +843,7 @@ class Key(object):
                 pass
         self.check_minion_cache()
         if self.opts.get('rotate_aes_key'):
-            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'], self.opts['sock_dir'])
+            salt.crypt.dropfile(self.opts['cachedir'], self.opts['user'])
         return self.list_keys()
 
     def finger(self, match):

salt/master.py

@@ -7,6 +7,7 @@ involves preparing the three listeners and the workers needed by the master.
 from __future__ import absolute_import
 
 # Import python libs
+import ctypes
 import os
 import re
 import time
@@ -77,6 +78,7 @@ class SMaster(object):
         :param dict opts: The salt options dictionary
         '''
         self.opts = opts
+        self.opts['aes'] = multiprocessing.Array(ctypes.c_char, salt.crypt.Crypticle.generate_key_string())
         self.master_key = salt.crypt.MasterKeys(self.opts)
         self.key = self.__prep_key()
         self.crypticle = self.__prep_crypticle()
@@ -85,7 +87,7 @@ class SMaster(object):
         '''
         Return the crypticle used for AES
         '''
-        return salt.crypt.Crypticle(self.opts, self.opts['aes'])
+        return salt.crypt.Crypticle(self.opts, self.opts['aes'].value)
 
     def __prep_key(self):
         '''
@@ -225,20 +227,35 @@ class Maintenance(multiprocessing.Process):
 
     def handle_key_rotate(self, now):
         '''
-        Rotate the AES key on a schedule
+        Rotate the AES key rotation
         '''
+        to_rotate = False
+        dfn = os.path.join(self.opts['cachedir'], '.dfn')
+        try:
+            stats = os.stat(dfn)
+            if stats.st_mode == 0o100400:
+                to_rotate = True
+            else:
+                log.error('Found dropfile with incorrect permissions, ignoring...')
+            os.remove(dfn)
+        except os.error:
+            pass
+
         if self.opts.get('publish_session'):
             if now - self.rotate >= self.opts['publish_session']:
-                salt.crypt.dropfile(
-                    self.opts['cachedir'],
-                    self.opts['user'],
-                    self.opts['sock_dir'])
-                self.rotate = now
-                if self.opts.get('ping_on_rotate'):
-                    # Ping all minions to get them to pick up the new key
-                    log.debug('Pinging all connected minions '
-                              'due to AES key rotation')
-                    salt.utils.master.ping_all_connected_minions(self.opts)
+                to_rotate = True
+
+        if to_rotate:
+            # should be unecessary-- since no one else should be modifying
+            with self.opts['aes'].get_lock():
+                self.opts['aes'].value = salt.crypt.Crypticle.generate_key_string()
+            self.event.fire_event({'rotate_aes_key': True}, tag='key')
+            self.rotate = now
+            if self.opts.get('ping_on_rotate'):
+                # Ping all minions to get them to pick up the new key
+                log.debug('Pinging all connected minions '
+                          'due to AES key rotation')
+                salt.utils.master.ping_all_connected_minions(self.opts)
 
     def handle_pillargit(self):
         '''
@@ -808,27 +825,10 @@ class MWorker(multiprocessing.Process):
         Check to see if a fresh AES key is available and update the components
         of the worker
         '''
-        dfn = os.path.join(self.opts['cachedir'], '.dfn')
-        try:
-            stats = os.stat(dfn)
-        except os.error:
-            return
-        if stats.st_mode != 0o100400:
-            # Invalid dfn, return
-            return
-        if stats.st_mtime > self.k_mtime:
-            # new key, refresh crypticle
-            with salt.utils.fopen(dfn) as fp_:
-                aes = fp_.read()
-            if len(aes) != 76:
-                return
-            log.debug('New master AES key found by pid {0}'.format(os.getpid()))
-            self.crypticle = salt.crypt.Crypticle(self.opts, aes)
+        if self.opts['aes'].value != self.crypticle.key_string:
+            self.crypticle = salt.crypt.Crypticle(self.opts, self.opts['aes'].value)
             self.clear_funcs.crypticle = self.crypticle
-            self.clear_funcs.opts['aes'] = aes
             self.aes_funcs.crypticle = self.crypticle
-            self.aes_funcs.opts['aes'] = aes
-            self.k_mtime = stats.st_mtime
 
     def run(self):
         '''
@@ -1832,13 +1832,13 @@ class ClearFuncs(object):
             if 'token' in load:
                 try:
                     mtoken = self.master_key.key.private_decrypt(load['token'], 4)
-                    aes = '{0}_|-{1}'.format(self.opts['aes'], mtoken)
+                    aes = '{0}_|-{1}'.format(self.opts['aes'].value, mtoken)
                 except Exception:
                     # Token failed to decrypt, send back the salty bacon to
                     # support older minions
                     pass
             else:
-                aes = self.opts['aes']
+                aes = self.opts['aes'].value
 
             ret['aes'] = pub.public_encrypt(aes, 4)
         else:
@@ -1853,8 +1853,8 @@ class ClearFuncs(object):
                     # support older minions
                     pass
 
-            aes = self.opts['aes']
-            ret['aes'] = pub.public_encrypt(self.opts['aes'], 4)
+            aes = self.opts['aes'].value
+            ret['aes'] = pub.public_encrypt(self.opts['aes'].value, 4)
         # Be aggressive about the signature
         digest = hashlib.sha256(aes).hexdigest()
         ret['sig'] = self.master_key.key.private_encrypt(digest, 5)

salt/pillar/__init__.py

@@ -134,6 +134,9 @@ class Pillar(object):
         # location of file_roots. Issue 5951
         ext_pillar_opts = dict(self.opts)
         ext_pillar_opts['file_roots'] = self.actual_file_roots
+        # TODO: consolidate into "sanitize opts"
+        if 'aes' in ext_pillar_opts:
+            ext_pillar_opts.pop('aes')
         self.merge_strategy = 'smart'
         if opts.get('pillar_source_merging_strategy'):
             self.merge_strategy = opts['pillar_source_merging_strategy']
@@ -591,6 +594,7 @@ class Pillar(object):
         errors.extend(terrors)
         if self.opts.get('pillar_opts', True):
             mopts = dict(self.opts)
+            # TODO: consolidate into sanitize function
             if 'grains' in mopts:
                 mopts.pop('grains')
             if 'aes' in mopts: