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

Issue with removing multiple secrets on consecutive lines on ios.rb due to \s matching linebreaks. #3239

Open
cory-usap opened this issue Aug 5, 2024 · 2 comments
Assignees
Labels
Milestone

Comments

@cory-usap
Copy link

I found a problem with the ios.rb file when you have multiple snmp-server host lines. As it is now it will only remove the first secret and keep the rest in tact. This is due to the \s matching \r\n which causes it to match most likely would be the rest of the file so it does not match the 2nd and on items.

Example IOS config of what ios.rb produces now.

snmp-server host 192.168.161.33 version 2c <secret hidden>  
snmp-server host 192.168.161.74 version 2c 5kz#thisisfaked
snmp-server host 192.168.161.89 version 2c 5kz#thisisfaked 

The issue is due to this line https://github.com/ytti/oxidized/blob/master/lib/oxidized/model/ios.rb#L30

(in case this link does not work down the road the line in question is cfg.gsub! /^(snmp-server host \S+( vrf \S+)?( informs?)?( version (1|2c|3 (noauth|auth|priv)))?)\s+\S+((\s+\S*)*)\s*/, '\\1 <secret hidden> \\7'

This matches the snmp-server host the secret and then keeps matching \s and \S which in theory should just be matching the rest of the document (I did not verify this but I suspect thats what is happening)

To fix this I adjusted all the lowercase \s matchers with [\t\f\v ] and this allows the gsub to work correctly.

That means that the line mentioned above now looks like such.

cfg.gsub! /^(snmp-server host \S+( vrf \S+)?( informs?)?( version (1|2c|3 (noauth|auth|priv)))?)[\t\f\v ]+\S+(([\t\f\v ]+\S*)*)[\t\f\v ]*/, '\\1 <secret hidden> \\7'

@ytti
Copy link
Owner

ytti commented Aug 5, 2024

\s certainly matches \r and \n, so you are absolutely correct on what is causing the problem. I don't have anything against your solution, however alternative would be to consider each line in isolation.

cfg = cfg.each_line.map { |line| line.gsub(....) }.join

Or

cfg = cfg.each_line.map do |line|
    line.gsub ..1..
    line.gsub ..2..
end.join

Or

In this case, as the regep ends with \s*, we can maybe make it non-greedy and match for lineend, so I think it may work if we replace the end \s*/ with \s*?$/

@cory-usap
Copy link
Author

Only issue I could see with single line would be if someone is matching multi-lines which is pretty rare on IOS but I have noticed a few folks doing multi-line matching for things in my learning how to best utilize Oxidized.

Maybe having both options?

For the time being I am using the ~/.config/oxidized/model/ overwrite method to use the modified Model so this doesn't need to be fixed ASAP and I think time can be spent coming up with the best solution. It will be about 6 months before I can sit down and create a PR for this so if someone else wants to take it they are more then welcome.

@robertcheramy robertcheramy self-assigned this Aug 9, 2024
@robertcheramy robertcheramy added this to the 0.31 milestone Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants