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.

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

hide secrets in debug log (bsc#1221194) #1006

Merged
merged 5 commits into from Mar 19, 2024

Conversation

cfconrad
Copy link
Collaborator

Replaces cdata in a copy of a config node and it's children that contain passwords before logging it.

Please merge #1005 first.

@cfconrad cfconrad force-pushed the hide_secrets_from_logs branch 2 times, most recently from 22901e9 to d1047fc Compare March 19, 2024 09:26
Copy link
Member

@mtomaschewski mtomaschewski left a comment

Choose a reason for hiding this comment

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

src/appconfig.c is about the /etc/wicked/*.xml appconfig files -- extension definition config in this case and not about interface configs/polices.

@mtomaschewski mtomaschewski self-requested a review March 19, 2024 09:34
@cfconrad
Copy link
Collaborator Author

I would simply use ni_debug_config function everywhere, IMO it has now the better api and looks more clean. But don't have hard feelings and take your advice.

Copy link
Member

@mtomaschewski mtomaschewski left a comment

Choose a reason for hiding this comment

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

See previous comment. Commit d1047fc is not needed and I'd not add it.
The xml does not contain any passwords (which would be valid for all users) and will never do. It's useless to clone the appconfig extension definition for nothing.

@mtomaschewski mtomaschewski self-requested a review March 19, 2024 10:04
Copy link
Member

@mtomaschewski mtomaschewski left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mtomaschewski mtomaschewski merged commit 210e557 into openSUSE:master Mar 19, 2024
@cfconrad cfconrad deleted the hide_secrets_from_logs branch March 19, 2024 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants