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

Data source username and password are visible in the changelog #13729

Closed
mtbutler07 opened this issue Sep 8, 2023 · 8 comments · Fixed by #15032
Closed

Data source username and password are visible in the changelog #13729

mtbutler07 opened this issue Sep 8, 2023 · 8 comments · Fixed by #15032
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@mtbutler07
Copy link

NetBox version

v3.6.1

Feature type

Change to existing functionality

Proposed functionality

When creating a new data source (git) in NetBox with username/password fields populated, a changelog entry is created that contains the diff of the username/password in plain text.

Screenshot from 2023-09-08 13-30-35

This is not ideal for a number of reasons, the primary one being that it exposes credentials to other users that can view the NetBox changelog.

I'm proposing that the the username and password fields be masked or excluded entirely from the changelog entry to prevent exposing credentials.

Use case

It would prevent exposing credentials to other users that are able to view the changelog.

Database changes

No response

External dependencies

No response

@mtbutler07 mtbutler07 added the type: feature Introduction of new functionality to the application label Sep 8, 2023
@mtbutler07
Copy link
Author

mtbutler07 commented Sep 8, 2023

For the Amazon S3 type, it appears the aws_access_key_id and aws_secret_access_key are also displayed in plain text as well.

@jsenecal
Copy link
Contributor

We probably need a custom method in the serializers for those models to obfuscate specific fields.

@abhi1693
Copy link
Member

We set sensitive_parameters on the DataBackend class, maybe there is a way to call this within the to_objectchange method

sensitive_parameters = []

Copy link
Contributor

github-actions bot commented Jan 5, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Jan 5, 2024
@jeffgdotorg jeffgdotorg removed the pending closure Requires immediate attention to avoid being closed for inactivity label Jan 24, 2024
@jeffgdotorg
Copy link
Contributor

This one and feels important enough to rescue, for the sake of our security posture.

@abhi1693
Copy link
Member

I'll take a stab at this and see if it's doable

@abhi1693 abhi1693 self-assigned this Jan 25, 2024
@abhi1693 abhi1693 added the status: accepted This issue has been accepted for implementation label Jan 25, 2024
@abhi1693
Copy link
Member

I cannot find a way to pop out sensitive parameters from both pre and post change data. The most I was able to do was remove from post change using a hack but it caused slight performance degradation. Maybe another maintainer can help fix this.

@abhi1693 abhi1693 removed their assignment Jan 25, 2024
@abhi1693 abhi1693 added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Jan 25, 2024
@jeremystretch jeremystretch self-assigned this Jan 26, 2024
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jan 26, 2024
@jeremystretch
Copy link
Member

Talked about this a bit with @jeffgdotorg this morning. IIRC we have a mechanism in place already to denote sensitive parameters. I'll take a shot at incorporating this into the serialization logic for data sources.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants