Skip to content
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

[security] Add missing no_log=True, and mark false-positives with no_log=False #223

Merged
merged 5 commits into from
Mar 15, 2021
Merged

[security] Add missing no_log=True, and mark false-positives with no_log=False #223

merged 5 commits into from
Mar 15, 2021

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Most places found by the new sanity test are false-positives, but there are some real problems:

  1. The avi_cloudconnectoruser module has several options which accept credentials for different cloud services. Their documentation is absolutely NOT helpful, and I was only able to find an example for one of them (in https://github.com/avinetworks/devops), but that clearly contained a password.
  2. The avi_sslkeyandcertificate has one option enckey_base64 which might contain a secret. The documentation is also not really helpful; I think it's better to mark this no_log=True as well.
  3. The avi_webhook module has one option verification_token which might be harmless, but might also be a secret. It's not really clear from the documentation and I was not able to find any example.

CC @dericcrago @gundalow @relrod

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

avi_cloudconnectoruser
avi_sslkeyandcertificate
avi_webhook
avi_pool
avi_serviceenginegroup
avi_virtualservice
cnos_user
netscaler_cs_vserver
netscaler_lb_vserver
netscaler_ssl_certkey

dericcrago
dericcrago approved these changes Mar 15, 2021
View reviewed changes
Copy link
Contributor

@dericcrago dericcrago left a comment

Choose a reason for hiding this comment

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

thanks @felixfontein

@dericcrago dericcrago merged commit af60326 into ansible-collections:main Mar 15, 2021
patchback bot pushed a commit that referenced this pull request Mar 15, 2021
@felixfontein
[security] Add missing no_log=True, and mark false-positives with no_…
31cc288 
…log=False (#223)

* Added no_log=False to clear false-positives.

* Some more that seem to be false-positives with the examples from https://github.com/avinetworks/devops.

* Guesswork.

* These definitely miss no_log=True.

* Add changelog fragment.

(cherry picked from commit af60326)
@patchback patchback bot mentioned this pull request Mar 15, 2021
Merged
patchback bot pushed a commit that referenced this pull request Mar 15, 2021
@felixfontein
[security] Add missing no_log=True, and mark false-positives with no_…
c159006 
…log=False (#223)

* Added no_log=False to clear false-positives.

* Some more that seem to be false-positives with the examples from https://github.com/avinetworks/devops.

* Guesswork.

* These definitely miss no_log=True.

* Add changelog fragment.

(cherry picked from commit af60326)
@patchback patchback bot mentioned this pull request Mar 15, 2021
Merged
@felixfontein felixfontein deleted the secrets branch March 15, 2021 15:09
@felixfontein
Copy link
Collaborator Author

@Andersson007 @dericcrago thanks for reviewing and merging!

dericcrago pushed a commit that referenced this pull request Mar 15, 2021
@patchback @felixfontein
[security] Add missing no_log=True, and mark false-positives with no_…
acd74b5 
…log=False (#223) (#224)

* Added no_log=False to clear false-positives.

* Some more that seem to be false-positives with the examples from https://github.com/avinetworks/devops.

* Guesswork.

* These definitely miss no_log=True.

* Add changelog fragment.

(cherry picked from commit af60326)

Co-authored-by: Felix Fontein <[email protected]>
dericcrago pushed a commit that referenced this pull request Mar 15, 2021
@patchback @felixfontein
[security] Add missing no_log=True, and mark false-positives with no_…
17b7872 
…log=False (#223) (#225)

* Added no_log=False to clear false-positives.

* Some more that seem to be false-positives with the examples from https://github.com/avinetworks/devops.

* Guesswork.

* These definitely miss no_log=True.

* Add changelog fragment.

(cherry picked from commit af60326)

Co-authored-by: Felix Fontein <[email protected]>
felixfontein added a commit to felixfontein/ansible that referenced this pull request Mar 15, 2021
@felixfontein
Backport of ansible-collections/community.network#223.
83ddf6d
@felixfontein felixfontein mentioned this pull request Mar 15, 2021
Merged
felixfontein added a commit to felixfontein/ansible that referenced this pull request Mar 15, 2021
@felixfontein
Backport of ansible-collections/community.network#223.
e1a9c6c
@felixfontein felixfontein mentioned this pull request Mar 15, 2021
Merged
relrod pushed a commit to ansible/ansible that referenced this pull request Apr 3, 2021
@felixfontein
Backport of ansible-collections/community.network#223. (#73909)
aee69d7
relrod pushed a commit to ansible/ansible that referenced this pull request Apr 3, 2021
@felixfontein
Backport of ansible-collections/community.network#223. (#73913)
ede3b02
clrpackages pushed a commit to clearlinux-pkgs/ansible that referenced this pull request Apr 15, 2021
@phmccarty @clrpackages
ansible: Autospec creation for update from version 2.9.19 to version …
69e3890 
…2.9.20

Alina Buzachis (1):
      New AWS module mod_defaults - rds_option_group (_info) modules (#74098)

Carlos Camacho (1):
      [stable-2.9] Fix: nmcli bridge-slave fails with error (#74125)

Felix Fontein (4):
      Backport of ansible-collections/community.docker#103. (#73890)
      Backport of ansible-collections/community.aws#475. (#73894)
      Backport of ansible-collections/community.general#2018. (#73893)
      Backport of ansible-collections/community.network#223. (#73909)

Jill R (1):
      New AWS module mod_defaults - wafv2 modules (#73975)

Mark Chappell (3):
      Ensure unit test paths for connection and inventory plugins are based on the context (#73877)
      Partial backport of community.aws/471 - no_log=True for aws_secret (#73874)
      [backport/2.9] module_defaults: Add rds_snapshot (#74113)

Matt Clay (1):
      [stable-2.9] Fix ansible-test coverage exporting.

Matt Martz (1):
      [stable-2.9] Ensure task from the worker is finalized/squashed (#73881) (#73929)

Rick Elrod (5):
      Update Ansible release version to v2.9.19.post0.
      [security] Add more missing no_logs (#74115)
      New release v2.9.20rc1
      Update Ansible release version to v2.9.20rc1.post0.
      New release v2.9.20

Sam Doran (2):
      Move file needed by cs_volume test to S3
      [stable-2.9] find - set proper default based on use_regex (#73961) (#73966)

Xabier Napal (1):
      Fix wrong backup directory var name in apt module (#73840) (#74003)

nitzmahone (1):
      add optional module_utils import support (#73832) (#73916)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Andersson007 Andersson007 Andersson007 approved these changes

@dericcrago dericcrago dericcrago approved these changes

@gundalow gundalow Awaiting requested review from gundalow

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@felixfontein @dericcrago @Andersson007