Skip to content

Prevent passwords and client keys being leaked when tasks fail #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 22, 2022
Merged

Prevent passwords and client keys being leaked when tasks fail #53

merged 6 commits into from
Nov 22, 2022

Conversation

@jackhodgkiss
Copy link
Contributor

@jackhodgkiss jackhodgkiss commented Nov 22, 2022

The original approach caused secrets to be leaked in the event of a task failing.

TASK [stackhpc.pulp.pulp_repository : Setup DEB repositories] *****************************************************************************************
Monday 21 November 2022  17:08:53 +0000 (0:00:00.024)       0:00:00.493 *******
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: urllib.error.URLError: <urlopen error timed out>
failed: [localhost] (item=Ubuntu focal) => changed=false
  ansible_loop_var: item
  item:
    architectures: amd64
    base_path: ubuntu/focal/
    components: main restricted universe multiverse
    distribution_name: ubuntu-focal-
    distributions: focal focal-updates focal-backports focal-security
    mirror: true
    mode: verbatim
    name: Ubuntu focal
    policy: on_demand
    publish: false
    short_name: ubuntu_focal
    state: present
    sync: false
    url: http://nova.clouds.archive.ubuntu.com/ubuntu/
  module_stderr: |-
    Traceback (most recent call last):
      File

The new approach involves iterating over the map against an attribute that can be disclosed such as name. Combined with an index_var and the modules will still receive the information they require.

@jackhodgkiss
feat: change loop approach in pulp_repository
d5d4163 
The original loop approach would leak sensitive data in the event the
task fails. This new approach does not have this issue as it loops over
`name` and use an `index_var` to access the contents of the items.

This same approach will also be included in other roles contained within
this repository.
@jackhodgkiss
feat: change loop approach in pulp_django_user
0e83328
@jackhodgkiss
feat: change loop approach in pulp_content_guard
d908c84
@jackhodgkiss jackhodgkiss self-assigned this Nov 22, 2022
@jackhodgkiss jackhodgkiss marked this pull request as ready for review November 22, 2022 13:24
@jackhodgkiss jackhodgkiss requested a review from a team as a code owner November 22, 2022 13:24
markgoddard
markgoddard approved these changes Nov 22, 2022
View reviewed changes
Copy link

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job

@markgoddard markgoddard merged commit 22aa5f6 into master Nov 22, 2022
@markgoddard markgoddard deleted the dev-786-stop-leaks branch November 22, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@markgoddard markgoddard markgoddard approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jackhodgkiss jackhodgkiss

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jackhodgkiss @markgoddard