Skip to content

Vault general fixes and security hardening#83697

Open
bcoca wants to merge 39 commits intoansible:develfrom
bcoca:vault_ciphers
Open

Vault general fixes and security hardening#83697
bcoca wants to merge 39 commits intoansible:develfrom
bcoca:vault_ciphers

Conversation

@bcoca
Copy link
Copy Markdown
Member

@bcoca bcoca commented Jul 31, 2024

Set new default Vault cipher to take the place of the, now deemed week, AES256 cipher, which is still kept for backwards compatibility.

also fixes #51862

ISSUE TYPE
  • Bugfix Pull Request

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. labels Jul 31, 2024
@Naya1217 Naya1217 removed the needs_triage Needs a first human triage before being processed. label Aug 1, 2024
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 8, 2024
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 8, 2024
@ansible ansible deleted a comment from azure-pipelines bot Aug 9, 2024
@ansible ansible deleted a comment from ansibot Aug 9, 2024
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 9, 2024
@ansible ansible deleted a comment from ansibot Aug 9, 2024
@ansibot ansibot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Aug 9, 2024
@ansibot ansibot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Aug 9, 2024
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 12, 2024
@ansible ansible deleted a comment from ansibot Aug 12, 2024
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 12, 2024
@ansible ansible deleted a comment from ansibot Aug 13, 2024
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 13, 2024
@ansible ansible deleted a comment from ansibot Aug 13, 2024
@ansible ansible deleted a comment from azure-pipelines bot Aug 14, 2024
@ansible ansible deleted a comment from webknjaz Aug 14, 2024
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 14, 2024
@bcoca bcoca force-pushed the vault_ciphers branch 2 times, most recently from 2caac12 to 84a53e6 Compare August 15, 2024 15:55
@ansible ansible deleted a comment from ansibot Aug 15, 2024
nitzmahone and others added 22 commits August 29, 2024 15:32
* enable type checking on more public vault API
 can remove adding exc test as it would now be dupe

 ansible.parsing.vault.AnsibleVaultError: Unable to encrypt vault with unsupported method. Invalid value "rotten" for configuration option "setting: VAULT_METHOD ", valid values are: aes256, v2
When config is used to specify the vault method, config reports the error.
When the CLI is used, argparse handles the error because choices is set.
When the API is used, load_vault_method reports the error.

This ensures the error message is appropriate for the context in which the invalid method was specified.
also some fixes
Also fix x2 exception reporting
Only caches on decrypt, encrypt won't hit cache due to salt generation
note: w/o cache, decrypt takes same time as encrypt
@bcoca bcoca force-pushed the vault_ciphers branch 2 times, most recently from d3773fe to 3f9dab2 Compare August 29, 2024 20:34
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 12, 2024
@pb82
Copy link
Copy Markdown

pb82 commented Oct 9, 2024

@bcoca how does this affect existing secrets encrypted with an older version of vault (and the old cipher)? Do I now need to specify method_name in order to decrypt them?

If so, I wonder if this would also require changes in Ansible Tower to handle existing secrets gracefully.

@bcoca
Copy link
Copy Markdown
Member Author

bcoca commented Oct 9, 2024

Decryption takes the info from the vault itself, so as not to impose any additional requirements on the user.

The existing vaults will keep working until we fully retire the old encryption method in the 'far off' future, there will be a full deprecation cycle then.

The new method becomes the default for encrypting in this PR, so if you want to continue using the old method to encrypt, then you do need to pass that explicitly, either on the CLI or via configuration.

There is a documentation update that will go out at the time this PR is merged to ensure all this is explained to users.

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Oct 11, 2024
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue/PR relates to a bug. has_issue needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ensure ansible-vault decrypt to stdout inserts newline before printing success message

7 participants