mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
Retain vault password as bytes in 2.2 (#22378)
* 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 (cherry picked from commit 5dcce0666a81917c68b76286685642fd72d84327)
This commit is contained in:
parent
8e910cce8a
commit
edcbef27ec
8 changed files with 47 additions and 47 deletions
|
@ -174,7 +174,7 @@ class CLI(with_metaclass(ABCMeta, object)):
|
|||
|
||||
# enforce no newline chars at the end of passwords
|
||||
if vault_pass:
|
||||
vault_pass = to_text(vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
|
||||
vault_pass = to_bytes(vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
|
||||
|
||||
return vault_pass
|
||||
|
||||
|
@ -190,7 +190,7 @@ class CLI(with_metaclass(ABCMeta, object)):
|
|||
pass
|
||||
|
||||
if new_vault_pass:
|
||||
new_vault_pass = to_text(new_vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
|
||||
new_vault_pass = to_bytes(new_vault_pass, errors='surrogate_or_strict', nonstring='simplerepr').strip()
|
||||
|
||||
return new_vault_pass
|
||||
|
||||
|
@ -641,7 +641,7 @@ class CLI(with_metaclass(ABCMeta, object)):
|
|||
except (OSError, IOError) as e:
|
||||
raise AnsibleError("Could not read vault password file %s: %s" % (this_path, e))
|
||||
|
||||
return to_text(vault_pass, errors='surrogate_or_strict')
|
||||
return vault_pass
|
||||
|
||||
def get_opt(self, k, defval=""):
|
||||
"""
|
||||
|
|
|
@ -105,7 +105,7 @@ class AdHocCLI(CLI):
|
|||
|
||||
sshpass = None
|
||||
becomepass = None
|
||||
vault_pass = None
|
||||
b_vault_pass = None
|
||||
|
||||
self.normalize_become_options()
|
||||
(sshpass, becomepass) = self.ask_passwords()
|
||||
|
@ -115,11 +115,11 @@ class AdHocCLI(CLI):
|
|||
|
||||
if self.options.vault_password_file:
|
||||
# read vault_pass from a file
|
||||
vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
|
||||
loader.set_vault_password(vault_pass)
|
||||
b_vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
|
||||
loader.set_vault_password(b_vault_pass)
|
||||
elif self.options.ask_vault_pass:
|
||||
vault_pass = self.ask_vault_passwords()
|
||||
loader.set_vault_password(vault_pass)
|
||||
b_vault_pass = self.ask_vault_passwords()
|
||||
loader.set_vault_password(b_vault_pass)
|
||||
|
||||
variable_manager = VariableManager()
|
||||
variable_manager.extra_vars = load_extra_vars(loader=loader, options=self.options)
|
||||
|
|
|
@ -90,7 +90,7 @@ class PlaybookCLI(CLI):
|
|||
# Manage passwords
|
||||
sshpass = None
|
||||
becomepass = None
|
||||
vault_pass = None
|
||||
b_vault_pass = None
|
||||
passwords = {}
|
||||
|
||||
# initial error check, to make sure all specified playbooks are accessible
|
||||
|
@ -111,11 +111,11 @@ class PlaybookCLI(CLI):
|
|||
|
||||
if self.options.vault_password_file:
|
||||
# read vault_pass from a file
|
||||
vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
|
||||
loader.set_vault_password(vault_pass)
|
||||
b_vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=loader)
|
||||
loader.set_vault_password(b_vault_pass)
|
||||
elif self.options.ask_vault_pass:
|
||||
vault_pass = self.ask_vault_passwords()
|
||||
loader.set_vault_password(vault_pass)
|
||||
b_vault_pass = self.ask_vault_passwords()
|
||||
loader.set_vault_password(b_vault_pass)
|
||||
|
||||
# create the variable manager, which will be shared throughout
|
||||
# the code, ensuring a consistent view of global variables
|
||||
|
|
|
@ -46,11 +46,9 @@ class VaultCLI(CLI):
|
|||
|
||||
def __init__(self, args):
|
||||
|
||||
self.vault_pass = None
|
||||
self.new_vault_pass = None
|
||||
|
||||
self.b_vault_pass = None
|
||||
self.b_new_vault_pass = None
|
||||
self.encrypt_string_read_stdin = False
|
||||
|
||||
super(VaultCLI, self).__init__(args)
|
||||
|
||||
def parse(self):
|
||||
|
@ -128,29 +126,29 @@ class VaultCLI(CLI):
|
|||
|
||||
if self.options.vault_password_file:
|
||||
# read vault_pass from a file
|
||||
self.vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader)
|
||||
self.b_vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader)
|
||||
|
||||
if self.options.new_vault_password_file:
|
||||
# for rekey only
|
||||
self.new_vault_pass = CLI.read_vault_password_file(self.options.new_vault_password_file, loader)
|
||||
self.b_new_vault_pass = CLI.read_vault_password_file(self.options.new_vault_password_file, loader)
|
||||
|
||||
if not self.vault_pass or self.options.ask_vault_pass:
|
||||
self.vault_pass = self.ask_vault_passwords()
|
||||
if not self.b_vault_pass or self.options.ask_vault_pass:
|
||||
self.b_vault_pass = self.ask_vault_passwords()
|
||||
|
||||
if not self.vault_pass:
|
||||
if not self.b_vault_pass:
|
||||
raise AnsibleOptionsError("A password is required to use Ansible's Vault")
|
||||
|
||||
if self.action == 'rekey':
|
||||
if not self.new_vault_pass:
|
||||
self.new_vault_pass = self.ask_new_vault_passwords()
|
||||
if not self.new_vault_pass:
|
||||
if not self.b_new_vault_pass:
|
||||
self.b_new_vault_pass = self.ask_new_vault_passwords()
|
||||
if not self.b_new_vault_pass:
|
||||
raise AnsibleOptionsError("A password is required to rekey Ansible's Vault")
|
||||
|
||||
if self.action == 'encrypt_string':
|
||||
if self.options.encrypt_string_prompt:
|
||||
self.encrypt_string_prompt = True
|
||||
|
||||
self.editor = VaultEditor(self.vault_pass)
|
||||
self.editor = VaultEditor(self.b_vault_pass)
|
||||
|
||||
self.execute()
|
||||
|
||||
|
@ -347,6 +345,6 @@ class VaultCLI(CLI):
|
|||
raise AnsibleError(f + " does not exist")
|
||||
|
||||
for f in self.args:
|
||||
self.editor.rekey_file(f, self.new_vault_pass)
|
||||
self.editor.rekey_file(f, self.b_new_vault_pass)
|
||||
|
||||
display.display("Rekey successful", stderr=True)
|
||||
|
|
|
@ -70,9 +70,9 @@ class DataLoader():
|
|||
# initialize the vault stuff with an empty password
|
||||
self.set_vault_password(None)
|
||||
|
||||
def set_vault_password(self, vault_password):
|
||||
self._vault_password = vault_password
|
||||
self._vault = VaultLib(password=vault_password)
|
||||
def set_vault_password(self, b_vault_password):
|
||||
self._b_vault_password = b_vault_password
|
||||
self._vault = VaultLib(b_password=b_vault_password)
|
||||
|
||||
def load(self, data, file_name='<string>', show_content=True):
|
||||
'''
|
||||
|
@ -150,7 +150,7 @@ class DataLoader():
|
|||
def _safe_load(self, stream, file_name=None):
|
||||
''' Implements yaml.safe_load(), except using our custom loader class. '''
|
||||
|
||||
loader = AnsibleLoader(stream, file_name, self._vault_password)
|
||||
loader = AnsibleLoader(stream, file_name, self._b_vault_password)
|
||||
try:
|
||||
return loader.get_single_data()
|
||||
finally:
|
||||
|
@ -372,7 +372,7 @@ class DataLoader():
|
|||
raise AnsibleFileNotFound("the file_name '%s' does not exist, or is not readable" % to_native(file_path))
|
||||
|
||||
if not self._vault:
|
||||
self._vault = VaultLib(password="")
|
||||
self._vault = VaultLib(b_password="")
|
||||
|
||||
real_path = self.path_dwim(file_path)
|
||||
|
||||
|
@ -386,7 +386,7 @@ class DataLoader():
|
|||
# the decrypt call would throw an error, but we check first
|
||||
# since the decrypt function doesn't know the file name
|
||||
data = f.read()
|
||||
if not self._vault_password:
|
||||
if not self._b_vault_password:
|
||||
raise AnsibleParserError("A vault password must be specified to decrypt %s" % file_path)
|
||||
|
||||
data = self._vault.decrypt(data, filename=real_path)
|
||||
|
|
|
@ -164,8 +164,8 @@ def is_encrypted_file(file_obj, start_pos=0, count=-1):
|
|||
|
||||
class VaultLib:
|
||||
|
||||
def __init__(self, password):
|
||||
self.b_password = to_bytes(password, errors='strict', encoding='utf-8')
|
||||
def __init__(self, b_password):
|
||||
self.b_password = to_bytes(b_password, errors='strict', encoding='utf-8')
|
||||
self.cipher_name = None
|
||||
self.b_version = b'1.1'
|
||||
|
||||
|
@ -311,8 +311,8 @@ class VaultLib:
|
|||
|
||||
class VaultEditor:
|
||||
|
||||
def __init__(self, password):
|
||||
self.vault = VaultLib(password)
|
||||
def __init__(self, b_password):
|
||||
self.vault = VaultLib(b_password)
|
||||
|
||||
# TODO: mv shred file stuff to it's own class
|
||||
def _shred_file_custom(self, tmp_path):
|
||||
|
@ -494,7 +494,7 @@ class VaultEditor:
|
|||
|
||||
return plaintext
|
||||
|
||||
def rekey_file(self, filename, new_password):
|
||||
def rekey_file(self, filename, b_new_password):
|
||||
|
||||
check_prereqs()
|
||||
|
||||
|
@ -510,10 +510,10 @@ class VaultEditor:
|
|||
raise AnsibleError("%s for %s" % (to_bytes(e),to_bytes(filename)))
|
||||
|
||||
# This is more or less an assert, see #18247
|
||||
if new_password is None:
|
||||
if b_new_password is None:
|
||||
raise AnsibleError('The value for the new_password to rekey %s with is not valid' % filename)
|
||||
|
||||
new_vault = VaultLib(new_password)
|
||||
new_vault = VaultLib(b_new_password)
|
||||
new_ciphertext = new_vault.encrypt(plaintext)
|
||||
|
||||
self.write_data(new_ciphertext, filename)
|
||||
|
|
|
@ -36,12 +36,12 @@ except ImportError:
|
|||
|
||||
|
||||
class AnsibleConstructor(SafeConstructor):
|
||||
def __init__(self, file_name=None, vault_password=None):
|
||||
self._vault_password = vault_password
|
||||
def __init__(self, file_name=None, b_vault_password=None):
|
||||
self._b_vault_password = b_vault_password
|
||||
self._ansible_file_name = file_name
|
||||
super(AnsibleConstructor, self).__init__()
|
||||
self._vaults = {}
|
||||
self._vaults['default'] = VaultLib(password=self._vault_password)
|
||||
self._vaults['default'] = VaultLib(b_password=self._b_vault_password)
|
||||
|
||||
def construct_yaml_map(self, node):
|
||||
data = AnsibleMapping()
|
||||
|
@ -98,7 +98,7 @@ class AnsibleConstructor(SafeConstructor):
|
|||
value = self.construct_scalar(node)
|
||||
ciphertext_data = to_bytes(value)
|
||||
|
||||
if self._vault_password is None:
|
||||
if self._b_vault_password is None:
|
||||
raise ConstructorError(None, None,
|
||||
"found vault but no vault password provided", node.start_mark)
|
||||
|
||||
|
@ -156,4 +156,6 @@ AnsibleConstructor.add_constructor(
|
|||
u'!unsafe',
|
||||
AnsibleConstructor.construct_yaml_unsafe)
|
||||
|
||||
AnsibleConstructor.add_constructor(u'!vault', AnsibleConstructor.construct_vault_encrypted_unicode)
|
||||
AnsibleConstructor.add_constructor(
|
||||
u'!vault',
|
||||
AnsibleConstructor.construct_vault_encrypted_unicode)
|
||||
|
|
|
@ -34,7 +34,7 @@ if HAVE_PYYAML_C:
|
|||
class AnsibleLoader(CParser, AnsibleConstructor, Resolver):
|
||||
def __init__(self, stream, file_name=None, vault_password=None):
|
||||
CParser.__init__(self, stream)
|
||||
AnsibleConstructor.__init__(self, file_name=file_name, vault_password=vault_password)
|
||||
AnsibleConstructor.__init__(self, file_name=file_name, b_vault_password=vault_password)
|
||||
Resolver.__init__(self)
|
||||
else:
|
||||
from yaml.composer import Composer
|
||||
|
@ -48,5 +48,5 @@ else:
|
|||
Scanner.__init__(self)
|
||||
Parser.__init__(self)
|
||||
Composer.__init__(self)
|
||||
AnsibleConstructor.__init__(self, file_name=file_name, vault_password=vault_password)
|
||||
AnsibleConstructor.__init__(self, file_name=file_name, b_vault_password=vault_password)
|
||||
Resolver.__init__(self)
|
||||
|
|
Loading…
Reference in a new issue