Browse code

Retain vault password as bytes in 2.2

Prior to 2.2.1, the vault password was read in as byes and then remained
bytes all the way through the code. A bug existed where bytes and text
were mixed, leading to a traceback with non-ascii passwords. In devel,
this was fixed by changing the read in password to text type to match
with our overall strategy of converting at the borders. This was
backported to stable-2.2 for the 2.2.1 release.

On reflection, this should not have been backported as it causes
passwords which were originally non-utf-8 to become utf-8. People will
then have their working 2.2.x vault files become in-accessible.

this commit pipes bytes all the way through the system for vault
password. That way if a password is read in as a non-utf-8 character
sequence, it will continue to work in 2.2.2+. This change is only for
the 2.2 branch, not for 2.3 and beyond.

Why not everywhere? The reason is that non-utf-8 passwords will cause
problems when vault files are shared between systems or users. If the
password is read from the prompt and one user/machine has a latin1
encoded locale while a second one has utf-8, the non-ascii password
typed in won't match between machines. Deal with this by making sure
that when we encrypt the data, we always use valid utf-8.

Fixes #20398

Toshio Kuratomi authored on 2017/02/16 07:08:30
Showing 8 changed files
... ...
@@ -169,7 +169,7 @@ class CLI(object):
169 169
 
170 170
         # enforce no newline chars at the end of passwords
171 171
         if vault_pass:
172
-            vault_pass = to_text(vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
172
+            vault_pass = to_bytes(vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
173 173
 
174 174
         return vault_pass
175 175
 
... ...
@@ -185,7 +185,7 @@ class CLI(object):
185 185
             pass
186 186
 
187 187
         if new_vault_pass:
188
-            new_vault_pass = to_text(new_vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
188
+            new_vault_pass = to_bytes(new_vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
189 189
 
190 190
         return new_vault_pass
191 191
 
... ...
@@ -586,7 +586,7 @@ class CLI(object):
586 586
             except (OSError, IOError) as e:
587 587
                 raise AnsibleError("Could not read vault password file %s: %s" % (this_path, e))
588 588
 
589
-        return to_text(vault_pass, errors='surrogate_or_strict')
589
+        return vault_pass
590 590
 
591 591
     def get_opt(self, k, defval=""):
592 592
         """
... ...
@@ -107,7 +107,7 @@ class AdHocCLI(CLI):
107 107
 
108 108
         sshpass    = None
109 109
         becomepass = None
110
-        vault_pass = None
110
+        b_vault_pass = None
111 111
 
112 112
         self.normalize_become_options()
113 113
         (sshpass, becomepass) = self.ask_passwords()
... ...
@@ -117,11 +117,11 @@ class AdHocCLI(CLI):
117 117
 
118 118
         if self.options.vault_password_file:
119 119
             # read vault_pass from a file
120
-            vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
121
-            loader.set_vault_password(vault_pass)
120
+            b_vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
121
+            loader.set_vault_password(b_vault_pass)
122 122
         elif self.options.ask_vault_pass:
123
-            vault_pass = self.ask_vault_passwords()
124
-            loader.set_vault_password(vault_pass)
123
+            b_vault_pass = self.ask_vault_passwords()
124
+            loader.set_vault_password(b_vault_pass)
125 125
 
126 126
         variable_manager = VariableManager()
127 127
         variable_manager.extra_vars = load_extra_vars(loader=loader, options=self.options)
... ...
@@ -94,7 +94,7 @@ class PlaybookCLI(CLI):
94 94
         # Manage passwords
95 95
         sshpass    = None
96 96
         becomepass    = None
97
-        vault_pass = None
97
+        b_vault_pass = None
98 98
         passwords = {}
99 99
 
100 100
         # don't deal with privilege escalation or passwords when we don't need to
... ...
@@ -107,11 +107,11 @@ class PlaybookCLI(CLI):
107 107
 
108 108
         if self.options.vault_password_file:
109 109
             # read vault_pass from a file
110
-            vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
111
-            loader.set_vault_password(vault_pass)
110
+            b_vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
111
+            loader.set_vault_password(b_vault_pass)
112 112
         elif self.options.ask_vault_pass:
113
-            vault_pass = self.ask_vault_passwords()
114
-            loader.set_vault_password(vault_pass)
113
+            b_vault_pass = self.ask_vault_passwords()
114
+            loader.set_vault_password(b_vault_pass)
115 115
 
116 116
         # initial error check, to make sure all specified playbooks are accessible
117 117
         # before we start running anything through the playbook executor
... ...
@@ -42,8 +42,8 @@ class VaultCLI(CLI):
42 42
 
43 43
     def __init__(self, args):
44 44
 
45
-        self.vault_pass = None
46
-        self.new_vault_pass = None
45
+        self.b_vault_pass = None
46
+        self.b_new_vault_pass = None
47 47
         super(VaultCLI, self).__init__(args)
48 48
 
49 49
     def parse(self):
... ...
@@ -99,25 +99,25 @@ class VaultCLI(CLI):
99 99
 
100 100
         if self.options.vault_password_file:
101 101
             # read vault_pass from a file
102
-            self.vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader)
102
+            self.b_vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader)
103 103
 
104 104
         if self.options.new_vault_password_file:
105 105
             # for rekey only
106
-            self.new_vault_pass = CLI.read_vault_password_file(self.options.new_vault_password_file, loader)
106
+            self.b_new_vault_pass = CLI.read_vault_password_file(self.options.new_vault_password_file, loader)
107 107
 
108
-        if not self.vault_pass or self.options.ask_vault_pass:
109
-            self.vault_pass = self.ask_vault_passwords()
108
+        if not self.b_vault_pass or self.options.ask_vault_pass:
109
+            self.b_vault_pass = self.ask_vault_passwords()
110 110
 
111
-        if not self.vault_pass:
111
+        if not self.b_vault_pass:
112 112
             raise AnsibleOptionsError("A password is required to use Ansible's Vault")
113 113
 
114 114
         if self.action == 'rekey':
115
-            if not self.new_vault_pass:
116
-                self.new_vault_pass = self.ask_new_vault_passwords()
117
-            if not self.new_vault_pass:
115
+            if not self.b_new_vault_pass:
116
+                self.b_new_vault_pass = self.ask_new_vault_passwords()
117
+            if not self.b_new_vault_pass:
118 118
                 raise AnsibleOptionsError("A password is required to rekey Ansible's Vault")
119 119
 
120
-        self.editor = VaultEditor(self.vault_pass)
120
+        self.editor = VaultEditor(self.b_vault_pass)
121 121
 
122 122
         self.execute()
123 123
 
... ...
@@ -173,6 +173,6 @@ class VaultCLI(CLI):
173 173
                 raise AnsibleError(f + " does not exist")
174 174
 
175 175
         for f in self.args:
176
-            self.editor.rekey_file(f, self.new_vault_pass)
176
+            self.editor.rekey_file(f, self.b_new_vault_pass)
177 177
 
178 178
         display.display("Rekey successful", stderr=True)
... ...
@@ -71,9 +71,9 @@ class DataLoader():
71 71
         # initialize the vault stuff with an empty password
72 72
         self.set_vault_password(None)
73 73
 
74
-    def set_vault_password(self, vault_password):
75
-        self._vault_password = vault_password
76
-        self._vault = VaultLib(password=vault_password)
74
+    def set_vault_password(self, b_vault_password):
75
+        self._b_vault_password = b_vault_password
76
+        self._vault = VaultLib(b_password=b_vault_password)
77 77
 
78 78
     def load(self, data, file_name='<string>', show_content=True):
79 79
         '''
... ...
@@ -151,7 +151,7 @@ class DataLoader():
151 151
     def _safe_load(self, stream, file_name=None):
152 152
         ''' Implements yaml.safe_load(), except using our custom loader class. '''
153 153
 
154
-        loader = AnsibleLoader(stream, file_name, self._vault_password)
154
+        loader = AnsibleLoader(stream, file_name, self._b_vault_password)
155 155
         try:
156 156
             return loader.get_single_data()
157 157
         finally:
... ...
@@ -359,7 +359,7 @@ class DataLoader():
359 359
                 raise AnsibleError("Problem running vault password script %s (%s)."
360 360
                         " If this is not a script, remove the executable bit from the file." % (' '.join(this_path), to_native(e)))
361 361
             stdout, stderr = p.communicate()
362
-            self.set_vault_password(stdout.strip('\r\n'))
362
+            self.set_vault_password(stdout.strip(b'\r\n'))
363 363
         else:
364 364
             try:
365 365
                 f = open(this_path, "rb")
... ...
@@ -397,7 +397,7 @@ class DataLoader():
397 397
             raise AnsibleFileNotFound("the file_name '%s' does not exist, or is not readable" % to_native(file_path))
398 398
 
399 399
         if not self._vault:
400
-            self._vault = VaultLib(password="")
400
+            self._vault = VaultLib(b_password="")
401 401
 
402 402
         real_path = self.path_dwim(file_path)
403 403
 
... ...
@@ -411,7 +411,7 @@ class DataLoader():
411 411
                     # the decrypt call would throw an error, but we check first
412 412
                     # since the decrypt function doesn't know the file name
413 413
                     data = f.read()
414
-                    if not self._vault_password:
414
+                    if not self._b_vault_password:
415 415
                         raise AnsibleParserError("A vault password must be specified to decrypt %s" % file_path)
416 416
 
417 417
                     data = self._vault.decrypt(data, filename=real_path)
... ...
@@ -164,8 +164,8 @@ def is_encrypted_file(file_obj, start_pos=0, count=-1):
164 164
 
165 165
 class VaultLib:
166 166
 
167
-    def __init__(self, password):
168
-        self.b_password = to_bytes(password, errors='strict', encoding='utf-8')
167
+    def __init__(self, b_password):
168
+        self.b_password = to_bytes(b_password, errors='strict', encoding='utf-8')
169 169
         self.cipher_name = None
170 170
         self.b_version = b'1.1'
171 171
 
... ...
@@ -311,8 +311,8 @@ class VaultLib:
311 311
 
312 312
 class VaultEditor:
313 313
 
314
-    def __init__(self, password):
315
-        self.vault = VaultLib(password)
314
+    def __init__(self, b_password):
315
+        self.vault = VaultLib(b_password)
316 316
 
317 317
     # TODO: mv shred file stuff to it's own class
318 318
     def _shred_file_custom(self, tmp_path):
... ...
@@ -475,7 +475,7 @@ class VaultEditor:
475 475
 
476 476
         return plaintext
477 477
 
478
-    def rekey_file(self, filename, new_password):
478
+    def rekey_file(self, filename, b_new_password):
479 479
 
480 480
         check_prereqs()
481 481
 
... ...
@@ -487,10 +487,10 @@ class VaultEditor:
487 487
             raise AnsibleError("%s for %s" % (to_bytes(e),to_bytes(filename)))
488 488
 
489 489
         # This is more or less an assert, see #18247
490
-        if new_password is None:
490
+        if b_new_password is None:
491 491
             raise AnsibleError('The value for the new_password to rekey %s with is not valid' % filename)
492 492
 
493
-        new_vault = VaultLib(new_password)
493
+        new_vault = VaultLib(b_new_password)
494 494
         new_ciphertext = new_vault.encrypt(plaintext)
495 495
 
496 496
         self.write_data(new_ciphertext, filename)
... ...
@@ -36,12 +36,12 @@ except ImportError:
36 36
 
37 37
 
38 38
 class AnsibleConstructor(Constructor):
39
-    def __init__(self, file_name=None, vault_password=None):
40
-        self._vault_password = vault_password
39
+    def __init__(self, file_name=None, b_vault_password=None):
40
+        self._b_vault_password = b_vault_password
41 41
         self._ansible_file_name = file_name
42 42
         super(AnsibleConstructor, self).__init__()
43 43
         self._vaults = {}
44
-        self._vaults['default'] = VaultLib(password=self._vault_password)
44
+        self._vaults['default'] = VaultLib(b_password=self._b_vault_password)
45 45
 
46 46
     def construct_yaml_map(self, node):
47 47
         data = AnsibleMapping()
... ...
@@ -98,7 +98,7 @@ class AnsibleConstructor(Constructor):
98 98
         value = self.construct_scalar(node)
99 99
         ciphertext_data = to_bytes(value)
100 100
 
101
-        if self._vault_password is None:
101
+        if self._b_vault_password is None:
102 102
             raise ConstructorError(None, None,
103 103
                     "found vault but no vault password provided", node.start_mark)
104 104
 
... ...
@@ -34,7 +34,7 @@ if HAVE_PYYAML_C:
34 34
     class AnsibleLoader(CParser, AnsibleConstructor, Resolver):
35 35
         def __init__(self, stream, file_name=None, vault_password=None):
36 36
             CParser.__init__(self, stream)
37
-            AnsibleConstructor.__init__(self, file_name=file_name, vault_password=vault_password)
37
+            AnsibleConstructor.__init__(self, file_name=file_name, b_vault_password=vault_password)
38 38
             Resolver.__init__(self)
39 39
 else:
40 40
     from yaml.composer import Composer
... ...
@@ -48,5 +48,5 @@ else:
48 48
             Scanner.__init__(self)
49 49
             Parser.__init__(self)
50 50
             Composer.__init__(self)
51
-            AnsibleConstructor.__init__(self, file_name=file_name, vault_password=vault_password)
51
+            AnsibleConstructor.__init__(self, file_name=file_name, b_vault_password=vault_password)
52 52
             Resolver.__init__(self)