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

Fix session_log failure to hide "no_log" data. #3331

Merged
merged 17 commits into from
Oct 31, 2023
Merged

Conversation

ktbyers
Copy link
Owner

@ktbyers ktbyers commented Oct 30, 2023

Netmiko's session_log failed to properly filter no_log data (password and secret) in certain situations. This appeared to mostly occur on configuration changes (i.e. using send_config_set or send_config_from_file).

It was due to the session_log.write() spanning to calls and hence the no_log filter failing to match the data.

@ktbyers
Copy link
Owner Author

ktbyers commented Oct 30, 2023

Fixes:

#3307

cc @dannywade

Please, test this PR (@dannywade ) and verify it fixes what you saw in #3307

@dannywade
Copy link
Contributor

@ktbyers - I confirmed it works! Test output is below (using the same test from before - sanitizing the 10.1.1.1 IP address). Thank you for looking into this and the quick turnaround!

danslab-sw01#
danslab-sw01#terminal width 511
danslab-sw01#terminal length 0
danslab-sw01#
danslab-sw01#
danslab-sw01#configure terminal
Enter configuration commands, one per line.  End with CNTL/Z.
danslab-sw01(config)#
danslab-sw01(config)#logging host ********
danslab-sw01(config)#logging host 10.1.1.2
danslab-sw01(config)#
danslab-sw01(config)#end
danslab-sw01#
danslab-sw01#
danslab-sw01#exit

@ktbyers
Copy link
Owner Author

ktbyers commented Oct 31, 2023

@dannywade FYI, there still are probably some edge cases where the session_log write operation is broken into two chunks and consequently misses the obfuscation. They should be very rare, but it is hard to cover all the cases (and it is a hard tradeoff between the use/value of the session log itself and the value of filtering/obscuring).

So I would always view the session log as containing sensitive data and act appropriately (of course, it really is this way filter or no filter as the router contains a lot of sensitive information that it will output and the vast majority of people will not filter it all).

@ktbyers ktbyers merged commit 56d2309 into develop Oct 31, 2023
10 checks passed
@dannywade
Copy link
Contributor

@ktbyers - Agreed. Thanks again for working through this - I really appreciate it!

@ktbyers
Copy link
Owner Author

ktbyers commented Oct 31, 2023

@dannywade Yeah, that change ended up being bigger than anticipated :-)

@ktbyers ktbyers deleted the fix_session_log branch February 28, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants